Restrict S2K types (#213)

RFC9580 says that:

    Argon2 is only used with AEAD (S2K usage octet 253).  An
    implementation MUST NOT create and MUST reject as malformed any
    secret key packet where the S2K usage octet is not AEAD (253) and
    the S2K specifier type is Argon2.

Therefore, we disallow reading and writing Argon2 keys without AEAD.

And:

    [The Simple and Salted S2K methods] are used only for reading in
    backwards compatibility mode.
    
So, we disallow encrypting keys using those methods.

Since v6 keys don't need backwards compatibility, we also disallow
reading Simple S2K there. We still allow reading Salted S2K since the
spec says it may be used "when [the password] is high entropy".

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)

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 {

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 {

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
 }

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)