Repository: lestrrat-go/jwx
Pull request #1142 (2024-06-18 16:27:28)

Add support for RSA-OAEP-384 and RSA-OAEP-512
This pull request would add support for the `RSA-OAEP-384` and `RSA-OAEP-512` key encryption algorithms. I think this is just a minor change as the base algorithms `RSA-OAEP` and `RSA-OAEP-256` are already implemented. The only difference these new algorithms have is that they use a different hashing algorithm `SHA384` and `SHA512` respectively instead of `SHA1` or `SHA256`.

The hashing algorithms `SHA384`and `SHA512` are already used in different algorithms, so these are not new. The only thing I wasn't sure about it how the following case should be handled:
[permalink](https://github.com/Hannes-Kunnen/jwx/blob/458bea5fa7a67cc251703d04654eb93f8df2bb9f/jwe/internal/keyenc/keyenc.go#L458-L477)
```go
func (e RSAOAEPEncrypt) EncryptKey(cek []byte) (keygen.ByteSource, error) {
	var hash hash.Hash
	switch e.alg {
	case jwa.RSA_OAEP:
		hash = sha1.New()
	case jwa.RSA_OAEP_256:
		hash = sha256.New()
	case jwa.RSA_OAEP_384:
		hash = sha512.New384()
	case jwa.RSA_OAEP_512:
		hash = sha512.New()
	default:
		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256/RSA_OAEP_384/RSA_OAEP_512 required`)
	}
	encrypted, err := rsa.EncryptOAEP(hash, rand.Reader, e.pubkey, cek, []byte{})
	if err != nil {
		return nil, fmt.Errorf(`failed to OAEP encrypt: %w`, err)
	}
	return keygen.ByteKey(encrypted), nil
}
```
In the original code the `'hash'.New()` method was used, so I continued this usage, however when searching where the `SHA384` and `SHA512` algorithms were used already I found an other method which would result in this diff:
```diff
func (e RSAOAEPEncrypt) EncryptKey(cek []byte) (keygen.ByteSource, error) {
-	var hash hash.Hash
+	var hash crypto.Hash
	switch e.alg {
	case jwa.RSA_OAEP:
-		hash = sha1.New()
+		hash = crypto.SHA1
	case jwa.RSA_OAEP_256:
-		hash = sha256.New()
+		hash = crypto.SHA256
	case jwa.RSA_OAEP_384:
-		hash = sha512.New384()
+		hash = crypto.SHA384
	case jwa.RSA_OAEP_512:
-		hash = sha512.New()
+		hash = crypto.SHA512
	default:
		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256/RSA_OAEP_384/RSA_OAEP_512 required`)
	}
-	encrypted, err := rsa.EncryptOAEP(hash, rand.Reader, e.pubkey, cek, []byte{})
+	encrypted, err := rsa.EncryptOAEP(hash.New(), rand.Reader, e.pubkey, cek, []byte{})
	if err != nil {
		return nil, fmt.Errorf(`failed to OAEP encrypt: %w`, err)
	}
	return keygen.ByteKey(encrypted), nil
}
```
I am not sure which one would be preferred (I assumed the method that was already used).

If I missed something or there is a reason these algorithms weren't implemented I would love to hear it.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/lestrrat-go/jwx/pull/1142?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) Report
Attention: Patch coverage is `37.50000%` with `10 lines` in your changes missing coverage. Please review.
> Project coverage is 75.03%. Comparing base [(`0a15b4d`)](https://app.codecov.io/gh/lestrrat-go/jwx/commit/0a15b4d9a180b06c6924c4a03b39ae70d05514f4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) to head [(`458bea5`)](https://app.codecov.io/gh/lestrrat-go/jwx/commit/458bea5fa7a67cc251703d04654eb93f8df2bb9f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go).
> Report is 84 commits behind head on develop/v2.

| [Files](https://app.codecov.io/gh/lestrrat-go/jwx/pull/1142?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) | Patch % | Lines |
|---|---|---|
| [jwe/internal/keyenc/keyenc.go](https://app.codecov.io/gh/lestrrat-go/jwx/pull/1142?src=pr&el=tree&filepath=jwe%2Finternal%2Fkeyenc%2Fkeyenc.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go#diff-andlL2ludGVybmFsL2tleWVuYy9rZXllbmMuZ28=) | 16.66% | [10 Missing :warning: ](https://app.codecov.io/gh/lestrrat-go/jwx/pull/1142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) |

<details><summary>Additional details and impacted files</summary>


```diff
@@              Coverage Diff               @@
##           develop/v2    #1142      +/-   ##
==============================================
+ Coverage       72.00%   75.03%   +3.03%     
==============================================
  Files              93       95       +2     
  Lines           13795    11965    -1830     
==============================================
- Hits             9933     8978     -955     
+ Misses           3044     2168     -876     
- Partials          818      819       +1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/lestrrat-go/jwx/pull/1142?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go).

lestrrat: LGTM! I'll merge, but please note that I won't be making a release anytime soon. I usually like to wait and see if anything breaks before making releases.

Thanks!

P.S.

> I am not sure which one would be preferred (I assumed the method that was already used).

It doesn't really matter. They end up calling the same thing. The rest is stylistic, but for this particular part I didn't have a preference :)

Patch:
diff --git a/jwa/key_encryption_gen.go b/jwa/key_encryption_gen.go
index 49ed1f67..db47f79f 100644
--- a/jwa/key_encryption_gen.go
+++ b/jwa/key_encryption_gen.go
@@ -30,6 +30,8 @@ const (
 	RSA1_5             KeyEncryptionAlgorithm = "RSA1_5"             // RSA-PKCS1v1.5
 	RSA_OAEP           KeyEncryptionAlgorithm = "RSA-OAEP"           // RSA-OAEP-SHA1
 	RSA_OAEP_256       KeyEncryptionAlgorithm = "RSA-OAEP-256"       // RSA-OAEP-SHA256
+	RSA_OAEP_384       KeyEncryptionAlgorithm = "RSA-OAEP-384"       // RSA-OAEP-SHA384
+	RSA_OAEP_512       KeyEncryptionAlgorithm = "RSA-OAEP-512"       // RSA-OAEP-SHA512
 )
 
 var muKeyEncryptionAlgorithms sync.RWMutex
@@ -57,6 +59,8 @@ func init() {
 	allKeyEncryptionAlgorithms[RSA1_5] = struct{}{}
 	allKeyEncryptionAlgorithms[RSA_OAEP] = struct{}{}
 	allKeyEncryptionAlgorithms[RSA_OAEP_256] = struct{}{}
+	allKeyEncryptionAlgorithms[RSA_OAEP_384] = struct{}{}
+	allKeyEncryptionAlgorithms[RSA_OAEP_512] = struct{}{}
 	rebuildKeyEncryptionAlgorithm()
 }
 
diff --git a/jwe/decrypt.go b/jwe/decrypt.go
index 8729e4e5..0b26a3c8 100644
--- a/jwe/decrypt.go
+++ b/jwe/decrypt.go
@@ -278,7 +278,7 @@ func (d *decrypter) BuildKeyDecrypter() (keyenc.Decrypter, error) {
 		}
 
 		return keyenc.NewRSAPKCS15Decrypt(alg, &privkey, cipher.KeySize()/2), nil
-	case jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+	case jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 		var privkey rsa.PrivateKey
 		if err := keyconv.RSAPrivateKey(&privkey, d.privkey); err != nil {
 			return nil, fmt.Errorf(`*rsa.PrivateKey is required as the key to build %s key decrypter: %w`, alg, err)
diff --git a/jwe/internal/keyenc/keyenc.go b/jwe/internal/keyenc/keyenc.go
index 2f57a316..52acb0fa 100644
--- a/jwe/internal/keyenc/keyenc.go
+++ b/jwe/internal/keyenc/keyenc.go
@@ -390,7 +390,7 @@ func (kw ECDHESDecrypt) Decrypt(enckey []byte) ([]byte, error) {
 // NewRSAOAEPEncrypt creates a new key encrypter using RSA OAEP
 func NewRSAOAEPEncrypt(alg jwa.KeyEncryptionAlgorithm, pubkey *rsa.PublicKey) (*RSAOAEPEncrypt, error) {
 	switch alg {
-	case jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+	case jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 	default:
 		return nil, fmt.Errorf("invalid RSA OAEP encrypt algorithm (%s)", alg)
 	}
@@ -462,8 +462,12 @@ func (e RSAOAEPEncrypt) EncryptKey(cek []byte) (keygen.ByteSource, error) {
 		hash = sha1.New()
 	case jwa.RSA_OAEP_256:
 		hash = sha256.New()
+	case jwa.RSA_OAEP_384:
+		hash = sha512.New384()
+	case jwa.RSA_OAEP_512:
+		hash = sha512.New()
 	default:
-		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256 required`)
+		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256/RSA_OAEP_384/RSA_OAEP_512 required`)
 	}
 	encrypted, err := rsa.EncryptOAEP(hash, rand.Reader, e.pubkey, cek, []byte{})
 	if err != nil {
@@ -536,7 +540,7 @@ func (d RSAPKCS15Decrypt) Decrypt(enckey []byte) ([]byte, error) {
 // NewRSAOAEPDecrypt creates a new key decrypter using RSA OAEP
 func NewRSAOAEPDecrypt(alg jwa.KeyEncryptionAlgorithm, privkey *rsa.PrivateKey) (*RSAOAEPDecrypt, error) {
 	switch alg {
-	case jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+	case jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 	default:
 		return nil, fmt.Errorf("invalid RSA OAEP decrypt algorithm (%s)", alg)
 	}
@@ -560,8 +564,12 @@ func (d RSAOAEPDecrypt) Decrypt(enckey []byte) ([]byte, error) {
 		hash = sha1.New()
 	case jwa.RSA_OAEP_256:
 		hash = sha256.New()
+	case jwa.RSA_OAEP_384:
+		hash = sha512.New384()
+	case jwa.RSA_OAEP_512:
+		hash = sha512.New()
 	default:
-		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256 required`)
+		return nil, fmt.Errorf(`failed to generate key encrypter for RSA-OAEP: RSA_OAEP/RSA_OAEP_256/RSA_OAEP_384/RSA_OAEP_512 required`)
 	}
 	return rsa.DecryptOAEP(hash, rand.Reader, d.privkey, enckey, []byte{})
 }
diff --git a/jwe/jwe.go b/jwe/jwe.go
index 1e6ecda5..03e20d03 100644
--- a/jwe/jwe.go
+++ b/jwe/jwe.go
@@ -118,7 +118,7 @@ func (b *recipientBuilder) Build(cek []byte, calg jwa.ContentEncryptionAlgorithm
 				return nil, nil, fmt.Errorf(`failed to create RSA PKCS encrypter: %w`, err)
 			}
 			enc = v
-		case jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+		case jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 			var pubkey rsa.PublicKey
 			if err := keyconv.RSAPublicKey(&pubkey, rawKey); err != nil {
 				return nil, nil, fmt.Errorf(`failed to generate public key from key (%T): %w`, rawKey, err)
diff --git a/tools/cmd/genjwa/main.go b/tools/cmd/genjwa/main.go
index b7f8a7f6..18169f00 100644
--- a/tools/cmd/genjwa/main.go
+++ b/tools/cmd/genjwa/main.go
@@ -249,6 +249,16 @@ func _main() error {
 					value:   "RSA-OAEP-256",
 					comment: `RSA-OAEP-SHA256`,
 				},
+				{
+					name:    `RSA_OAEP_384`,
+					value:   "RSA-OAEP-384",
+					comment: `RSA-OAEP-SHA384`,
+				},
+				{
+					name:    `RSA_OAEP_512`,
+					value:   "RSA-OAEP-512",
+					comment: `RSA-OAEP-SHA512`,
+				},
 				{
 					name:    `A128KW`,
 					value:   "A128KW",


Test patch:
diff --git a/internal/jwxtest/jwxtest.go b/internal/jwxtest/jwxtest.go
index cd9e8af2..9b5e2514 100644
--- a/internal/jwxtest/jwxtest.go
+++ b/internal/jwxtest/jwxtest.go
@@ -286,7 +286,7 @@ func EncryptJweFile(ctx context.Context, payload []byte, keyalg jwa.KeyEncryptio
 	var keyif interface{}
 
 	switch keyalg {
-	case jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+	case jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 		var rawkey rsa.PrivateKey
 		if err := key.Raw(&rawkey); err != nil {
 			return "", nil, fmt.Errorf(`failed to obtain raw key: %w`, err)
diff --git a/jwa/key_encryption_gen_test.go b/jwa/key_encryption_gen_test.go
index 26250b58..0db993b8 100644
--- a/jwa/key_encryption_gen_test.go
+++ b/jwa/key_encryption_gen_test.go
@@ -623,6 +623,78 @@ func TestKeyEncryptionAlgorithm(t *testing.T) {
 			return
 		}
 	})
+	t.Run(`accept jwa constant RSA_OAEP_384`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept(jwa.RSA_OAEP_384), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_384, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`accept the string RSA-OAEP-384`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept("RSA-OAEP-384"), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_384, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`accept fmt.Stringer for RSA-OAEP-384`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept(stringer{src: "RSA-OAEP-384"}), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_384, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`stringification for RSA-OAEP-384`, func(t *testing.T) {
+		t.Parallel()
+		if !assert.Equal(t, "RSA-OAEP-384", jwa.RSA_OAEP_384.String(), `stringified value matches`) {
+			return
+		}
+	})
+	t.Run(`accept jwa constant RSA_OAEP_512`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept(jwa.RSA_OAEP_512), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_512, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`accept the string RSA-OAEP-512`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept("RSA-OAEP-512"), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_512, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`accept fmt.Stringer for RSA-OAEP-512`, func(t *testing.T) {
+		t.Parallel()
+		var dst jwa.KeyEncryptionAlgorithm
+		if !assert.NoError(t, dst.Accept(stringer{src: "RSA-OAEP-512"}), `accept is successful`) {
+			return
+		}
+		if !assert.Equal(t, jwa.RSA_OAEP_512, dst, `accepted value should be equal to constant`) {
+			return
+		}
+	})
+	t.Run(`stringification for RSA-OAEP-512`, func(t *testing.T) {
+		t.Parallel()
+		if !assert.Equal(t, "RSA-OAEP-512", jwa.RSA_OAEP_512.String(), `stringified value matches`) {
+			return
+		}
+	})
 	t.Run(`bail out on random integer value`, func(t *testing.T) {
 		t.Parallel()
 		var dst jwa.KeyEncryptionAlgorithm
@@ -690,6 +762,12 @@ func TestKeyEncryptionAlgorithm(t *testing.T) {
 		t.Run(`RSA_OAEP_256`, func(t *testing.T) {
 			assert.False(t, jwa.RSA_OAEP_256.IsSymmetric(), `jwa.RSA_OAEP_256 should NOT be symmetric`)
 		})
+		t.Run(`RSA_OAEP_384`, func(t *testing.T) {
+			assert.False(t, jwa.RSA_OAEP_384.IsSymmetric(), `jwa.RSA_OAEP_384 should NOT be symmetric`)
+		})
+		t.Run(`RSA_OAEP_512`, func(t *testing.T) {
+			assert.False(t, jwa.RSA_OAEP_512.IsSymmetric(), `jwa.RSA_OAEP_512 should NOT be symmetric`)
+		})
 	})
 	t.Run(`check list of elements`, func(t *testing.T) {
 		t.Parallel()
@@ -711,6 +789,8 @@ func TestKeyEncryptionAlgorithm(t *testing.T) {
 			jwa.RSA1_5:             {},
 			jwa.RSA_OAEP:           {},
 			jwa.RSA_OAEP_256:       {},
+			jwa.RSA_OAEP_384:       {},
+			jwa.RSA_OAEP_512:       {},
 		}
 		for _, v := range jwa.KeyEncryptionAlgorithms() {
 			if _, ok := expected[v]; !assert.True(t, ok, `%s should be in the expected list`, v) {
diff --git a/jwx_test.go b/jwx_test.go
index b21e1518..7145ee45 100644
--- a/jwx_test.go
+++ b/jwx_test.go
@@ -188,7 +188,7 @@ func TestJoseCompatibility(t *testing.T) {
 
 		var tests []interopTest
 
-		for _, keyenc := range []jwa.KeyEncryptionAlgorithm{jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256} {
+		for _, keyenc := range []jwa.KeyEncryptionAlgorithm{jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512} {
 			if !set.Has(keyenc.String()) {
 				t.Logf("jose does not support key encryption algorithm %q: skipping", keyenc)
 				continue
@@ -297,7 +297,7 @@ func joseInteropTest(ctx context.Context, spec interopTest, t *testing.T) {
 
 	t.Run("Parse JWK via jwx", func(t *testing.T) {
 		switch spec.alg {
-		case jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256:
+		case jwa.RSA1_5, jwa.RSA_OAEP, jwa.RSA_OAEP_256, jwa.RSA_OAEP_384, jwa.RSA_OAEP_512:
 			var rawkey rsa.PrivateKey
 			if !assert.NoError(t, jwxJwk.Raw(&rawkey), `jwk.Raw should succeed`) {
 				return