Restrict S2K types

ProtonMail · Jul 4, 2024 · 6028a9f · 6028a9f
1 parent 3272cd7
commit 6028a9f
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 0 deletions.
diff --git a/openpgp/keys_test.go b/openpgp/keys_test.go
@@ -1497,6 +1497,9 @@ func TestEncryptAndDecryptPrivateKeys(t *testing.T) {
 					S2KMode: mode,
 				},
 			}
+			if mode == s2k.Argon2S2K {
+				config.AEADConfig = &packet.AEADConfig{}
+			}
 			err = entity.EncryptPrivateKeys(passphrase, config)
 			if err != nil {
 				t.Fatal(err)

diff --git a/openpgp/packet/private_key.go b/openpgp/packet/private_key.go
@@ -261,6 +261,12 @@ func (pk *PrivateKey) parse(r io.Reader) (err error) {
 		if pk.s2kParams.Dummy() {
 			return
 		}
+		if pk.s2kParams.Mode() == s2k.Argon2S2K && pk.s2kType != S2KAEAD {
+			return errors.StructuralError("using Argon2 S2K without AEAD is not allowed")
+		}
+		if pk.s2kParams.Mode() == s2k.SimpleS2K && pk.Version == 6 {
+			return errors.StructuralError("using Simple S2K with version 6 keys is not allowed")
+		}
 		pk.s2k, err = pk.s2kParams.Function()
 		if err != nil {
 			return
@@ -655,6 +661,13 @@ func (pk *PrivateKey) encrypt(key []byte, params *s2k.Params, s2kType S2KType, c
 		return errors.InvalidArgumentError("supplied encryption key has the wrong size")
 	}
 
+	if params.Mode() == s2k.Argon2S2K && s2kType != S2KAEAD {
+		return errors.InvalidArgumentError("using Argon2 S2K without AEAD is not allowed")
+	}
+	if params.Mode() != s2k.Argon2S2K && params.Mode() != s2k.IteratedSaltedS2K {
+		return errors.InvalidArgumentError("insecure S2K mode")
+	}
+
 	priv := bytes.NewBuffer(nil)
 	err := pk.serializePrivateKey(priv)
 	if err != nil {

diff --git a/openpgp/packet/private_key_test.go b/openpgp/packet/private_key_test.go
@@ -144,6 +144,9 @@ func TestExternalPrivateKeyEncryptDecryptS2KModes(t *testing.T) {
 	sk2KeyTypes := []S2KType{S2KAEAD, S2KSHA1}
 	for _, s2kMode := range sk2Modes {
 		for _, sk2KeyType := range sk2KeyTypes {
+			if s2kMode == s2k.Argon2S2K && sk2KeyType == S2KSHA1 {
+				continue
+			}
 			t.Run(fmt.Sprintf("s2kMode:%d-s2kType:%d", s2kMode, sk2KeyType), func(t *testing.T) {
 				var configAEAD *AEADConfig
 				if sk2KeyType == S2KAEAD {

diff --git a/openpgp/s2k/s2k.go b/openpgp/s2k/s2k.go
@@ -300,6 +300,10 @@ func ParseIntoParams(r io.Reader) (params *Params, err error) {
 	return nil, errors.UnsupportedError("S2K function")
 }
 
+func (params *Params) Mode() Mode {
+	return params.mode
+}
+
 func (params *Params) Dummy() bool {
 	return params != nil && params.mode == GnuS2K
 }

diff --git a/openpgp/v2/keys_test.go b/openpgp/v2/keys_test.go
@@ -1475,6 +1475,9 @@ func TestEncryptAndDecryptPrivateKeys(t *testing.T) {
 					S2KMode: mode,
 				},
 			}
+			if mode == s2k.Argon2S2K {
+				config.AEADConfig = &packet.AEADConfig{}
+			}
 			err = entity.EncryptPrivateKeys(passphrase, config)
 			if err != nil {
 				t.Fatal(err)