clamp argon2 memory when parsing untrusted packets

openpgp/packet/config.go

@@ -64,7 +64,7 @@ type Config struct {
 	// CompressionConfig configures the compression settings.
 	CompressionConfig *CompressionConfig
 	// S2K (String to Key) config, used for key derivation in the context of secret key encryption
-	// and password-encrypted data.
+	// and password-encrypted data, as well as for the corresponding decryption.
 	// If nil, the default configuration is used
 	S2KConfig *s2k.Config
 	// Iteration count for Iterated S2K (String to Key).

openpgp/packet/private_key.go

@@ -44,7 +44,6 @@ type PrivateKey struct {
 	Encrypted     bool // if true then the private key is unavailable until Decrypt has been called.
 	encryptedData []byte
 	cipher        CipherFunction
-	s2k           func(out, in []byte)
 	aead          AEADMode // only relevant if S2KAEAD is enabled
 	// An *{rsa|dsa|elgamal|ecdh|ecdsa|ed25519|ed448}.PrivateKey or
 	// crypto.Signer/crypto.Decrypter (Decryptor RSA only).
@@ -231,7 +230,7 @@ func (pk *PrivateKey) parse(r io.Reader) (err error) {
 
 	switch pk.s2kType {
 	case S2KNON:
-		pk.s2k = nil
+		pk.s2kParams = nil
 		pk.Encrypted = false
 	case S2KSHA1, S2KCHECKSUM, S2KAEAD:
 		if (v5 || v6) && pk.s2kType == S2KCHECKSUM {
@@ -281,10 +280,6 @@ func (pk *PrivateKey) parse(r io.Reader) (err error) {
 		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
-		}
 		pk.Encrypted = true
 	default:
 		return errors.UnsupportedError("deprecated s2k function in private key")
@@ -639,21 +634,21 @@ func (pk *PrivateKey) decrypt(decryptionKey []byte) error {
 
 	// Mark key as unencrypted
 	pk.s2kType = S2KNON
-	pk.s2k = nil
+	pk.s2kParams = nil
 	pk.Encrypted = false
 	pk.encryptedData = nil
 	return nil
 }
 
-func (pk *PrivateKey) decryptWithCache(passphrase []byte, keyCache *s2k.Cache) error {
+func (pk *PrivateKey) decryptWithCache(passphrase []byte, keyCache *s2k.Cache, config *Config) error {
 	if pk.Dummy() {
 		return errors.ErrDummyPrivateKey("dummy key found")
 	}
 	if !pk.Encrypted {
 		return nil
 	}
 
-	key, err := keyCache.GetOrComputeDerivedKey(passphrase, pk.s2kParams, pk.cipher.KeySize())
+	key, err := keyCache.GetOrComputeDerivedKeyWithConfig(passphrase, pk.s2kParams, pk.cipher.KeySize(), config.S2K())
 	if err != nil {
 		return err
 	}
@@ -665,29 +660,52 @@ func (pk *PrivateKey) decryptWithCache(passphrase []byte, keyCache *s2k.Cache) e
 
 // Decrypt decrypts an encrypted private key using a passphrase.
 func (pk *PrivateKey) Decrypt(passphrase []byte) error {
+	return pk.DecryptWithConfig(passphrase, nil)
+}
+
+// DecryptWithConfig decrypts an encrypted private key using a passphrase and
+// the config. It fails if the s2k parameters of the key exceed the limits set
+// in the config, i.e. if they ask for more memory than
+// `config.S2KConfig.Argon2Config.MaxMemory`.
+// If config is nil, sensible defaults will be used.
+func (pk *PrivateKey) DecryptWithConfig(passphrase []byte, config *Config) error {
 	if pk.Dummy() {
 		return errors.ErrDummyPrivateKey("dummy key found")
 	}
 	if !pk.Encrypted {
 		return nil
 	}
 
+	s2kFunc, err := pk.s2kParams.FunctionWithConfig(config.S2K())
+	if err != nil {
+		return err
+	}
 	key := make([]byte, pk.cipher.KeySize())
-	pk.s2k(key, passphrase)
+	s2kFunc(key, passphrase)
 	if pk.s2kType == S2KAEAD {
 		key = pk.applyHKDF(key)
 	}
 	return pk.decrypt(key)
 }
 
-// DecryptPrivateKeys decrypts all encrypted keys with the given config and passphrase.
+// DecryptPrivateKeys decrypts all encrypted keys with the given passphrase.
 // Avoids recomputation of similar s2k key derivations.
 func DecryptPrivateKeys(keys []*PrivateKey, passphrase []byte) error {
+	return DecryptPrivateKeysWithConfig(keys, passphrase, nil)
+}
+
+// DecryptPrivateKeysWithConfig decrypts all encrypted keys with the given
+// passphrase and config. It fails if the s2k parameters of a key exceed the
+// limits set in the config, i.e. if they ask for more memory than
+// s2k.Argon2Config.MaxMemory.
+// Avoids recomputation of similar s2k key derivations.
+// If config is nil, sensible defaults will be used.
+func DecryptPrivateKeysWithConfig(keys []*PrivateKey, passphrase []byte, config *Config) error {
 	// Create a cache to avoid recomputation of key derviations for the same passphrase.
 	s2kCache := &s2k.Cache{}
 	for _, key := range keys {
 		if key != nil && !key.Dummy() && key.Encrypted {
-			err := key.decryptWithCache(passphrase, s2kCache)
+			err := key.decryptWithCache(passphrase, s2kCache, config)
 			if err != nil {
 				return err
 			}
@@ -725,10 +743,6 @@ func (pk *PrivateKey) encrypt(key []byte, params *s2k.Params, s2kType S2KType, c
 
 	pk.cipher = cipherFunction
 	pk.s2kParams = params
-	pk.s2k, err = pk.s2kParams.Function()
-	if err != nil {
-		return err
-	}
 
 	privateKeyBytes := priv.Bytes()
 	pk.s2kType = s2kType

openpgp/packet/symmetric_key_encrypted.go

@@ -26,7 +26,7 @@ type SymmetricKeyEncrypted struct {
 	Version      int
 	CipherFunc   CipherFunction
 	Mode         AEADMode
-	s2k          func(out, in []byte)
+	s2kParams    *s2k.Params
 	iv           []byte
 	encryptedKey []byte // Contains also the authentication tag for AEAD
 }
@@ -81,12 +81,12 @@ func (ske *SymmetricKeyEncrypted) parse(r io.Reader) error {
 	}
 
 	var err error
-	if ske.s2k, err = s2k.Parse(r); err != nil {
-		if _, ok := err.(errors.ErrDummyPrivateKey); ok {
-			return errors.UnsupportedError("missing key GNU extension in session key")
-		}
+	if ske.s2kParams, err = s2k.ParseIntoParams(r); err != nil {
 		return err
 	}
+	if ske.s2kParams.Dummy() {
+		return errors.UnsupportedError("missing key GNU extension in session key")
+	}
 
 	if ske.Version >= 5 {
 		// AEAD IV
@@ -120,8 +120,21 @@ func (ske *SymmetricKeyEncrypted) parse(r io.Reader) error {
 // the cipher to use when decrypting a subsequent Symmetrically Encrypted Data
 // packet.
 func (ske *SymmetricKeyEncrypted) Decrypt(passphrase []byte) ([]byte, CipherFunction, error) {
+	return ske.DecryptWithConfig(passphrase, nil)
+}
+
+// DecryptWithConfig attempts to decrypt an encrypted session key and returns the
+// key and the cipher to use when decrypting a subsequent Symmetrically Encrypted
+// Data packet. It fails if the s2k parameters of the packet exceed the limits set
+// in the config, i.e. if it asks for more memory than s2k.Argon2Config.MaxMemory.
+// If config is nil, sensible defaults will be used.
+func (ske *SymmetricKeyEncrypted) DecryptWithConfig(passphrase []byte, config *Config) ([]byte, CipherFunction, error) {
+	s2kFunc, err := ske.s2kParams.FunctionWithConfig(config.S2K())
+	if err != nil {
+		return nil, CipherFunction(0), err
+	}
 	key := make([]byte, ske.CipherFunc.KeySize())
-	ske.s2k(key, passphrase)
+	s2kFunc(key, passphrase)
 	if len(ske.encryptedKey) == 0 {
 		return key, ske.CipherFunc, nil
 	}
@@ -133,7 +146,7 @@ func (ske *SymmetricKeyEncrypted) Decrypt(passphrase []byte) ([]byte, CipherFunc
 		plaintextKey, err := ske.aeadDecrypt(ske.Version, key)
 		return plaintextKey, CipherFunction(0), err
 	}
-	err := errors.UnsupportedError("unknown SymmetricKeyEncrypted version")
+	err = errors.UnsupportedError("unknown SymmetricKeyEncrypted version")
 	return nil, CipherFunction(0), err
 }
 

openpgp/packet/symmetric_key_encrypted_test.go

@@ -285,3 +285,28 @@ func TestSerializeSymmetricKeyEncryptedCiphersV4(t *testing.T) {
 		})
 	}
 }
+
+func TestSymmetricKeyEncryptedArgon2MaxMemory(t *testing.T) {
+	// A v4 SKESK packet whose argon2 parameters ask for 2**31 kibibytes (i.e. 2 TiB) of memory.
+	body := append([]byte{4 /* version */, byte(CipherAES256), byte(s2k.Argon2S2K)}, make([]byte, s2k.Argon2SaltSize)...)
+	body = append(body, 3 /* passes */, 4 /* parallelism */, 31 /* memoryExp */)
+
+	var buf bytes.Buffer
+	if err := serializeHeader(&buf, packetTypeSymmetricKeyEncrypted, len(body)); err != nil {
+		t.Fatalf("failed to serialize the packet header: %s", err)
+	}
+	buf.Write(body)
+
+	p, err := Read(&buf)
+	if err != nil {
+		t.Fatalf("failed to parse the packet: %s", err)
+	}
+	ske, ok := p.(*SymmetricKeyEncrypted)
+	if !ok {
+		t.Fatalf("parsed a different packet type: %#v", p)
+	}
+
+	if _, _, err := ske.Decrypt([]byte("password")); err == nil {
+		t.Error("expected an error for parameters above the default maximum memory")
+	}
+}

openpgp/read.go

@@ -216,7 +216,7 @@ FindKey:
 		// Try the symmetric passphrase first
 		if len(symKeys) != 0 && passphrase != nil {
 			for _, s := range symKeys {
-				key, cipherFunc, err := s.Decrypt(passphrase)
+				key, cipherFunc, err := s.DecryptWithConfig(passphrase, config)
 				// In v4, on wrong passphrase, session key decryption is very likely to result in an invalid cipherFunc:
 				// only for < 5% of cases we will proceed to decrypt the data
 				if err == nil {

openpgp/s2k/s2k.go

@@ -322,7 +322,19 @@ func (params *Params) salt() []byte {
 	}
 }
 
+// Function returns the s2k function described by the parameters.
+// The default configuration is used to bound the resources the
+// derivation is allowed to use.
 func (params *Params) Function() (f func(out, in []byte), err error) {
+	return params.FunctionWithConfig(nil)
+}
+
+// FunctionWithConfig returns the s2k function described by the parameters.
+// It returns an error if the parameters ask for more resources than the
+// configuration allows, i.e. if an Argon2 packet requests more memory than
+// Argon2Config.MaxMemory. The configuration c may be nil, in which case
+// sensible defaults will be used.
+func (params *Params) FunctionWithConfig(c *Config) (f func(out, in []byte), err error) {
 	if params.Dummy() {
 		return nil, errors.ErrDummyPrivateKey("dummy key found")
 	}
@@ -358,6 +370,9 @@ func (params *Params) Function() (f func(out, in []byte), err error) {
 
 		return f, nil
 	case Argon2S2K:
+		if decodeMemory(params.memoryExp) > c.Argon2().MaxMemoryUsage() {
+			return nil, errors.UnsupportedError("argon2 memory exceeds the configured maximum")
+		}
 		f := func(out, in []byte) {
 			Argon2(out, in, params.salt(), params.passes, params.parallelism, params.memoryExp)
 		}
@@ -408,7 +423,7 @@ func Serialize(w io.Writer, key []byte, rand io.Reader, passphrase []byte, c *Co
 		return err
 	}
 
-	f, err := params.Function()
+	f, err := params.FunctionWithConfig(c)
 	if err != nil {
 		return err
 	}

openpgp/s2k/s2k_cache.go

@@ -9,12 +9,23 @@ type Cache map[Params][]byte
 // for the given s2k parameters from the cache.
 // If there is no hit, it derives the key with the s2k function from the passphrase,
 // updates the cache, and returns the key.
+// The default configuration is used to bound the resources the derivation
+// is allowed to use.
 func (c *Cache) GetOrComputeDerivedKey(passphrase []byte, params *Params, expectedKeySize int) ([]byte, error) {
+	return c.GetOrComputeDerivedKeyWithConfig(passphrase, params, expectedKeySize, nil)
+}
+
+// GetOrComputeDerivedKeyWithConfig tries to retrieve the key
+// for the given s2k parameters from the cache.
+// If there is no hit, it derives the key with the s2k function from the passphrase,
+// updates the cache, and returns the key.
+// The configuration config may be nil, in which case sensible defaults will be used.
+func (c *Cache) GetOrComputeDerivedKeyWithConfig(passphrase []byte, params *Params, expectedKeySize int, config *Config) ([]byte, error) {
 	key, found := (*c)[*params]
 	if !found || len(key) != expectedKeySize {
 		var err error
 		derivedKey := make([]byte, expectedKeySize)
-		s2k, err := params.Function()
+		s2k, err := params.FunctionWithConfig(config)
 		if err != nil {
 			return nil, err
 		}

openpgp/s2k/s2k_config.go

@@ -20,7 +20,8 @@ type Config struct {
 	// nil, SHA256 is used.
 	Hash crypto.Hash
 	// Argon2 parameters for S2K (String to Key).
-	// Only relevant if S2KMode is set to s2k.Argon2S2K.
+	// Only relevant if S2KMode is set to s2k.Argon2S2K, or
+	// (in the case of decryption) if Argon2 was used for encryption.
 	// If nil, default parameters are used.
 	// For more details on the choice of parameters, see https://tools.ietf.org/html/rfc9106#section-4.
 	Argon2Config *Argon2Config
@@ -53,6 +54,12 @@ type Argon2Config struct {
 	// Memory specifies the desired Argon2 memory usage in kibibytes.
 	// For example memory=64*1024 sets the memory cost to ~64 MB.
 	Memory uint32
+	// MaxMemory specifies the maximum Argon2 memory usage in kibibytes that is
+	// accepted when deriving a key. Since the memory usage is chosen by whoever
+	// wrote the packet, deriving a key from parameters that ask for more memory
+	// than this fails instead of allocating it.
+	// By default, the maximum memory usage is 2 GiB.
+	MaxMemory uint32
 }
 
 func (c *Config) Mode() Mode {
@@ -127,3 +134,10 @@ func (c *Argon2Config) EncodedMemory() uint8 {
 
 	return encodeMemory(memory, c.Parallelism())
 }
+
+func (c *Argon2Config) MaxMemoryUsage() uint32 {
+	if c == nil || c.MaxMemory == 0 {
+		return 2097152 // 2 GiB of RAM
+	}
+	return c.MaxMemory
+}

openpgp/s2k/s2k_test.go

@@ -322,3 +322,23 @@ func TestValidateArgon2Params(t *testing.T) {
 		}
 	}
 }
+
+func TestArgon2MaxMemory(t *testing.T) {
+	// Argon2 parameters asking for 2**31 kibibytes (i.e. 2 TiB) of memory.
+	buf := append([]byte{byte(Argon2S2K)}, make([]byte, Argon2SaltSize)...)
+	buf = append(buf, 3 /* passes */, 4 /* parallelism */, 31 /* memoryExp */)
+
+	params, err := ParseIntoParams(bytes.NewReader(buf))
+	if err != nil {
+		t.Fatalf("failed to parse the argon2 parameters: %s", err)
+	}
+
+	if _, err := params.Function(); err == nil {
+		t.Error("expected an error for parameters above the default maximum memory")
+	}
+
+	config := &Config{Argon2Config: &Argon2Config{MaxMemory: 1 << 31}}
+	if _, err := params.FunctionWithConfig(config); err != nil {
+		t.Errorf("expected no error for a configuration allowing the memory: %s", err)
+	}
+}

openpgp/v2/read.go

@@ -243,7 +243,7 @@ FindKey:
 		// Try the symmetric passphrase first
 		if len(symKeys) != 0 && passphrase != nil {
 			for _, s := range symKeys {
-				key, cipherFunc, err := s.Decrypt(passphrase)
+				key, cipherFunc, err := s.DecryptWithConfig(passphrase, config)
 				// In v4, on wrong passphrase, session key decryption is very likely to result in an invalid cipherFunc:
 				// only for < 5% of cases we will proceed to decrypt the data
 				if err == nil {