From afaf038311228a626c7622728285df06229518e4 Mon Sep 17 00:00:00 2001
From: Lukas Burkhalter <lukas.burkhalter@proton.ch>
Date: Mon, 11 Nov 2024 11:36:52 +0100
Subject: [PATCH] feat: Validate argon2 s2k params on read

---
 openpgp/packet/private_key.go             |  6 ++-
 openpgp/packet/symmetric_key_encrypted.go |  6 ++-
 openpgp/s2k/s2k.go                        | 50 +++++++++++++++++++----
 3 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/openpgp/packet/private_key.go b/openpgp/packet/private_key.go
index f04e6c6b8..6e5433d05 100644
--- a/openpgp/packet/private_key.go
+++ b/openpgp/packet/private_key.go
@@ -40,7 +40,7 @@ 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)
+	s2k           func(out, in []byte) error
 	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).
@@ -629,7 +629,9 @@ func (pk *PrivateKey) Decrypt(passphrase []byte) error {
 	}
 
 	key := make([]byte, pk.cipher.KeySize())
-	pk.s2k(key, passphrase)
+	if err := pk.s2k(key, passphrase); err != nil {
+		return err
+	}
 	if pk.s2kType == S2KAEAD {
 		key = pk.applyHKDF(key)
 	}
diff --git a/openpgp/packet/symmetric_key_encrypted.go b/openpgp/packet/symmetric_key_encrypted.go
index f843c35bf..c18a8b080 100644
--- a/openpgp/packet/symmetric_key_encrypted.go
+++ b/openpgp/packet/symmetric_key_encrypted.go
@@ -26,7 +26,7 @@ type SymmetricKeyEncrypted struct {
 	Version      int
 	CipherFunc   CipherFunction
 	Mode         AEADMode
-	s2k          func(out, in []byte)
+	s2k          func(out, in []byte) error
 	iv           []byte
 	encryptedKey []byte // Contains also the authentication tag for AEAD
 }
@@ -121,7 +121,9 @@ func (ske *SymmetricKeyEncrypted) parse(r io.Reader) error {
 // packet.
 func (ske *SymmetricKeyEncrypted) Decrypt(passphrase []byte) ([]byte, CipherFunction, error) {
 	key := make([]byte, ske.CipherFunc.KeySize())
-	ske.s2k(key, passphrase)
+	if err := ske.s2k(key, passphrase); err != nil {
+		return nil, ske.CipherFunc, err
+	}
 	if len(ske.encryptedKey) == 0 {
 		return key, ske.CipherFunc, nil
 	}
diff --git a/openpgp/s2k/s2k.go b/openpgp/s2k/s2k.go
index 925115807..28950a9a7 100644
--- a/openpgp/s2k/s2k.go
+++ b/openpgp/s2k/s2k.go
@@ -199,8 +199,8 @@ func Generate(rand io.Reader, c *Config) (*Params, error) {
 		}
 
 		params = &Params{
-			mode:      SaltedS2K,
-			hashId:    hashId,
+			mode:   SaltedS2K,
+			hashId: hashId,
 		}
 	} else { // Enforce IteratedSaltedS2K method otherwise
 		hashId, ok := algorithm.HashToHashId(c.hash())
@@ -226,7 +226,7 @@ func Generate(rand io.Reader, c *Config) (*Params, error) {
 // and returns a function which performs that transform. If the S2K is a special
 // GNU extension that indicates that the private key is missing, then the error
 // returned is errors.ErrDummyPrivateKey.
-func Parse(r io.Reader) (f func(out, in []byte), err error) {
+func Parse(r io.Reader) (f func(out, in []byte) error, err error) {
 	params, err := ParseIntoParams(r)
 	if err != nil {
 		return nil, err
@@ -319,7 +319,7 @@ func (params *Params) salt() []byte {
 	}
 }
 
-func (params *Params) Function() (f func(out, in []byte), err error) {
+func (params *Params) Function() (f func(out, in []byte) error, err error) {
 	if params.Dummy() {
 		return nil, errors.ErrDummyPrivateKey("dummy key found")
 	}
@@ -337,26 +337,33 @@ func (params *Params) Function() (f func(out, in []byte), err error) {
 
 	switch params.mode {
 	case SimpleS2K:
-		f := func(out, in []byte) {
+		f := func(out, in []byte) error {
 			Simple(out, hashObj.New(), in)
+			return nil
 		}
 
 		return f, nil
 	case SaltedS2K:
-		f := func(out, in []byte) {
+		f := func(out, in []byte) error {
 			Salted(out, hashObj.New(), in, params.salt())
+			return nil
 		}
 
 		return f, nil
 	case IteratedSaltedS2K:
-		f := func(out, in []byte) {
+		f := func(out, in []byte) error {
 			Iterated(out, hashObj.New(), in, params.salt(), decodeCount(params.countByte))
+			return nil
 		}
 
 		return f, nil
 	case Argon2S2K:
-		f := func(out, in []byte) {
+		f := func(out, in []byte) error {
+			if err := validateArgon2Params(params); err != nil {
+				return err
+			}
 			Argon2(out, in, params.salt(), params.passes, params.parallelism, params.memoryExp)
+			return nil
 		}
 		return f, nil
 	}
@@ -412,3 +419,30 @@ func Serialize(w io.Writer, key []byte, rand io.Reader, passphrase []byte, c *Co
 	f(key, passphrase)
 	return nil
 }
+
+// validateArgon2Params checks that the argon2 parameters are valid according to RFC9580.
+func validateArgon2Params(params *Params) error {
+	// The number of passes t and the degree of parallelism p MUST be non-zero.
+	if params.parallelism == 0 {
+		return errors.StructuralError("invalid argon2 params: parallelism is 0")
+	}
+	if params.passes == 0 {
+		return errors.StructuralError("invalid argon2 params: iterations is 0")
+	}
+
+	// The encoded memory size MUST be a value from 3+ceil(log2(p)) to 31.
+	p := params.parallelism
+	ceiledLogarithm := byte(0)
+	for p > 1 {
+		p /= 2
+		ceiledLogarithm += 1
+	}
+	if byte(1)<<ceiledLogarithm != params.parallelism {
+		ceiledLogarithm += 1
+	}
+	if params.memoryExp < 3+ceiledLogarithm || params.memoryExp > 31 {
+		return errors.StructuralError("invalid argon2 params: memory is out of bounds")
+	}
+
+	return nil
+}