Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIST Elliptic Curve JWK field element octet string padding #901

Closed
7ing opened this issue Jan 23, 2024 · 1 comment · Fixed by #903
Closed

NIST Elliptic Curve JWK field element octet string padding #901

7ing opened this issue Jan 23, 2024 · 1 comment · Fixed by #903
Milestone

Comments

@7ing
Copy link

7ing commented Jan 23, 2024

Describe the bug
According to RFC 7518, Section 6.2.1.2 and Section 6.2.1.3, The length of the X and Y string MUST be the full size of a coordinate for the curve specified in the crv parameter. For example, if the value of crv is P-521, the octet string must be 66 octets long.

However, the jjwt library 0.12 version violates this requirement. Instead, it removes the leading zero byte and result in 65 octets long.

To Reproduce
Here is a (failed) test case:

// NOT A CONTRIBUTION
package io.jsonwebtoken.impl.security

import io.jsonwebtoken.Jwts
import io.jsonwebtoken.impl.DefaultJwsHeader
import io.jsonwebtoken.io.Decoders
import io.jsonwebtoken.security.Jwks
import org.json.JSONObject
import org.junit.Test

import static org.junit.Assert.assertEquals

class RFC7518Section6Dot2Test {

 @Test
 void test() {
 def key = Jwts.SIG.ES512.keyPair().build().getPublic();
 def jwk = Jwks.builder().key(key).build();
 def header = new DefaultJwsHeader(new JSONObject(jwk).toMap())
 def x = Decoders.BASE64URL.decode((String) header.get("x"))
 def y = Decoders.BASE64URL.decode((String) header.get("y"))
 assertEquals(x.size(), 66)      ////////// fail if X has a leading zero byte
 assertEquals(y.size(), 66)      ////////// fail if Y has a leading zero byte
 }

}

Expected behavior
jjwt implementation shall NOT change the bytes for X, Y coordinates.

@lhazlewood
Copy link
Contributor

I could have sworn we had a test case for this, thank you for the issue! We'll incorporate your test (and probably a few more) and add any changes to the immediate next (0.12.4) release.

@lhazlewood lhazlewood added this to the 0.12.4 milestone Jan 23, 2024
@lhazlewood lhazlewood changed the title JWS Header (>=0.12) fails to comply with RFC 7518 (Section 6.2.1) about X, Y coordinate. NIST Elliptic Curve JWK x, y, and d byte array padding Jan 26, 2024
@lhazlewood lhazlewood changed the title NIST Elliptic Curve JWK x, y, and d byte array padding NIST Elliptic Curve JWK field element octet string padding Jan 26, 2024
lhazlewood added a commit that referenced this issue Jan 26, 2024
* Ensured NIST Elliptic Curve JWKs pre-pad their X, Y and D byte arrays with zero bytes before Base64URL-encoding if necessary per length requirements defined in:
- https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.2
- https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.3
- https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.2.1

Fixes #901.
lhazlewood added a commit that referenced this issue Jan 28, 2024
…arger than the identified HS* algorithm. This is allowed per https://datatracker.ietf.org/doc/html/rfc7518#section-3.2: "A key of the same size as the hash output ... _or larger_ MUST be used with this algorithm"

- Ensured that, when using the JwkBuilder, Secret JWK 'alg' values would automatically be set to 'HS256', 'HS384', or 'HS512' if the specified Java SecretKey algorithm name equals a JCA standard name (HmacSHA256, HmacSHA384, etc) or JCA standard HMAC-SHA OID.

Fixes #901.
lhazlewood added a commit that referenced this issue Jan 28, 2024
- Ensured Secret JWK 'k' byte arrays for HMAC-SHA algorithms can be larger than the identified HS* algorithm. This is allowed per https://datatracker.ietf.org/doc/html/rfc7518#section-3.2: "A key of the same size as the hash output ... _or larger_ MUST be used with this algorithm"

- Ensured that, when using the JwkBuilder, Secret JWK 'alg' values would automatically be set to 'HS256', 'HS384', or 'HS512' if the specified Java SecretKey algorithm name equals a JCA standard name (HmacSHA256, HmacSHA384, etc) or JCA standard HMAC-SHA OID.

-  Updated CHANGELOG.md accordingly.

Fixes #901.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants