From 8db30c7de94cc93d538cd4178b7fd920651e4590 Mon Sep 17 00:00:00 2001 From: Spencer Witt Date: Tue, 27 Feb 2024 16:29:02 -0600 Subject: [PATCH 1/4] test for invalid signature using EC521 --- src/test/java/io/fusionauth/jwt/JWTTest.java | 29 +++++++- .../io/fusionauth/jwt/ec/ECSignerTest.java | 67 +++++++++++++++---- .../io/fusionauth/security/KeyUtilsTests.java | 7 +- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/test/java/io/fusionauth/jwt/JWTTest.java b/src/test/java/io/fusionauth/jwt/JWTTest.java index 37bc2b2..ce89374 100644 --- a/src/test/java/io/fusionauth/jwt/JWTTest.java +++ b/src/test/java/io/fusionauth/jwt/JWTTest.java @@ -965,7 +965,7 @@ public void test_openssl_keys_p_256() { assertEquals(actual.subject, jwt.subject); } - @Test + @Test(invocationCount = 1000) public void test_openssl_keys_p_521() { JWT jwt = new JWT() .setSubject("1234567890") @@ -996,6 +996,33 @@ public void test_openssl_keys_p_521() { assertEquals(actual.subject, jwt.subject); } + @Test(invocationCount = 1000) + public void test_openssl_keys_p_521_failed_validation() { + JWT jwt = new JWT() + .setSubject("1234567890") + .addClaim("name", "John Doe 208") + .addClaim("admin", true) + .addClaim("iat", 1516239022); + + // PKCS#8 PEM, needs no encapsulation + Signer signer = ECSigner.newSHA512Signer("-----BEGIN PRIVATE KEY-----\n" + + "MF8CAQAwEAYHKoZIzj0CAQYFK4EEACMESDBGAgEBBEHNkRW+AZ87Hobcc0BIW1YV\n" + + "Ia5uQ0QVj8kkAN7sgTzKmawnf82FqFnHzqyhshHfwx9eIv723E3/XrA+3nw+8/l9\n" + + "ew==\n" + + "-----END PRIVATE KEY-----"); + String encodedJWT = JWT.getEncoder().encode(jwt, signer, header + -> header.set("kid", "S-CdQFQmkHB1rFfzQS4kxhzKk8xtZPAECItUvB0plIM")); + + Verifier verifier = ECVerifier.newVerifier("-----BEGIN PUBLIC KEY-----\n" + + "MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQBa5VzP87t5eGBqoZFLs9xQiodlD7E\n" + + "0xCzD7SrjshhOfOYei9kb7Yt5bvXShTfji16r1tgpaVzc5ffG693O4RVsUsBV7nl\n" + + "mCRG7g5x9FD2VIiSwnrDHyVf0bYNMeBM4vrt245VNT0E1xkJWhUB2JIl4dGvv0qE\n" + + "dYamwqrHFpoN7QW530Y=\n" + + "-----END PUBLIC KEY-----"); + JWT actual = JWT.getDecoder().decode(encodedJWT, verifier); + assertEquals(actual.subject, jwt.subject); + } + @Test public void test_zonedDateTime() { ZonedDateTime expiration = ZonedDateTime.now(ZoneOffset.UTC).plusMinutes(60).truncatedTo(ChronoUnit.SECONDS); diff --git a/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java b/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java index 10686a9..425c093 100644 --- a/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java +++ b/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java @@ -16,8 +16,15 @@ package io.fusionauth.jwt.ec; +import io.fusionauth.jwks.JSONWebKeyBuilder; import io.fusionauth.jwt.BaseJWTTest; +import io.fusionauth.jwt.InvalidJWTSignatureException; import io.fusionauth.jwt.InvalidKeyTypeException; +import io.fusionauth.jwt.JWTUtils; +import io.fusionauth.jwt.Signer; +import io.fusionauth.jwt.Verifier; +import io.fusionauth.jwt.domain.JWT; +import io.fusionauth.pem.PEMEncoder; import io.fusionauth.pem.domain.PEM; import io.fusionauth.security.BCFIPSCryptoProvider; import org.testng.annotations.Test; @@ -27,6 +34,7 @@ import java.nio.file.Paths; import java.security.KeyPair; import java.security.KeyPairGenerator; +import java.security.SecureRandom; import java.security.Signature; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; @@ -59,28 +67,61 @@ public void test_invalidKey() { } } - @Test + @Test(invocationCount = 1000) public void round_trip_raw1() throws Exception { // Generate a key-pair and sign and verify a message KeyPairGenerator g = KeyPairGenerator.getInstance("EC"); - ECGenParameterSpec parameterSpec = new ECGenParameterSpec("secp256r1"); + ECGenParameterSpec parameterSpec = new ECGenParameterSpec("secp384r1"); g.initialize(parameterSpec); KeyPair pair = g.generateKeyPair(); + System.out.println(new PEMEncoder().encode(pair.getPublic())); + System.out.println(new PEMEncoder().encode(pair.getPrivate())); + System.out.println(JWTUtils.generateJWS_kid_S256(new JSONWebKeyBuilder().build(pair.getPublic()))); // Instance of signature class with SHA256withECDSA algorithm - Signature signature = Signature.getInstance("SHA256withECDSA"); + Signature signature = Signature.getInstance("SHA384withECDSA"); signature.initSign(pair.getPrivate()); + Signer signer = ECSigner.newSHA384Signer(pair.getPrivate()); + Verifier verifier = ECVerifier.newVerifier(pair.getPublic()); + + for (int i = 0; i < 250; i++) { + JWT jwt = new JWT() + .setSubject("1234567890") + .addClaim("name", "John Doe " + i) + .addClaim("admin", true) + .addClaim("iat", 1516239022); + + String encodedJWT = JWT.getEncoder().encode(jwt, signer, header + -> header.set("kid", JWTUtils.generateJWS_kid_S256(new JSONWebKeyBuilder().build(pair.getPublic())))); + + JWT actual = JWT.getDecoder().decode(encodedJWT, verifier); + assertEquals(actual.subject, jwt.subject); + assertEquals(actual.getString("name"), jwt.getString("name")); +// try { +// System.out.println("Good " + encodedJWT); +// } catch (InvalidJWTSignatureException e) { +// System.out.println("Bad " + encodedJWT); +// } + } - // Sign a message - String message = "text ecdsa with sha256"; - signature.update((message).getBytes(StandardCharsets.UTF_8)); - byte[] signatureBytes = signature.sign(); - - // Validation - Signature verifier = Signature.getInstance("SHA256withECDSA"); - verifier.initVerify(pair.getPublic()); - verifier.update(message.getBytes(StandardCharsets.UTF_8)); - assertTrue(verifier.verify(signatureBytes)); +// for (int i = 0; i < 250; i++) { +// // Sign a message +// byte[] bytes = new byte[i+1]; +// new SecureRandom().nextBytes(bytes); +// signature.update(bytes); +// byte[] signatureBytes = signature.sign(); +// +// // Validation +// Signature verifier = Signature.getInstance("SHA512withECDSA"); +// verifier.initVerify(pair.getPublic()); +// verifier.update(bytes); +// boolean verified = verifier.verify(signatureBytes); +// if (!verified) { +// System.out.println("Public: " + new String(pair.getPublic().getEncoded(), StandardCharsets.UTF_8)); +// System.out.println("Private: " + new String(pair.getPrivate().getEncoded(), StandardCharsets.UTF_8)); +// } +// assertTrue(verified); +// } } @Test diff --git a/src/test/java/io/fusionauth/security/KeyUtilsTests.java b/src/test/java/io/fusionauth/security/KeyUtilsTests.java index ed3dd6e..194e589 100644 --- a/src/test/java/io/fusionauth/security/KeyUtilsTests.java +++ b/src/test/java/io/fusionauth/security/KeyUtilsTests.java @@ -16,6 +16,7 @@ package io.fusionauth.security; +import io.fusionauth.pem.PEMDecoder; import io.fusionauth.pem.domain.PEM; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -34,8 +35,8 @@ public class KeyUtilsTests { @DataProvider(name = "ecKeyLengths") public Object[][] ecKeyLengths() { return new Object[][]{ - {"EC", 256, 256, 256}, - {"EC", 384, 384, 384}, +// {"EC", 256, 256, 256}, +// {"EC", 384, 384, 384}, {"EC", 521, 521, 521} }; } @@ -63,7 +64,7 @@ public void problematicKey() { // failures where the key is not an exact size and we have to figure out which key size it should be reported as. // - For testing locally, you can ramp up this invocation count to 100k or something like that to prove that we have // consistency over time. - @Test(dataProvider = "ecKeyLengths", invocationCount = 500) + @Test(dataProvider = "ecKeyLengths", invocationCount = 20000) public void ec_getKeyLength(String algorithm, int keySize, int privateKeySize, int publicKeySize) throws Exception { KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(algorithm); keyPairGenerator.initialize(keySize); From af79d25b40d6868aa68794402570170ab0210554 Mon Sep 17 00:00:00 2001 From: Daniel DeGroff Date: Tue, 27 Feb 2024 18:04:27 -0700 Subject: [PATCH 2/4] Fix padding when extracting the r and s components of the EC signature from a DER encoded sequence. --- fusionauth-jwt.iml | 6 +- .../io/fusionauth/jwt/ec/ECDSASignature.java | 17 +++-- src/test/java/io/fusionauth/jwt/JWTTest.java | 47 ++++--------- .../io/fusionauth/jwt/ec/ECSignerTest.java | 67 ++++--------------- .../io/fusionauth/security/KeyUtilsTests.java | 7 +- 5 files changed, 45 insertions(+), 99 deletions(-) diff --git a/fusionauth-jwt.iml b/fusionauth-jwt.iml index aa5199a..1785c4c 100644 --- a/fusionauth-jwt.iml +++ b/fusionauth-jwt.iml @@ -20,7 +20,7 @@ - + @@ -31,7 +31,7 @@ - + @@ -42,7 +42,7 @@ - + diff --git a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java index 6e96a0e..1b31f9c 100644 --- a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java +++ b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2019, FusionAuth, All Rights Reserved + * Copyright (c) 2018-2024, FusionAuth, All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -86,10 +86,17 @@ public byte[] derDecode(Algorithm algorithm) throws IOException { } int len = result.length / 2; - //noinspection ManualMinMaxCalculation - System.arraycopy(r, r.length > len ? 1 : 0, result, r.length < len ? 1 : 0, r.length > len ? len : r.length); - //noinspection ManualMinMaxCalculation - System.arraycopy(s, s.length > len ? 1 : 0, result, s.length < len ? (len + 1) : len, s.length > len ? len : s.length); + + int rSrcPos = r.length > len ? (r.length - len) : 0; + int rDstPos = Math.max(0, len - r.length); + int rLength = Math.min(r.length, len); + System.arraycopy(r, rSrcPos, result, rDstPos, rLength); + + int sSrcPos = s.length > len ? (s.length - len) : 0; + int sDstPos = s.length < len ? (len + (len - s.length)) : len; + int sLength = Math.min(s.length, len); + System.arraycopy(s, sSrcPos, result, sDstPos, sLength); + return result; } diff --git a/src/test/java/io/fusionauth/jwt/JWTTest.java b/src/test/java/io/fusionauth/jwt/JWTTest.java index ce89374..a1726df 100644 --- a/src/test/java/io/fusionauth/jwt/JWTTest.java +++ b/src/test/java/io/fusionauth/jwt/JWTTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2022, FusionAuth, All Rights Reserved + * Copyright (c) 2016-2024, FusionAuth, All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -742,8 +742,10 @@ public void test_expiration_clockSkew() { assertEquals(actual.subject, "1234567890"); } - @Test + @Test(invocationCount = 2_000) public void test_external_ec_521() { + // The purpose of the large invocation is to ensure we are consistently extracting the r and s components of the DER encoded signature. + // - Performing this test 1-3k times is generally sufficient to produce at least 1-3 errors prior to fixing the bug. JWT jwt = new JWT() .setSubject("1234567890") .addClaim("name", "John Doe") @@ -778,8 +780,10 @@ public void test_external_ec_521() { assertEquals(actual.subject, jwt.subject); } - @Test + @Test(invocationCount = 2_000) public void test_external_ec_p256() { + // The purpose of the large invocation is to ensure we are consistently extracting the r and s components of the DER encoded signature. + // - Performing this test 1-3k times is generally sufficient to produce at least 1-3 errors prior to fixing the bug. JWT jwt = new JWT() .setSubject("1234567890") .addClaim("name", "John Doe") @@ -806,8 +810,10 @@ public void test_external_ec_p256() { assertEquals(actual.subject, jwt.subject); } - @Test + @Test(invocationCount = 2_000) public void test_external_ec_p384() { + // The purpose of the large invocation is to ensure we are consistently extracting the r and s components of the DER encoded signature. + // - Performing this test 1-3k times is generally sufficient to produce at least 1-3 errors prior to fixing the bug. JWT jwt = new JWT() .setSubject("1234567890") .addClaim("name", "John Doe") @@ -887,7 +893,7 @@ public void test_multipleSignersAndVerifiers() throws Exception { verifiers.put("verifier2", verifier2); verifiers.put("verifier3", verifier3); - // decode all of the encoded JWTs and ensure they come out the same. + // decode all the encoded JWTs and ensure they come out the same. JWT jwt1 = JWT.getDecoder().decode(encodedJWT1, verifiers); JWT jwt2 = JWT.getDecoder().decode(encodedJWT2, verifiers); JWT jwt3 = JWT.getDecoder().decode(encodedJWT3, verifiers); @@ -965,8 +971,10 @@ public void test_openssl_keys_p_256() { assertEquals(actual.subject, jwt.subject); } - @Test(invocationCount = 1000) + @Test(invocationCount = 2_000) public void test_openssl_keys_p_521() { + // The purpose of the large invocation is to ensure we are consistently extracting the r and s components of the DER encoded signature. + // - Performing this test 1-3k times is generally sufficient to produce at least 1-3 errors prior to fixing the bug. JWT jwt = new JWT() .setSubject("1234567890") .addClaim("name", "John Doe") @@ -996,33 +1004,6 @@ public void test_openssl_keys_p_521() { assertEquals(actual.subject, jwt.subject); } - @Test(invocationCount = 1000) - public void test_openssl_keys_p_521_failed_validation() { - JWT jwt = new JWT() - .setSubject("1234567890") - .addClaim("name", "John Doe 208") - .addClaim("admin", true) - .addClaim("iat", 1516239022); - - // PKCS#8 PEM, needs no encapsulation - Signer signer = ECSigner.newSHA512Signer("-----BEGIN PRIVATE KEY-----\n" + - "MF8CAQAwEAYHKoZIzj0CAQYFK4EEACMESDBGAgEBBEHNkRW+AZ87Hobcc0BIW1YV\n" + - "Ia5uQ0QVj8kkAN7sgTzKmawnf82FqFnHzqyhshHfwx9eIv723E3/XrA+3nw+8/l9\n" + - "ew==\n" + - "-----END PRIVATE KEY-----"); - String encodedJWT = JWT.getEncoder().encode(jwt, signer, header - -> header.set("kid", "S-CdQFQmkHB1rFfzQS4kxhzKk8xtZPAECItUvB0plIM")); - - Verifier verifier = ECVerifier.newVerifier("-----BEGIN PUBLIC KEY-----\n" + - "MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQBa5VzP87t5eGBqoZFLs9xQiodlD7E\n" + - "0xCzD7SrjshhOfOYei9kb7Yt5bvXShTfji16r1tgpaVzc5ffG693O4RVsUsBV7nl\n" + - "mCRG7g5x9FD2VIiSwnrDHyVf0bYNMeBM4vrt245VNT0E1xkJWhUB2JIl4dGvv0qE\n" + - "dYamwqrHFpoN7QW530Y=\n" + - "-----END PUBLIC KEY-----"); - JWT actual = JWT.getDecoder().decode(encodedJWT, verifier); - assertEquals(actual.subject, jwt.subject); - } - @Test public void test_zonedDateTime() { ZonedDateTime expiration = ZonedDateTime.now(ZoneOffset.UTC).plusMinutes(60).truncatedTo(ChronoUnit.SECONDS); diff --git a/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java b/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java index 425c093..10686a9 100644 --- a/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java +++ b/src/test/java/io/fusionauth/jwt/ec/ECSignerTest.java @@ -16,15 +16,8 @@ package io.fusionauth.jwt.ec; -import io.fusionauth.jwks.JSONWebKeyBuilder; import io.fusionauth.jwt.BaseJWTTest; -import io.fusionauth.jwt.InvalidJWTSignatureException; import io.fusionauth.jwt.InvalidKeyTypeException; -import io.fusionauth.jwt.JWTUtils; -import io.fusionauth.jwt.Signer; -import io.fusionauth.jwt.Verifier; -import io.fusionauth.jwt.domain.JWT; -import io.fusionauth.pem.PEMEncoder; import io.fusionauth.pem.domain.PEM; import io.fusionauth.security.BCFIPSCryptoProvider; import org.testng.annotations.Test; @@ -34,7 +27,6 @@ import java.nio.file.Paths; import java.security.KeyPair; import java.security.KeyPairGenerator; -import java.security.SecureRandom; import java.security.Signature; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; @@ -67,61 +59,28 @@ public void test_invalidKey() { } } - @Test(invocationCount = 1000) + @Test public void round_trip_raw1() throws Exception { // Generate a key-pair and sign and verify a message KeyPairGenerator g = KeyPairGenerator.getInstance("EC"); - ECGenParameterSpec parameterSpec = new ECGenParameterSpec("secp384r1"); + ECGenParameterSpec parameterSpec = new ECGenParameterSpec("secp256r1"); g.initialize(parameterSpec); KeyPair pair = g.generateKeyPair(); - System.out.println(new PEMEncoder().encode(pair.getPublic())); - System.out.println(new PEMEncoder().encode(pair.getPrivate())); - System.out.println(JWTUtils.generateJWS_kid_S256(new JSONWebKeyBuilder().build(pair.getPublic()))); // Instance of signature class with SHA256withECDSA algorithm - Signature signature = Signature.getInstance("SHA384withECDSA"); + Signature signature = Signature.getInstance("SHA256withECDSA"); signature.initSign(pair.getPrivate()); - Signer signer = ECSigner.newSHA384Signer(pair.getPrivate()); - Verifier verifier = ECVerifier.newVerifier(pair.getPublic()); - - for (int i = 0; i < 250; i++) { - JWT jwt = new JWT() - .setSubject("1234567890") - .addClaim("name", "John Doe " + i) - .addClaim("admin", true) - .addClaim("iat", 1516239022); - - String encodedJWT = JWT.getEncoder().encode(jwt, signer, header - -> header.set("kid", JWTUtils.generateJWS_kid_S256(new JSONWebKeyBuilder().build(pair.getPublic())))); - - JWT actual = JWT.getDecoder().decode(encodedJWT, verifier); - assertEquals(actual.subject, jwt.subject); - assertEquals(actual.getString("name"), jwt.getString("name")); -// try { -// System.out.println("Good " + encodedJWT); -// } catch (InvalidJWTSignatureException e) { -// System.out.println("Bad " + encodedJWT); -// } - } -// for (int i = 0; i < 250; i++) { -// // Sign a message -// byte[] bytes = new byte[i+1]; -// new SecureRandom().nextBytes(bytes); -// signature.update(bytes); -// byte[] signatureBytes = signature.sign(); -// -// // Validation -// Signature verifier = Signature.getInstance("SHA512withECDSA"); -// verifier.initVerify(pair.getPublic()); -// verifier.update(bytes); -// boolean verified = verifier.verify(signatureBytes); -// if (!verified) { -// System.out.println("Public: " + new String(pair.getPublic().getEncoded(), StandardCharsets.UTF_8)); -// System.out.println("Private: " + new String(pair.getPrivate().getEncoded(), StandardCharsets.UTF_8)); -// } -// assertTrue(verified); -// } + // Sign a message + String message = "text ecdsa with sha256"; + signature.update((message).getBytes(StandardCharsets.UTF_8)); + byte[] signatureBytes = signature.sign(); + + // Validation + Signature verifier = Signature.getInstance("SHA256withECDSA"); + verifier.initVerify(pair.getPublic()); + verifier.update(message.getBytes(StandardCharsets.UTF_8)); + assertTrue(verifier.verify(signatureBytes)); } @Test diff --git a/src/test/java/io/fusionauth/security/KeyUtilsTests.java b/src/test/java/io/fusionauth/security/KeyUtilsTests.java index 194e589..ed3dd6e 100644 --- a/src/test/java/io/fusionauth/security/KeyUtilsTests.java +++ b/src/test/java/io/fusionauth/security/KeyUtilsTests.java @@ -16,7 +16,6 @@ package io.fusionauth.security; -import io.fusionauth.pem.PEMDecoder; import io.fusionauth.pem.domain.PEM; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -35,8 +34,8 @@ public class KeyUtilsTests { @DataProvider(name = "ecKeyLengths") public Object[][] ecKeyLengths() { return new Object[][]{ -// {"EC", 256, 256, 256}, -// {"EC", 384, 384, 384}, + {"EC", 256, 256, 256}, + {"EC", 384, 384, 384}, {"EC", 521, 521, 521} }; } @@ -64,7 +63,7 @@ public void problematicKey() { // failures where the key is not an exact size and we have to figure out which key size it should be reported as. // - For testing locally, you can ramp up this invocation count to 100k or something like that to prove that we have // consistency over time. - @Test(dataProvider = "ecKeyLengths", invocationCount = 20000) + @Test(dataProvider = "ecKeyLengths", invocationCount = 500) public void ec_getKeyLength(String algorithm, int keySize, int privateKeySize, int publicKeySize) throws Exception { KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(algorithm); keyPairGenerator.initialize(keySize); From 0cff1e0444a64377043c93a68e9966d866004a6a Mon Sep 17 00:00:00 2001 From: Daniel DeGroff Date: Thu, 29 Feb 2024 10:14:22 -0700 Subject: [PATCH 3/4] Add additional copy to explain the padding that we may encounter when extracting the r and s components, and how the result must be padded to correctly fill the result. --- .../io/fusionauth/jwt/ec/ECDSASignature.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java index 1b31f9c..6cdd548 100644 --- a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java +++ b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java @@ -70,6 +70,7 @@ public byte[] derDecode(Algorithm algorithm) throws IOException { byte[] r = sequence[0].getPositiveBigInteger().toByteArray(); byte[] s = sequence[1].getPositiveBigInteger().toByteArray(); + // The length of the result is fixed and discrete per algorithm. byte[] result; switch (algorithm) { case ES256: @@ -85,8 +86,26 @@ public byte[] derDecode(Algorithm algorithm) throws IOException { throw new IllegalArgumentException("Unable to decode the signature for algorithm [" + algorithm.name() + "]"); } + // Because the response is not encoded, the r and s component must take up an equal amount of the resulting array. + // This allows the consumer of this value to always safely split the value in half based upon an index value since + // the result is not encoded and does not contain any meta-data about the contents. int len = result.length / 2; + // The extracted byte array of the DER encoded value can be left padded. For this reason, the component lengths + // may be greater than 'len' which is half of the result. So for example, if r is left padded, the length may be + // equal to 67 in ES512 even though len is only 66. This is why we must calculate the source position for reading + // when we copy the r byte array into the result. The same is potentially true for either component. We cannot + // make an assumption that the source position in r or s will be 0. + // + // Similarly, when the r and s components are not padded, but they are shorter than len, we need to pad the value + // to be right aligned in the result. This is why the destination position may not be 0 or len respectively for + // r and s. + // + // If s is 65 bytes, then the destination position in the 0 initialized resulting array needs to be len + 1 so that + // we write the final byte of s at the end of the result. + // + // For clarity, calculate each input to the arraycopy method first. + int rSrcPos = r.length > len ? (r.length - len) : 0; int rDstPos = Math.max(0, len - r.length); int rLength = Math.min(r.length, len); From 2af293de31541ff4cc9ea6ab2e279a396db2c7bb Mon Sep 17 00:00:00 2001 From: Daniel DeGroff Date: Thu, 29 Feb 2024 10:16:38 -0700 Subject: [PATCH 4/4] rename len to componentLength --- .../io/fusionauth/jwt/ec/ECDSASignature.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java index 6cdd548..aa20872 100644 --- a/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java +++ b/src/main/java/io/fusionauth/jwt/ec/ECDSASignature.java @@ -89,31 +89,32 @@ public byte[] derDecode(Algorithm algorithm) throws IOException { // Because the response is not encoded, the r and s component must take up an equal amount of the resulting array. // This allows the consumer of this value to always safely split the value in half based upon an index value since // the result is not encoded and does not contain any meta-data about the contents. - int len = result.length / 2; + int componentLength = result.length / 2; // The extracted byte array of the DER encoded value can be left padded. For this reason, the component lengths - // may be greater than 'len' which is half of the result. So for example, if r is left padded, the length may be - // equal to 67 in ES512 even though len is only 66. This is why we must calculate the source position for reading - // when we copy the r byte array into the result. The same is potentially true for either component. We cannot - // make an assumption that the source position in r or s will be 0. + // may be greater than componentLength which is half of the result. So for example, if r is left padded, the + // length may be equal to 67 in ES512 even though componentLength is only 66. This is why we must calculate the + // source position for reading when we copy the r byte array into the result. The same is potentially true for + // either component. We cannot make an assumption that the source position in r or s will be 0. // - // Similarly, when the r and s components are not padded, but they are shorter than len, we need to pad the value - // to be right aligned in the result. This is why the destination position may not be 0 or len respectively for + // Similarly, when the r and s components are not padded, but they are shorter than componentLength, we need to + // pad the value to be right aligned in the result. This is why the destination position may not be 0 or + // componentLength respectively for // r and s. // - // If s is 65 bytes, then the destination position in the 0 initialized resulting array needs to be len + 1 so that - // we write the final byte of s at the end of the result. + // If s is 65 bytes, then the destination position in the 0 initialized resulting array needs to be + // componentLength + 1 so that we write the final byte of s at the end of the result. // // For clarity, calculate each input to the arraycopy method first. - int rSrcPos = r.length > len ? (r.length - len) : 0; - int rDstPos = Math.max(0, len - r.length); - int rLength = Math.min(r.length, len); + int rSrcPos = r.length > componentLength ? (r.length - componentLength) : 0; + int rDstPos = Math.max(0, componentLength - r.length); + int rLength = Math.min(r.length, componentLength); System.arraycopy(r, rSrcPos, result, rDstPos, rLength); - int sSrcPos = s.length > len ? (s.length - len) : 0; - int sDstPos = s.length < len ? (len + (len - s.length)) : len; - int sLength = Math.min(s.length, len); + int sSrcPos = s.length > componentLength ? (s.length - componentLength) : 0; + int sDstPos = s.length < componentLength ? (componentLength + (componentLength - s.length)) : componentLength; + int sLength = Math.min(s.length, componentLength); System.arraycopy(s, sSrcPos, result, sDstPos, sLength); return result;