From 55171498712af76a9f4f5262f674925adb18208e Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 18:16:23 +0200 Subject: [PATCH 01/16] Add function WebAuthnCodecs.parseDerLength and expand encodeDerLength domain --- .../com/yubico/webauthn/WebAuthnCodecs.java | 92 +++++++++++++++++-- .../yubico/webauthn/WebAuthnCodecsSpec.scala | 56 ++++++++++- 2 files changed, 139 insertions(+), 9 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java index 24dffeb4c..2d90aba8c 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java @@ -41,6 +41,8 @@ import java.util.HashMap; import java.util.Map; import java.util.stream.Stream; +import lombok.AllArgsConstructor; +import lombok.NonNull; final class WebAuthnCodecs { @@ -209,17 +211,91 @@ private static PublicKey importCoseEcdsaPublicKey(CBORObject cose) return kFact.generatePublic(new X509EncodedKeySpec(x509Key.getBytes())); } - private static ByteArray encodeDerLength(final int length) { - if (length <= 127) { - return new ByteArray(new byte[] {(byte) length}); + static ByteArray encodeDerLength(final int length) { + if (length < 0) { + throw new IllegalArgumentException("Length is negative: " + length); + } else if (length <= 0x7f) { + return new ByteArray(new byte[] {(byte) (length & 0xff)}); + } else if (length <= 0xff) { + return new ByteArray(new byte[] {(byte) (0x80 | 0x01), (byte) (length & 0xff)}); } else if (length <= 0xffff) { - if (length <= 255) { - return new ByteArray(new byte[] {-127, (byte) length}); + return new ByteArray( + new byte[] {(byte) (0x80 | 0x02), (byte) ((length >> 8) & 0xff), (byte) (length & 0xff)}); + } else if (length <= 0xffffff) { + return new ByteArray( + new byte[] { + (byte) (0x80 | 0x03), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }); + } else { + return new ByteArray( + new byte[] { + (byte) (0x80 | 0x04), + (byte) ((length >> 24) & 0xff), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }); + } + } + + @AllArgsConstructor + static class ParseDerResult { + final T result; + final int nextOffset; + } + + static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { + final int len = der.length - offset; + if (len == 0) { + throw new IllegalArgumentException("Empty input"); + } else if ((der[offset] & 0x80) == 0) { + return new ParseDerResult<>(der[offset] & 0xff, offset + 1); + } else { + final int longLen = der[offset] & 0x7f; + if (len >= longLen) { + switch (longLen) { + case 0: + throw new IllegalArgumentException( + String.format( + "Empty length encoding at offset %d: %s", offset, new ByteArray(der))); + case 1: + return new ParseDerResult<>(der[offset + 1] & 0xff, offset + 2); + case 2: + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 8) | (der[offset + 2] & 0xff), offset + 3); + case 3: + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 16) + | ((der[offset + 2] & 0xff) << 8) + | (der[offset + 3] & 0xff), + offset + 4); + case 4: + if ((der[offset + 1] & 0x80) == 0) { + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 24) + | ((der[offset + 2] & 0xff) << 16) + | ((der[offset + 3] & 0xff) << 8) + | (der[offset + 4] & 0xff), + offset + 5); + } else { + throw new UnsupportedOperationException( + String.format( + "Length out of range of int: 0x%02x%02x%02x%02x", + der[offset + 1], der[offset + 2], der[offset + 3], der[offset + 4])); + } + default: + throw new UnsupportedOperationException( + String.format("Length is too long for int: %d octets", longLen)); + } } else { - return new ByteArray(new byte[] {-126, (byte) (length >> 8), (byte) (length & 0x00ff)}); + throw new IllegalArgumentException( + String.format( + "Length encoding needs %d octets but only %s remain at index %d: %s", + longLen, len - (offset + 1), offset + 1, new ByteArray(der))); } - } else { - throw new UnsupportedOperationException("Too long: " + length); } } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala index c1efa2775..3613cc900 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala @@ -25,6 +25,7 @@ package com.yubico.webauthn import com.yubico.webauthn.data.ByteArray +import com.yubico.webauthn.data.Generators.arbitraryByteArray import com.yubico.webauthn.test.Util import org.junit.runner.RunWith import org.scalacheck.Arbitrary @@ -125,6 +126,59 @@ class WebAuthnCodecsSpec } } - } + describe("DER parsing and encoding:") { + it("encodeDerLength and parseDerLength are each other's inverse.") { + forAll(Gen.chooseNum(0, Int.MaxValue), arbitraryByteArray.arbitrary) { + (len: Int, prefix: ByteArray) => + val encoded = WebAuthnCodecs.encodeDerLength(len) + val decoded = WebAuthnCodecs.parseDerLength(encoded.getBytes, 0) + val decodedWithPrefix = WebAuthnCodecs.parseDerLength( + prefix.concat(encoded).getBytes, + prefix.size, + ) + + decoded.result should equal(len) + decoded.nextOffset should equal(encoded.size) + decodedWithPrefix.result should equal(len) + decodedWithPrefix.nextOffset should equal( + prefix.size + encoded.size + ) + + val recoded = WebAuthnCodecs.encodeDerLength(decoded.result) + recoded should equal(encoded) + } + } + + it("parseDerLength tolerates unnecessarily long encodings.") { + WebAuthnCodecs + .parseDerLength(Array(0x81, 0).map(_.toByte), 0) + .result should equal(0) + WebAuthnCodecs + .parseDerLength(Array(0x82, 0, 0).map(_.toByte), 0) + .result should equal(0) + WebAuthnCodecs + .parseDerLength(Array(0x83, 0, 0, 0).map(_.toByte), 0) + .result should equal(0) + WebAuthnCodecs + .parseDerLength(Array(0x84, 0, 0, 0, 0).map(_.toByte), 0) + .result should equal(0) + WebAuthnCodecs + .parseDerLength(Array(0x81, 7).map(_.toByte), 0) + .result should equal(7) + WebAuthnCodecs + .parseDerLength(Array(0x82, 0, 7).map(_.toByte), 0) + .result should equal(7) + WebAuthnCodecs + .parseDerLength(Array(0x83, 0, 0, 7).map(_.toByte), 0) + .result should equal(7) + WebAuthnCodecs + .parseDerLength(Array(0x84, 0, 0, 4, 2).map(_.toByte), 0) + .result should equal(1026) + WebAuthnCodecs + .parseDerLength(Array(0x84, 0, 1, 33, 7).map(_.toByte), 0) + .result should equal(73991) + } + } + } } From c5a23e4cdbdc0354671355b839d1aed45efd9115 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 19:18:50 +0200 Subject: [PATCH 02/16] Add function WebAuthnCodecs.parseDerSequence --- .../com/yubico/webauthn/WebAuthnCodecs.java | 19 +++++++- .../yubico/webauthn/WebAuthnCodecsSpec.scala | 44 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java index 2d90aba8c..9e329e32e 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java @@ -299,6 +299,23 @@ static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { } } + /** Parse a SEQUENCE and return a copy of the content octets. */ + static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { + final int len = der.length - offset; + if (len == 0) { + throw new IllegalArgumentException( + String.format("Empty input at offset %d: %s", offset, new ByteArray(der))); + } else if ((der[offset] & 0xff) == 0x30) { + final ParseDerResult contentLen = parseDerLength(der, offset + 1); + final int contentEnd = contentLen.nextOffset + contentLen.result; + return new ParseDerResult<>( + new ByteArray(Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd)), contentEnd); + } else { + throw new IllegalArgumentException( + String.format("Not a SEQUENCE tag (0x30) at offset %d: %s", offset, new ByteArray(der))); + } + } + private static ByteArray encodeDerObjectId(final ByteArray oid) { return new ByteArray(new byte[] {0x06, (byte) oid.size()}).concat(oid); } @@ -310,7 +327,7 @@ private static ByteArray encodeDerBitStringWithZeroUnused(final ByteArray conten .concat(content); } - private static ByteArray encodeDerSequence(final ByteArray... items) { + static ByteArray encodeDerSequence(final ByteArray... items) { final ByteArray content = Stream.of(items).reduce(ByteArray::concat).orElseGet(() -> new ByteArray(new byte[0])); return new ByteArray(new byte[] {0x30}).concat(encodeDerLength(content.size())).concat(content); diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala index 3613cc900..799c0ede0 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala @@ -179,6 +179,50 @@ class WebAuthnCodecsSpec .parseDerLength(Array(0x84, 0, 1, 33, 7).map(_.toByte), 0) .result should equal(73991) } + + it("encodeDerSequence and parseDerSequenceEnd are (almost) each other's inverse.") { + forAll { (data: Array[ByteArray], prefix: ByteArray) => + val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) + val decoded = WebAuthnCodecs.parseDerSequence(encoded.getBytes, 0) + val encodedWithPrefix = prefix.concat(encoded) + val decodedWithPrefix = WebAuthnCodecs.parseDerSequence( + encodedWithPrefix.getBytes, + prefix.size, + ) + + val expectedContent: ByteArray = + data.fold(new ByteArray(Array.empty))((a, b) => a.concat(b)) + decoded.result should equal(expectedContent) + decodedWithPrefix.result should equal(expectedContent) + decoded.nextOffset should equal(encoded.size) + decodedWithPrefix.nextOffset should equal(prefix.size + encoded.size) + } + } + + it("parseDerSequence fails if the first byte is not 0x30.") { + forAll { (tag: Byte, data: Array[ByteArray]) => + whenever(tag != 0x30) { + val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) + an[IllegalArgumentException] shouldBe thrownBy { + WebAuthnCodecs.parseDerSequence( + encoded.getBytes.updated(0, tag), + 0, + ) + } + } + } + } + + it("parseDerSequence fails on empty input.") { + an[IllegalArgumentException] shouldBe thrownBy { + WebAuthnCodecs.parseDerSequence(Array.empty, 0) + } + forAll { data: Array[Byte] => + an[IllegalArgumentException] shouldBe thrownBy { + WebAuthnCodecs.parseDerSequence(data, data.length) + } + } + } } } } From 726a63b2bb0a86e91b7853b801e05f336a9cae0e Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Fri, 12 Jul 2024 12:31:59 +0200 Subject: [PATCH 03/16] Add function WebAuthnCodecs.parseDerExplicitlyTaggedContextSpecificConstructed --- .../com/yubico/webauthn/WebAuthnCodecs.java | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java index 9e329e32e..371c8fb8b 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java @@ -299,20 +299,45 @@ static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { } } - /** Parse a SEQUENCE and return a copy of the content octets. */ - static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { + private static ParseDerResult parseDerTagged( + @NonNull byte[] der, int offset, byte expectTag) { final int len = der.length - offset; if (len == 0) { throw new IllegalArgumentException( String.format("Empty input at offset %d: %s", offset, new ByteArray(der))); - } else if ((der[offset] & 0xff) == 0x30) { - final ParseDerResult contentLen = parseDerLength(der, offset + 1); - final int contentEnd = contentLen.nextOffset + contentLen.result; - return new ParseDerResult<>( - new ByteArray(Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd)), contentEnd); } else { - throw new IllegalArgumentException( - String.format("Not a SEQUENCE tag (0x30) at offset %d: %s", offset, new ByteArray(der))); + final byte tag = der[offset]; + if (tag == expectTag) { + final ParseDerResult contentLen = parseDerLength(der, offset + 1); + final int contentEnd = contentLen.nextOffset + contentLen.result; + return new ParseDerResult<>( + new ByteArray(Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd)), contentEnd); + } else { + throw new IllegalArgumentException( + String.format( + "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: %s", + tag, expectTag, offset, new ByteArray(der))); + } + } + } + + /** Parse a SEQUENCE and return a copy of the content octets. */ + static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { + return parseDerTagged(der, offset, (byte) 0x30); + } + + /** + * Parse an explicitly tagged value of class "context-specific" (bits 8-7 are 0b10), in + * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the + * content octets. + */ + static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( + @NonNull byte[] der, int offset, byte tagNumber) { + if (tagNumber <= 30 && tagNumber >= 0) { + return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); + } else { + throw new UnsupportedOperationException( + String.format("Tag number out of range: %d (expected 0 to 30, inclusive)", tagNumber)); } } From 75146630eae79f5ac43c1fd3075025aabf9ce282 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 20:29:54 +0200 Subject: [PATCH 04/16] Add functions BinaryUtil.copyInto and .concat --- .../com/yubico/internal/util/BinaryUtil.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java index 2f47aee3f..8c8eb55b3 100644 --- a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java @@ -36,6 +36,37 @@ public static byte[] copy(byte[] bytes) { return Arrays.copyOf(bytes, bytes.length); } + /** + * Copy src into dest beginning at the offset destFrom, + * then return the modified dest. + */ + public static byte[] copyInto(byte[] src, byte[] dest, int destFrom) { + if (dest.length - destFrom < src.length) { + throw new IllegalArgumentException("Source array will not fit in destination array"); + } + if (destFrom < 0) { + throw new IllegalArgumentException("Invalid destination range"); + } + + for (int i = 0; i < src.length; ++i) { + dest[destFrom + i] = src[i]; + } + + return dest; + } + + /** Return a new array containing the concatenation of the argument arrays. */ + public static byte[] concat(byte[]... arrays) { + final int len = Arrays.stream(arrays).map(a -> a.length).reduce(0, Integer::sum); + byte[] result = new byte[len]; + int i = 0; + for (byte[] src : arrays) { + copyInto(src, result, i); + i += src.length; + } + return result; + } + /** * @param bytes Bytes to encode */ From 1b547a4b1c10a24a0cd4b7c2e277e2626e7409bb Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 20:30:30 +0200 Subject: [PATCH 05/16] Use byte[] instead of ByteArray in DER encoding and parsing functions --- .../com/yubico/webauthn/WebAuthnCodecs.java | 121 +++++++++--------- .../yubico/webauthn/WebAuthnCodecsSpec.scala | 63 ++++----- 2 files changed, 94 insertions(+), 90 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java index 371c8fb8b..74d910b3e 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java @@ -26,6 +26,7 @@ import com.google.common.primitives.Bytes; import com.upokecenter.cbor.CBORObject; +import com.yubico.internal.util.BinaryUtil; import com.yubico.webauthn.data.ByteArray; import com.yubico.webauthn.data.COSEAlgorithmIdentifier; import java.io.IOException; @@ -40,7 +41,6 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; -import java.util.stream.Stream; import lombok.AllArgsConstructor; import lombok.NonNull; @@ -175,69 +175,70 @@ private static PublicKey importCoseRsaPublicKey(CBORObject cose) private static PublicKey importCoseEcdsaPublicKey(CBORObject cose) throws NoSuchAlgorithmException, InvalidKeySpecException { final int crv = cose.get(CBORObject.FromObject(-1)).AsInt32Value(); - final ByteArray x = new ByteArray(cose.get(CBORObject.FromObject(-2)).GetByteString()); - final ByteArray y = new ByteArray(cose.get(CBORObject.FromObject(-3)).GetByteString()); + final byte[] x = cose.get(CBORObject.FromObject(-2)).GetByteString(); + final byte[] y = cose.get(CBORObject.FromObject(-3)).GetByteString(); - final ByteArray curveOid; + final byte[] curveOid; switch (crv) { case 1: - curveOid = P256_CURVE_OID; + curveOid = P256_CURVE_OID.getBytes(); break; case 2: - curveOid = P384_CURVE_OID; + curveOid = P384_CURVE_OID.getBytes(); break; case 3: - curveOid = P512_CURVE_OID; + curveOid = P512_CURVE_OID.getBytes(); break; default: throw new IllegalArgumentException("Unknown COSE EC2 curve: " + crv); } - final ByteArray algId = - encodeDerSequence(encodeDerObjectId(EC_PUBLIC_KEY_OID), encodeDerObjectId(curveOid)); + final byte[] algId = + encodeDerSequence( + encodeDerObjectId(EC_PUBLIC_KEY_OID.getBytes()), encodeDerObjectId(curveOid)); - final ByteArray rawKey = + final byte[] rawKey = encodeDerBitStringWithZeroUnused( - new ByteArray(new byte[] {0x04}) // Raw EC public key with x and y - .concat(x) - .concat(y)); + BinaryUtil.concat( + new byte[] {0x04}, // Raw EC public key with x and y + x, + y)); - final ByteArray x509Key = encodeDerSequence(algId, rawKey); + final byte[] x509Key = encodeDerSequence(algId, rawKey); KeyFactory kFact = KeyFactory.getInstance("EC"); - return kFact.generatePublic(new X509EncodedKeySpec(x509Key.getBytes())); + return kFact.generatePublic(new X509EncodedKeySpec(x509Key)); } - static ByteArray encodeDerLength(final int length) { + static byte[] encodeDerLength(final int length) { if (length < 0) { throw new IllegalArgumentException("Length is negative: " + length); } else if (length <= 0x7f) { - return new ByteArray(new byte[] {(byte) (length & 0xff)}); + return new byte[] {(byte) (length & 0xff)}; } else if (length <= 0xff) { - return new ByteArray(new byte[] {(byte) (0x80 | 0x01), (byte) (length & 0xff)}); + return new byte[] {(byte) (0x80 | 0x01), (byte) (length & 0xff)}; } else if (length <= 0xffff) { - return new ByteArray( - new byte[] {(byte) (0x80 | 0x02), (byte) ((length >> 8) & 0xff), (byte) (length & 0xff)}); + return new byte[] { + (byte) (0x80 | 0x02), (byte) ((length >> 8) & 0xff), (byte) (length & 0xff) + }; } else if (length <= 0xffffff) { - return new ByteArray( - new byte[] { - (byte) (0x80 | 0x03), - (byte) ((length >> 16) & 0xff), - (byte) ((length >> 8) & 0xff), - (byte) (length & 0xff) - }); + return new byte[] { + (byte) (0x80 | 0x03), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }; } else { - return new ByteArray( - new byte[] { - (byte) (0x80 | 0x04), - (byte) ((length >> 24) & 0xff), - (byte) ((length >> 16) & 0xff), - (byte) ((length >> 8) & 0xff), - (byte) (length & 0xff) - }); + return new byte[] { + (byte) (0x80 | 0x04), + (byte) ((length >> 24) & 0xff), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }; } } @@ -260,7 +261,7 @@ static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { case 0: throw new IllegalArgumentException( String.format( - "Empty length encoding at offset %d: %s", offset, new ByteArray(der))); + "Empty length encoding at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); case 1: return new ParseDerResult<>(der[offset + 1] & 0xff, offset + 2); case 2: @@ -293,36 +294,36 @@ static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { } else { throw new IllegalArgumentException( String.format( - "Length encoding needs %d octets but only %s remain at index %d: %s", - longLen, len - (offset + 1), offset + 1, new ByteArray(der))); + "Length encoding needs %d octets but only %s remain at index %d: 0x%s", + longLen, len - (offset + 1), offset + 1, BinaryUtil.toHex(der))); } } } - private static ParseDerResult parseDerTagged( + private static ParseDerResult parseDerTagged( @NonNull byte[] der, int offset, byte expectTag) { final int len = der.length - offset; if (len == 0) { throw new IllegalArgumentException( - String.format("Empty input at offset %d: %s", offset, new ByteArray(der))); + String.format("Empty input at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); } else { final byte tag = der[offset]; if (tag == expectTag) { final ParseDerResult contentLen = parseDerLength(der, offset + 1); final int contentEnd = contentLen.nextOffset + contentLen.result; return new ParseDerResult<>( - new ByteArray(Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd)), contentEnd); + Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), contentEnd); } else { throw new IllegalArgumentException( String.format( - "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: %s", - tag, expectTag, offset, new ByteArray(der))); + "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: 0x%s", + tag, expectTag, offset, BinaryUtil.toHex(der))); } } } /** Parse a SEQUENCE and return a copy of the content octets. */ - static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { + static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { return parseDerTagged(der, offset, (byte) 0x30); } @@ -331,7 +332,7 @@ static ParseDerResult parseDerSequence(@NonNull byte[] der, int offse * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the * content octets. */ - static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( + static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( @NonNull byte[] der, int offset, byte tagNumber) { if (tagNumber <= 30 && tagNumber >= 0) { return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); @@ -341,21 +342,21 @@ static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstruc } } - private static ByteArray encodeDerObjectId(final ByteArray oid) { - return new ByteArray(new byte[] {0x06, (byte) oid.size()}).concat(oid); + private static byte[] encodeDerObjectId(@NonNull byte[] oid) { + byte[] result = new byte[2 + oid.length]; + result[0] = 0x06; + result[1] = (byte) oid.length; + return BinaryUtil.copyInto(oid, result, 2); } - private static ByteArray encodeDerBitStringWithZeroUnused(final ByteArray content) { - return new ByteArray(new byte[] {0x03}) - .concat(encodeDerLength(1 + content.size())) - .concat(new ByteArray(new byte[] {0})) - .concat(content); + private static byte[] encodeDerBitStringWithZeroUnused(@NonNull byte[] content) { + return BinaryUtil.concat( + new byte[] {0x03}, encodeDerLength(1 + content.length), new byte[] {0}, content); } - static ByteArray encodeDerSequence(final ByteArray... items) { - final ByteArray content = - Stream.of(items).reduce(ByteArray::concat).orElseGet(() -> new ByteArray(new byte[0])); - return new ByteArray(new byte[] {0x30}).concat(encodeDerLength(content.size())).concat(content); + static byte[] encodeDerSequence(final byte[]... items) { + byte[] content = BinaryUtil.concat(items); + return BinaryUtil.concat(new byte[] {0x30}, encodeDerLength(content.length), content); } private static PublicKey importCoseEdDsaPublicKey(CBORObject cose) @@ -371,12 +372,12 @@ private static PublicKey importCoseEdDsaPublicKey(CBORObject cose) private static PublicKey importCoseEd25519PublicKey(CBORObject cose) throws InvalidKeySpecException, NoSuchAlgorithmException { - final ByteArray rawKey = new ByteArray(cose.get(CBORObject.FromObject(-2)).GetByteString()); - final ByteArray x509Key = - encodeDerSequence(ED25519_ALG_ID, encodeDerBitStringWithZeroUnused(rawKey)); + final byte[] rawKey = cose.get(CBORObject.FromObject(-2)).GetByteString(); + final byte[] x509Key = + encodeDerSequence(ED25519_ALG_ID.getBytes(), encodeDerBitStringWithZeroUnused(rawKey)); KeyFactory kFact = KeyFactory.getInstance("EdDSA"); - return kFact.generatePublic(new X509EncodedKeySpec(x509Key.getBytes())); + return kFact.generatePublic(new X509EncodedKeySpec(x509Key)); } static String getJavaAlgorithmName(COSEAlgorithmIdentifier alg) { diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala index 799c0ede0..6c055b69c 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala @@ -24,8 +24,8 @@ package com.yubico.webauthn +import com.yubico.internal.util.BinaryUtil import com.yubico.webauthn.data.ByteArray -import com.yubico.webauthn.data.Generators.arbitraryByteArray import com.yubico.webauthn.test.Util import org.junit.runner.RunWith import org.scalacheck.Arbitrary @@ -129,24 +129,26 @@ class WebAuthnCodecsSpec describe("DER parsing and encoding:") { it("encodeDerLength and parseDerLength are each other's inverse.") { - forAll(Gen.chooseNum(0, Int.MaxValue), arbitraryByteArray.arbitrary) { - (len: Int, prefix: ByteArray) => - val encoded = WebAuthnCodecs.encodeDerLength(len) - val decoded = WebAuthnCodecs.parseDerLength(encoded.getBytes, 0) - val decodedWithPrefix = WebAuthnCodecs.parseDerLength( - prefix.concat(encoded).getBytes, - prefix.size, - ) - - decoded.result should equal(len) - decoded.nextOffset should equal(encoded.size) - decodedWithPrefix.result should equal(len) - decodedWithPrefix.nextOffset should equal( - prefix.size + encoded.size - ) - - val recoded = WebAuthnCodecs.encodeDerLength(decoded.result) - recoded should equal(encoded) + forAll( + Gen.chooseNum(0, Int.MaxValue), + Arbitrary.arbitrary[Array[Byte]], + ) { (len: Int, prefix: Array[Byte]) => + val encoded = WebAuthnCodecs.encodeDerLength(len) + val decoded = WebAuthnCodecs.parseDerLength(encoded, 0) + val decodedWithPrefix = WebAuthnCodecs.parseDerLength( + BinaryUtil.concat(prefix, encoded), + prefix.length, + ) + + decoded.result should equal(len) + decoded.nextOffset should equal(encoded.length) + decodedWithPrefix.result should equal(len) + decodedWithPrefix.nextOffset should equal( + prefix.length + encoded.length + ) + + val recoded = WebAuthnCodecs.encodeDerLength(decoded.result) + recoded should equal(encoded) } } @@ -181,31 +183,32 @@ class WebAuthnCodecsSpec } it("encodeDerSequence and parseDerSequenceEnd are (almost) each other's inverse.") { - forAll { (data: Array[ByteArray], prefix: ByteArray) => + forAll { (data: Array[Array[Byte]], prefix: Array[Byte]) => val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) - val decoded = WebAuthnCodecs.parseDerSequence(encoded.getBytes, 0) - val encodedWithPrefix = prefix.concat(encoded) + val decoded = WebAuthnCodecs.parseDerSequence(encoded, 0) + val encodedWithPrefix = BinaryUtil.concat(prefix, encoded) val decodedWithPrefix = WebAuthnCodecs.parseDerSequence( - encodedWithPrefix.getBytes, - prefix.size, + encodedWithPrefix, + prefix.length, ) - val expectedContent: ByteArray = - data.fold(new ByteArray(Array.empty))((a, b) => a.concat(b)) + val expectedContent: Array[Byte] = BinaryUtil.concat(data: _*) decoded.result should equal(expectedContent) decodedWithPrefix.result should equal(expectedContent) - decoded.nextOffset should equal(encoded.size) - decodedWithPrefix.nextOffset should equal(prefix.size + encoded.size) + decoded.nextOffset should equal(encoded.length) + decodedWithPrefix.nextOffset should equal( + prefix.length + encoded.length + ) } } it("parseDerSequence fails if the first byte is not 0x30.") { - forAll { (tag: Byte, data: Array[ByteArray]) => + forAll { (tag: Byte, data: Array[Array[Byte]]) => whenever(tag != 0x30) { val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) an[IllegalArgumentException] shouldBe thrownBy { WebAuthnCodecs.parseDerSequence( - encoded.getBytes.updated(0, tag), + encoded.updated(0, tag), 0, ) } From 6dfbe9a0c796acd5bbba45b7f68b4b4b111c6ce7 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 21:40:41 +0200 Subject: [PATCH 06/16] Move DER encoding/parsing functions to BinaryUtil --- .../com/yubico/webauthn/WebAuthnCodecs.java | 160 +----------------- .../yubico/webauthn/WebAuthnCodecsSpec.scala | 101 ----------- .../com/yubico/internal/util/BinaryUtil.java | 148 ++++++++++++++++ .../yubico/internal/util/BinaryUtilSpec.scala | 102 +++++++++++ 4 files changed, 257 insertions(+), 254 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java index 74d910b3e..df223757b 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java @@ -41,8 +41,6 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; -import lombok.AllArgsConstructor; -import lombok.NonNull; final class WebAuthnCodecs { @@ -197,168 +195,23 @@ private static PublicKey importCoseEcdsaPublicKey(CBORObject cose) } final byte[] algId = - encodeDerSequence( - encodeDerObjectId(EC_PUBLIC_KEY_OID.getBytes()), encodeDerObjectId(curveOid)); + BinaryUtil.encodeDerSequence( + BinaryUtil.encodeDerObjectId(EC_PUBLIC_KEY_OID.getBytes()), + BinaryUtil.encodeDerObjectId(curveOid)); final byte[] rawKey = - encodeDerBitStringWithZeroUnused( + BinaryUtil.encodeDerBitStringWithZeroUnused( BinaryUtil.concat( new byte[] {0x04}, // Raw EC public key with x and y x, y)); - final byte[] x509Key = encodeDerSequence(algId, rawKey); + final byte[] x509Key = BinaryUtil.encodeDerSequence(algId, rawKey); KeyFactory kFact = KeyFactory.getInstance("EC"); return kFact.generatePublic(new X509EncodedKeySpec(x509Key)); } - static byte[] encodeDerLength(final int length) { - if (length < 0) { - throw new IllegalArgumentException("Length is negative: " + length); - } else if (length <= 0x7f) { - return new byte[] {(byte) (length & 0xff)}; - } else if (length <= 0xff) { - return new byte[] {(byte) (0x80 | 0x01), (byte) (length & 0xff)}; - } else if (length <= 0xffff) { - return new byte[] { - (byte) (0x80 | 0x02), (byte) ((length >> 8) & 0xff), (byte) (length & 0xff) - }; - } else if (length <= 0xffffff) { - return new byte[] { - (byte) (0x80 | 0x03), - (byte) ((length >> 16) & 0xff), - (byte) ((length >> 8) & 0xff), - (byte) (length & 0xff) - }; - } else { - return new byte[] { - (byte) (0x80 | 0x04), - (byte) ((length >> 24) & 0xff), - (byte) ((length >> 16) & 0xff), - (byte) ((length >> 8) & 0xff), - (byte) (length & 0xff) - }; - } - } - - @AllArgsConstructor - static class ParseDerResult { - final T result; - final int nextOffset; - } - - static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { - final int len = der.length - offset; - if (len == 0) { - throw new IllegalArgumentException("Empty input"); - } else if ((der[offset] & 0x80) == 0) { - return new ParseDerResult<>(der[offset] & 0xff, offset + 1); - } else { - final int longLen = der[offset] & 0x7f; - if (len >= longLen) { - switch (longLen) { - case 0: - throw new IllegalArgumentException( - String.format( - "Empty length encoding at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); - case 1: - return new ParseDerResult<>(der[offset + 1] & 0xff, offset + 2); - case 2: - return new ParseDerResult<>( - ((der[offset + 1] & 0xff) << 8) | (der[offset + 2] & 0xff), offset + 3); - case 3: - return new ParseDerResult<>( - ((der[offset + 1] & 0xff) << 16) - | ((der[offset + 2] & 0xff) << 8) - | (der[offset + 3] & 0xff), - offset + 4); - case 4: - if ((der[offset + 1] & 0x80) == 0) { - return new ParseDerResult<>( - ((der[offset + 1] & 0xff) << 24) - | ((der[offset + 2] & 0xff) << 16) - | ((der[offset + 3] & 0xff) << 8) - | (der[offset + 4] & 0xff), - offset + 5); - } else { - throw new UnsupportedOperationException( - String.format( - "Length out of range of int: 0x%02x%02x%02x%02x", - der[offset + 1], der[offset + 2], der[offset + 3], der[offset + 4])); - } - default: - throw new UnsupportedOperationException( - String.format("Length is too long for int: %d octets", longLen)); - } - } else { - throw new IllegalArgumentException( - String.format( - "Length encoding needs %d octets but only %s remain at index %d: 0x%s", - longLen, len - (offset + 1), offset + 1, BinaryUtil.toHex(der))); - } - } - } - - private static ParseDerResult parseDerTagged( - @NonNull byte[] der, int offset, byte expectTag) { - final int len = der.length - offset; - if (len == 0) { - throw new IllegalArgumentException( - String.format("Empty input at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); - } else { - final byte tag = der[offset]; - if (tag == expectTag) { - final ParseDerResult contentLen = parseDerLength(der, offset + 1); - final int contentEnd = contentLen.nextOffset + contentLen.result; - return new ParseDerResult<>( - Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), contentEnd); - } else { - throw new IllegalArgumentException( - String.format( - "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: 0x%s", - tag, expectTag, offset, BinaryUtil.toHex(der))); - } - } - } - - /** Parse a SEQUENCE and return a copy of the content octets. */ - static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { - return parseDerTagged(der, offset, (byte) 0x30); - } - - /** - * Parse an explicitly tagged value of class "context-specific" (bits 8-7 are 0b10), in - * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the - * content octets. - */ - static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( - @NonNull byte[] der, int offset, byte tagNumber) { - if (tagNumber <= 30 && tagNumber >= 0) { - return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); - } else { - throw new UnsupportedOperationException( - String.format("Tag number out of range: %d (expected 0 to 30, inclusive)", tagNumber)); - } - } - - private static byte[] encodeDerObjectId(@NonNull byte[] oid) { - byte[] result = new byte[2 + oid.length]; - result[0] = 0x06; - result[1] = (byte) oid.length; - return BinaryUtil.copyInto(oid, result, 2); - } - - private static byte[] encodeDerBitStringWithZeroUnused(@NonNull byte[] content) { - return BinaryUtil.concat( - new byte[] {0x03}, encodeDerLength(1 + content.length), new byte[] {0}, content); - } - - static byte[] encodeDerSequence(final byte[]... items) { - byte[] content = BinaryUtil.concat(items); - return BinaryUtil.concat(new byte[] {0x30}, encodeDerLength(content.length), content); - } - private static PublicKey importCoseEdDsaPublicKey(CBORObject cose) throws InvalidKeySpecException, NoSuchAlgorithmException { final int curveId = cose.get(CBORObject.FromObject(-1)).AsInt32(); @@ -374,7 +227,8 @@ private static PublicKey importCoseEd25519PublicKey(CBORObject cose) throws InvalidKeySpecException, NoSuchAlgorithmException { final byte[] rawKey = cose.get(CBORObject.FromObject(-2)).GetByteString(); final byte[] x509Key = - encodeDerSequence(ED25519_ALG_ID.getBytes(), encodeDerBitStringWithZeroUnused(rawKey)); + BinaryUtil.encodeDerSequence( + ED25519_ALG_ID.getBytes(), BinaryUtil.encodeDerBitStringWithZeroUnused(rawKey)); KeyFactory kFact = KeyFactory.getInstance("EdDSA"); return kFact.generatePublic(new X509EncodedKeySpec(x509Key)); diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala index 6c055b69c..a22d19e54 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/WebAuthnCodecsSpec.scala @@ -24,7 +24,6 @@ package com.yubico.webauthn -import com.yubico.internal.util.BinaryUtil import com.yubico.webauthn.data.ByteArray import com.yubico.webauthn.test.Util import org.junit.runner.RunWith @@ -127,105 +126,5 @@ class WebAuthnCodecsSpec } - describe("DER parsing and encoding:") { - it("encodeDerLength and parseDerLength are each other's inverse.") { - forAll( - Gen.chooseNum(0, Int.MaxValue), - Arbitrary.arbitrary[Array[Byte]], - ) { (len: Int, prefix: Array[Byte]) => - val encoded = WebAuthnCodecs.encodeDerLength(len) - val decoded = WebAuthnCodecs.parseDerLength(encoded, 0) - val decodedWithPrefix = WebAuthnCodecs.parseDerLength( - BinaryUtil.concat(prefix, encoded), - prefix.length, - ) - - decoded.result should equal(len) - decoded.nextOffset should equal(encoded.length) - decodedWithPrefix.result should equal(len) - decodedWithPrefix.nextOffset should equal( - prefix.length + encoded.length - ) - - val recoded = WebAuthnCodecs.encodeDerLength(decoded.result) - recoded should equal(encoded) - } - } - - it("parseDerLength tolerates unnecessarily long encodings.") { - WebAuthnCodecs - .parseDerLength(Array(0x81, 0).map(_.toByte), 0) - .result should equal(0) - WebAuthnCodecs - .parseDerLength(Array(0x82, 0, 0).map(_.toByte), 0) - .result should equal(0) - WebAuthnCodecs - .parseDerLength(Array(0x83, 0, 0, 0).map(_.toByte), 0) - .result should equal(0) - WebAuthnCodecs - .parseDerLength(Array(0x84, 0, 0, 0, 0).map(_.toByte), 0) - .result should equal(0) - WebAuthnCodecs - .parseDerLength(Array(0x81, 7).map(_.toByte), 0) - .result should equal(7) - WebAuthnCodecs - .parseDerLength(Array(0x82, 0, 7).map(_.toByte), 0) - .result should equal(7) - WebAuthnCodecs - .parseDerLength(Array(0x83, 0, 0, 7).map(_.toByte), 0) - .result should equal(7) - WebAuthnCodecs - .parseDerLength(Array(0x84, 0, 0, 4, 2).map(_.toByte), 0) - .result should equal(1026) - WebAuthnCodecs - .parseDerLength(Array(0x84, 0, 1, 33, 7).map(_.toByte), 0) - .result should equal(73991) - } - - it("encodeDerSequence and parseDerSequenceEnd are (almost) each other's inverse.") { - forAll { (data: Array[Array[Byte]], prefix: Array[Byte]) => - val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) - val decoded = WebAuthnCodecs.parseDerSequence(encoded, 0) - val encodedWithPrefix = BinaryUtil.concat(prefix, encoded) - val decodedWithPrefix = WebAuthnCodecs.parseDerSequence( - encodedWithPrefix, - prefix.length, - ) - - val expectedContent: Array[Byte] = BinaryUtil.concat(data: _*) - decoded.result should equal(expectedContent) - decodedWithPrefix.result should equal(expectedContent) - decoded.nextOffset should equal(encoded.length) - decodedWithPrefix.nextOffset should equal( - prefix.length + encoded.length - ) - } - } - - it("parseDerSequence fails if the first byte is not 0x30.") { - forAll { (tag: Byte, data: Array[Array[Byte]]) => - whenever(tag != 0x30) { - val encoded = WebAuthnCodecs.encodeDerSequence(data: _*) - an[IllegalArgumentException] shouldBe thrownBy { - WebAuthnCodecs.parseDerSequence( - encoded.updated(0, tag), - 0, - ) - } - } - } - } - - it("parseDerSequence fails on empty input.") { - an[IllegalArgumentException] shouldBe thrownBy { - WebAuthnCodecs.parseDerSequence(Array.empty, 0) - } - forAll { data: Array[Byte] => - an[IllegalArgumentException] shouldBe thrownBy { - WebAuthnCodecs.parseDerSequence(data, data.length) - } - } - } - } } } diff --git a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java index 8c8eb55b3..0223cf1a8 100644 --- a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java @@ -29,6 +29,8 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.Arrays; +import lombok.AllArgsConstructor; +import lombok.NonNull; public class BinaryUtil { @@ -197,4 +199,150 @@ public static byte[] readAll(InputStream is) throws IOException { } } } + + public static byte[] encodeDerLength(final int length) { + if (length < 0) { + throw new IllegalArgumentException("Length is negative: " + length); + } else if (length <= 0x7f) { + return new byte[] {(byte) (length & 0xff)}; + } else if (length <= 0xff) { + return new byte[] {(byte) (0x80 | 0x01), (byte) (length & 0xff)}; + } else if (length <= 0xffff) { + return new byte[] { + (byte) (0x80 | 0x02), (byte) ((length >> 8) & 0xff), (byte) (length & 0xff) + }; + } else if (length <= 0xffffff) { + return new byte[] { + (byte) (0x80 | 0x03), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }; + } else { + return new byte[] { + (byte) (0x80 | 0x04), + (byte) ((length >> 24) & 0xff), + (byte) ((length >> 16) & 0xff), + (byte) ((length >> 8) & 0xff), + (byte) (length & 0xff) + }; + } + } + + @AllArgsConstructor + public static class ParseDerResult { + public final T result; + public final int nextOffset; + } + + public static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { + final int len = der.length - offset; + if (len == 0) { + throw new IllegalArgumentException("Empty input"); + } else if ((der[offset] & 0x80) == 0) { + return new ParseDerResult<>(der[offset] & 0xff, offset + 1); + } else { + final int longLen = der[offset] & 0x7f; + if (len >= longLen) { + switch (longLen) { + case 0: + throw new IllegalArgumentException( + String.format( + "Empty length encoding at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); + case 1: + return new ParseDerResult<>(der[offset + 1] & 0xff, offset + 2); + case 2: + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 8) | (der[offset + 2] & 0xff), offset + 3); + case 3: + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 16) + | ((der[offset + 2] & 0xff) << 8) + | (der[offset + 3] & 0xff), + offset + 4); + case 4: + if ((der[offset + 1] & 0x80) == 0) { + return new ParseDerResult<>( + ((der[offset + 1] & 0xff) << 24) + | ((der[offset + 2] & 0xff) << 16) + | ((der[offset + 3] & 0xff) << 8) + | (der[offset + 4] & 0xff), + offset + 5); + } else { + throw new UnsupportedOperationException( + String.format( + "Length out of range of int: 0x%02x%02x%02x%02x", + der[offset + 1], der[offset + 2], der[offset + 3], der[offset + 4])); + } + default: + throw new UnsupportedOperationException( + String.format("Length is too long for int: %d octets", longLen)); + } + } else { + throw new IllegalArgumentException( + String.format( + "Length encoding needs %d octets but only %s remain at index %d: 0x%s", + longLen, len - (offset + 1), offset + 1, BinaryUtil.toHex(der))); + } + } + } + + private static ParseDerResult parseDerTagged( + @NonNull byte[] der, int offset, byte expectTag) { + final int len = der.length - offset; + if (len == 0) { + throw new IllegalArgumentException( + String.format("Empty input at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); + } else { + final byte tag = der[offset]; + if (tag == expectTag) { + final ParseDerResult contentLen = parseDerLength(der, offset + 1); + final int contentEnd = contentLen.nextOffset + contentLen.result; + return new ParseDerResult<>( + Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), contentEnd); + } else { + throw new IllegalArgumentException( + String.format( + "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: 0x%s", + tag, expectTag, offset, BinaryUtil.toHex(der))); + } + } + } + + /** Parse a SEQUENCE and return a copy of the content octets. */ + public static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { + return parseDerTagged(der, offset, (byte) 0x30); + } + + /** + * Parse an explicitly tagged value of class "context-specific" (bits 8-7 are 0b10), in + * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the + * content octets. + */ + public static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( + @NonNull byte[] der, int offset, byte tagNumber) { + if (tagNumber <= 30 && tagNumber >= 0) { + return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); + } else { + throw new UnsupportedOperationException( + String.format("Tag number out of range: %d (expected 0 to 30, inclusive)", tagNumber)); + } + } + + public static byte[] encodeDerObjectId(@NonNull byte[] oid) { + byte[] result = new byte[2 + oid.length]; + result[0] = 0x06; + result[1] = (byte) oid.length; + return BinaryUtil.copyInto(oid, result, 2); + } + + public static byte[] encodeDerBitStringWithZeroUnused(@NonNull byte[] content) { + return BinaryUtil.concat( + new byte[] {0x03}, encodeDerLength(1 + content.length), new byte[] {0}, content); + } + + public static byte[] encodeDerSequence(final byte[]... items) { + byte[] content = BinaryUtil.concat(items); + return BinaryUtil.concat(new byte[] {0x30}, encodeDerLength(content.length), content); + } } diff --git a/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala b/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala index b834f95b7..53b9d8835 100644 --- a/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala +++ b/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala @@ -25,6 +25,7 @@ package com.yubico.internal.util import org.junit.runner.RunWith +import org.scalacheck.Arbitrary import org.scalacheck.Gen import org.scalatest.funspec.AnyFunSpec import org.scalatest.matchers.should.Matchers @@ -149,4 +150,105 @@ class BinaryUtilSpec } } + describe("DER parsing and encoding:") { + it("encodeDerLength and parseDerLength are each other's inverse.") { + forAll( + Gen.chooseNum(0, Int.MaxValue), + Arbitrary.arbitrary[Array[Byte]], + ) { (len: Int, prefix: Array[Byte]) => + val encoded = BinaryUtil.encodeDerLength(len) + val decoded = BinaryUtil.parseDerLength(encoded, 0) + val decodedWithPrefix = BinaryUtil.parseDerLength( + BinaryUtil.concat(prefix, encoded), + prefix.length, + ) + + decoded.result should equal(len) + decoded.nextOffset should equal(encoded.length) + decodedWithPrefix.result should equal(len) + decodedWithPrefix.nextOffset should equal( + prefix.length + encoded.length + ) + + val recoded = BinaryUtil.encodeDerLength(decoded.result) + recoded should equal(encoded) + } + } + + it("parseDerLength tolerates unnecessarily long encodings.") { + BinaryUtil + .parseDerLength(Array(0x81, 0).map(_.toByte), 0) + .result should equal(0) + BinaryUtil + .parseDerLength(Array(0x82, 0, 0).map(_.toByte), 0) + .result should equal(0) + BinaryUtil + .parseDerLength(Array(0x83, 0, 0, 0).map(_.toByte), 0) + .result should equal(0) + BinaryUtil + .parseDerLength(Array(0x84, 0, 0, 0, 0).map(_.toByte), 0) + .result should equal(0) + BinaryUtil + .parseDerLength(Array(0x81, 7).map(_.toByte), 0) + .result should equal(7) + BinaryUtil + .parseDerLength(Array(0x82, 0, 7).map(_.toByte), 0) + .result should equal(7) + BinaryUtil + .parseDerLength(Array(0x83, 0, 0, 7).map(_.toByte), 0) + .result should equal(7) + BinaryUtil + .parseDerLength(Array(0x84, 0, 0, 4, 2).map(_.toByte), 0) + .result should equal(1026) + BinaryUtil + .parseDerLength(Array(0x84, 0, 1, 33, 7).map(_.toByte), 0) + .result should equal(73991) + } + + it("encodeDerSequence and parseDerSequenceEnd are (almost) each other's inverse.") { + forAll { (data: Array[Array[Byte]], prefix: Array[Byte]) => + val encoded = BinaryUtil.encodeDerSequence(data: _*) + val decoded = BinaryUtil.parseDerSequence(encoded, 0) + val encodedWithPrefix = BinaryUtil.concat(prefix, encoded) + val decodedWithPrefix = BinaryUtil.parseDerSequence( + encodedWithPrefix, + prefix.length, + ) + + val expectedContent: Array[Byte] = BinaryUtil.concat(data: _*) + decoded.result should equal(expectedContent) + decodedWithPrefix.result should equal(expectedContent) + decoded.nextOffset should equal(encoded.length) + decodedWithPrefix.nextOffset should equal( + prefix.length + encoded.length + ) + } + } + + it("parseDerSequence fails if the first byte is not 0x30.") { + forAll { (tag: Byte, data: Array[Array[Byte]]) => + whenever(tag != 0x30) { + val encoded = BinaryUtil.encodeDerSequence(data: _*) + an[IllegalArgumentException] shouldBe thrownBy { + BinaryUtil.parseDerSequence( + encoded.updated(0, tag), + 0, + ) + } + } + } + } + + it("parseDerSequence fails on empty input.") { + an[IllegalArgumentException] shouldBe thrownBy { + BinaryUtil.parseDerSequence(Array.empty, 0) + } + forAll { data: Array[Byte] => + an[IllegalArgumentException] shouldBe thrownBy { + BinaryUtil.parseDerSequence(data, data.length) + } + } + } + } + } From e60862a51909756eb0aa530c5390b6e1a58dfa50 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 11 Jul 2024 22:00:27 +0200 Subject: [PATCH 07/16] Use BinaryUtil.concat instead of ByteArray.concat where appropriate --- .../java/com/yubico/fido/metadata/AAGUID.java | 16 +- .../webauthn/RelyingPartyAssertionSpec.scala | 8 +- .../RelyingPartyRegistrationSpec.scala | 11 +- .../RelyingPartyV2AssertionSpec.scala | 8 +- .../RelyingPartyV2RegistrationSpec.scala | 11 +- .../yubico/webauthn/TestAuthenticator.scala | 153 ++++++++---------- 6 files changed, 97 insertions(+), 110 deletions(-) diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/AAGUID.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/AAGUID.java index e5adeff30..e11dac599 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/AAGUID.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/AAGUID.java @@ -2,9 +2,9 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; +import com.yubico.internal.util.BinaryUtil; import com.yubico.internal.util.ExceptionUtil; import com.yubico.webauthn.data.ByteArray; -import com.yubico.webauthn.data.exception.HexException; import java.util.regex.Matcher; import java.util.regex.Pattern; import lombok.AccessLevel; @@ -105,12 +105,14 @@ private static ByteArray parse(String value) { Matcher matcher = AAGUID_PATTERN.matcher(value); if (matcher.find()) { try { - return ByteArray.fromHex(matcher.group(1)) - .concat(ByteArray.fromHex(matcher.group(2))) - .concat(ByteArray.fromHex(matcher.group(3))) - .concat(ByteArray.fromHex(matcher.group(4))) - .concat(ByteArray.fromHex(matcher.group(5))); - } catch (HexException e) { + return new ByteArray( + BinaryUtil.concat( + BinaryUtil.fromHex(matcher.group(1)), + BinaryUtil.fromHex(matcher.group(2)), + BinaryUtil.fromHex(matcher.group(3)), + BinaryUtil.fromHex(matcher.group(4)), + BinaryUtil.fromHex(matcher.group(5)))); + } catch (Exception e) { throw new RuntimeException( "This exception should be impossible, please file a bug report.", e); } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala index ef809d116..ca9ee43c2 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala @@ -29,6 +29,7 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory import com.fasterxml.jackson.databind.node.ObjectNode import com.fasterxml.jackson.databind.node.TextNode import com.upokecenter.cbor.CBORObject +import com.yubico.internal.util.BinaryUtil import com.yubico.internal.util.JacksonCodecs import com.yubico.webauthn.data.AssertionExtensionInputs import com.yubico.webauthn.data.AuthenticatorAssertionResponse @@ -2419,13 +2420,14 @@ class RelyingPartyAssertionSpec it("a U2F-formatted public key.") { val testData = RealExamples.YubiKeyNeo.asRegistrationTestData - val x = ByteArray.fromHex( + val x = BinaryUtil.fromHex( "39C94FBBDDC694A925E6F8657C66916CFE84CD0222EDFCF281B21F5CDC347923" ) - val y = ByteArray.fromHex( + val y = BinaryUtil.fromHex( "D6B0D2021CFE1724A6FE81E3568C4FFAE339298216A30AFC18C0B975F2E2A891" ) - val u2fPubkey = ByteArray.fromHex("04").concat(x).concat(y) + val u2fPubkey = + new ByteArray(BinaryUtil.concat(BinaryUtil.fromHex("04"), x, y)) val cred1 = RegisteredCredential .builder() diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala index 49ee87a4b..de55d059e 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala @@ -1745,18 +1745,15 @@ class RelyingPartyRegistrationSpec key, COSEAlgorithmIdentifier.RS256, ) - new ByteArray( + BinaryUtil.concat( java.util.Arrays.copyOfRange( authDataBytes, 0, 32 + 1 + 4 + 16 + 2, - ) + ), + authData.getAttestedCredentialData.get.getCredentialId.getBytes, + reencodedKey.getBytes, ) - .concat( - authData.getAttestedCredentialData.get.getCredentialId - ) - .concat(reencodedKey) - .getBytes } def modifyAttobjPubkeyAlg(attObjBytes: ByteArray) diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2AssertionSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2AssertionSpec.scala index 794db38fb..8d2579d8a 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2AssertionSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2AssertionSpec.scala @@ -29,6 +29,7 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory import com.fasterxml.jackson.databind.node.ObjectNode import com.fasterxml.jackson.databind.node.TextNode import com.upokecenter.cbor.CBORObject +import com.yubico.internal.util.BinaryUtil import com.yubico.internal.util.JacksonCodecs import com.yubico.webauthn.data.AssertionExtensionInputs import com.yubico.webauthn.data.AuthenticatorAssertionResponse @@ -2511,13 +2512,14 @@ class RelyingPartyV2AssertionSpec it("a U2F-formatted public key.") { val testData = RealExamples.YubiKeyNeo.asRegistrationTestData - val x = ByteArray.fromHex( + val x = BinaryUtil.fromHex( "39C94FBBDDC694A925E6F8657C66916CFE84CD0222EDFCF281B21F5CDC347923" ) - val y = ByteArray.fromHex( + val y = BinaryUtil.fromHex( "D6B0D2021CFE1724A6FE81E3568C4FFAE339298216A30AFC18C0B975F2E2A891" ) - val u2fPubkey = ByteArray.fromHex("04").concat(x).concat(y) + val u2fPubkey = + new ByteArray(BinaryUtil.concat(BinaryUtil.fromHex("04"), x, y)) val rp = RelyingParty .builder() diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2RegistrationSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2RegistrationSpec.scala index be6274d6b..f623ac2ef 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2RegistrationSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2RegistrationSpec.scala @@ -1736,18 +1736,15 @@ class RelyingPartyV2RegistrationSpec key, COSEAlgorithmIdentifier.RS256, ) - new ByteArray( + BinaryUtil.concat( java.util.Arrays.copyOfRange( authDataBytes, 0, 32 + 1 + 4 + 16 + 2, - ) + ), + authData.getAttestedCredentialData.get.getCredentialId.getBytes, + reencodedKey.getBytes, ) - .concat( - authData.getAttestedCredentialData.get.getCredentialId - ) - .concat(reencodedKey) - .getBytes } def modifyAttobjPubkeyAlg(attObjBytes: ByteArray) diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/TestAuthenticator.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/TestAuthenticator.scala index 83f3b5606..5c5f115d2 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/TestAuthenticator.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/TestAuthenticator.scala @@ -930,26 +930,21 @@ object TestAuthenticator { case 3 => { // RSA val cose = CBORObject.DecodeFromBytes(cosePubkey.getBytes) ( - new ByteArray(BinaryUtil.encodeUint16(symmetric getOrElse 0x0010)) - .concat( - new ByteArray( - BinaryUtil.encodeUint16(scheme getOrElse TpmRsaScheme.RSASSA) - ) - ) - .concat( - new ByteArray(BinaryUtil.encodeUint16(RsaKeySizeBits)) - ) // key_bits - .concat( - new ByteArray( - BinaryUtil.encodeUint32( - new BigInteger(1, cose.get(-2).GetByteString()).longValue() - ) - ) - ) // exponent - , - new ByteArray( - BinaryUtil.encodeUint16(cose.get(-1).GetByteString().length) - ).concat(new ByteArray(cose.get(-1).GetByteString())), // modulus + BinaryUtil.concat( + BinaryUtil.encodeUint16(symmetric getOrElse 0x0010), + BinaryUtil.encodeUint16(scheme getOrElse TpmRsaScheme.RSASSA), + // key_bits + BinaryUtil.encodeUint16(RsaKeySizeBits), + // exponent + BinaryUtil.encodeUint32( + new BigInteger(1, cose.get(-2).GetByteString()).longValue() + ), + ), + BinaryUtil.concat( + BinaryUtil.encodeUint16(cose.get(-1).GetByteString().length), + // modulus + cose.get(-1).GetByteString(), + ), ) } case 2 => { // EC @@ -957,78 +952,70 @@ object TestAuthenticator { .importCosePublicKey(cosePubkey) .asInstanceOf[ECPublicKey] ( - new ByteArray(BinaryUtil.encodeUint16(symmetric getOrElse 0x0010)) - .concat( - new ByteArray(BinaryUtil.encodeUint16(scheme getOrElse 0x0010)) - ) - .concat( - new ByteArray(BinaryUtil.encodeUint16(coseKeyAlg match { - case COSEAlgorithmIdentifier.ES256 => 0x0003 - case COSEAlgorithmIdentifier.ES384 => 0x0004 - case COSEAlgorithmIdentifier.ES512 => 0x0005 - case COSEAlgorithmIdentifier.RS1 | - COSEAlgorithmIdentifier.RS256 | - COSEAlgorithmIdentifier.RS384 | - COSEAlgorithmIdentifier.RS512 | - COSEAlgorithmIdentifier.EdDSA => - ??? - })) - ) - .concat( - new ByteArray(BinaryUtil.encodeUint16(0x0010)) - ) // kdf_scheme: ??? (unused?) - , - new ByteArray( - BinaryUtil.encodeUint16(pubkey.getW.getAffineX.toByteArray.length) - ) - .concat(new ByteArray(pubkey.getW.getAffineX.toByteArray)) - .concat( - new ByteArray( - BinaryUtil.encodeUint16( - pubkey.getW.getAffineY.toByteArray.length - ) - ) - ) - .concat(new ByteArray(pubkey.getW.getAffineY.toByteArray)), + BinaryUtil.concat( + BinaryUtil.encodeUint16(symmetric getOrElse 0x0010), + BinaryUtil.encodeUint16(scheme getOrElse 0x0010), + BinaryUtil.encodeUint16(coseKeyAlg match { + case COSEAlgorithmIdentifier.ES256 => 0x0003 + case COSEAlgorithmIdentifier.ES384 => 0x0004 + case COSEAlgorithmIdentifier.ES512 => 0x0005 + case COSEAlgorithmIdentifier.RS1 | COSEAlgorithmIdentifier.RS256 | + COSEAlgorithmIdentifier.RS384 | + COSEAlgorithmIdentifier.RS512 | + COSEAlgorithmIdentifier.EdDSA => + ??? + }), + // kdf_scheme: ??? (unused?) + BinaryUtil.encodeUint16(0x0010), + ), + BinaryUtil.concat( + BinaryUtil.encodeUint16(pubkey.getW.getAffineX.toByteArray.length), + pubkey.getW.getAffineX.toByteArray, + BinaryUtil.encodeUint16( + pubkey.getW.getAffineY.toByteArray.length + ), + pubkey.getW.getAffineY.toByteArray, + ), ) } } - val pubArea = new ByteArray(BinaryUtil.encodeUint16(signAlg)) - .concat(new ByteArray(BinaryUtil.encodeUint16(hashId))) - .concat( - new ByteArray( - BinaryUtil.encodeUint32(attributes getOrElse Attributes.SIGN_ENCRYPT) - ) + val pubArea = new ByteArray( + BinaryUtil.concat( + BinaryUtil.encodeUint16(signAlg), + BinaryUtil.encodeUint16(hashId), + BinaryUtil.encodeUint32(attributes getOrElse Attributes.SIGN_ENCRYPT), + // authPolicy is ignored by TpmAttestationStatementVerifier + BinaryUtil.encodeUint16(0), + parameters, + unique, ) - .concat( - new ByteArray(BinaryUtil.encodeUint16(0)) - ) // authPolicy is ignored by TpmAttestationStatementVerifier - .concat(parameters) - .concat(unique) - - val qualifiedSigner = ByteArray.fromHex("") - val clockInfo = ByteArray.fromHex("0000000000000000111111112222222233") - val firmwareVersion = ByteArray.fromHex("0000000000000000") + ) + + val qualifiedSigner = BinaryUtil.fromHex("") + val clockInfo = BinaryUtil.fromHex("0000000000000000111111112222222233") + val firmwareVersion = BinaryUtil.fromHex("0000000000000000") val attestedName = modifyAttestedName( new ByteArray(BinaryUtil.encodeUint16(hashId)).concat(hashFunc(pubArea)) ) - val attestedQualifiedName = ByteArray.fromHex("") - - val certInfo = magic - .concat(`type`) - .concat(new ByteArray(BinaryUtil.encodeUint16(qualifiedSigner.size))) - .concat(qualifiedSigner) - .concat(new ByteArray(BinaryUtil.encodeUint16(extraData.size))) - .concat(extraData) - .concat(clockInfo) - .concat(firmwareVersion) - .concat(new ByteArray(BinaryUtil.encodeUint16(attestedName.size))) - .concat(attestedName) - .concat( - new ByteArray(BinaryUtil.encodeUint16(attestedQualifiedName.size)) + val attestedQualifiedName = BinaryUtil.fromHex("") + + val certInfo = new ByteArray( + BinaryUtil.concat( + magic.getBytes, + `type`.getBytes, + BinaryUtil.encodeUint16(qualifiedSigner.length), + qualifiedSigner, + BinaryUtil.encodeUint16(extraData.size), + extraData.getBytes, + clockInfo, + firmwareVersion, + BinaryUtil.encodeUint16(attestedName.size), + attestedName.getBytes, + BinaryUtil.encodeUint16(attestedQualifiedName.length), + attestedQualifiedName, ) - .concat(attestedQualifiedName) + ) val sig = sign(certInfo, cert.key, cert.alg) From 7c7de8f3659958da01fcba5a33cd4334901751ff Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Mon, 15 Jul 2024 10:58:25 +0200 Subject: [PATCH 08/16] Make parseDerExplicitlyTaggedContextSpecificConstructed private --- .../src/main/java/com/yubico/internal/util/BinaryUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java index 0223cf1a8..ecf68d43c 100644 --- a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java @@ -319,7 +319,7 @@ public static ParseDerResult parseDerSequence(@NonNull byte[] der, int o * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the * content octets. */ - public static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( + private static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( @NonNull byte[] der, int offset, byte tagNumber) { if (tagNumber <= 30 && tagNumber >= 0) { return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); From aa2605b09a94c0186d3b4869c8186169e2a6131a Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Mon, 15 Jul 2024 11:00:11 +0200 Subject: [PATCH 09/16] Parse X.509 CRLDistributionPoints extension Co-authored by: Dennis Fokin --- .../com/yubico/internal/util/BinaryUtil.java | 211 +++++++++++++++--- .../internal/util/CertificateParser.java | 151 +++++++++++++ .../yubico/internal/util/BinaryUtilSpec.scala | 46 ---- 3 files changed, 336 insertions(+), 72 deletions(-) diff --git a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java index ecf68d43c..037be9dec 100644 --- a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java @@ -28,9 +28,13 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.ArrayList; import java.util.Arrays; -import lombok.AllArgsConstructor; +import java.util.List; +import java.util.Optional; import lombok.NonNull; +import lombok.ToString; +import lombok.Value; public class BinaryUtil { @@ -229,10 +233,48 @@ public static byte[] encodeDerLength(final int length) { } } - @AllArgsConstructor + @ToString + public enum DerTagClass { + UNIVERSAL, + APPLICATION, + CONTEXT_SPECIFIC, + PRIVATE; + + public static DerTagClass parse(byte tag) { + switch ((tag >> 6) & 0x03) { + case 0x0: + return DerTagClass.UNIVERSAL; + case 0x1: + return DerTagClass.APPLICATION; + case 0x2: + return DerTagClass.CONTEXT_SPECIFIC; + case 0x3: + return DerTagClass.PRIVATE; + default: + throw new RuntimeException("This should be impossible"); + } + } + } + + @Value + private static class ParseDerAnyResult { + DerTagClass tagClass; + boolean constructed; + byte tagValue; + byte[] content; + int nextOffset; + } + + @Value public static class ParseDerResult { - public final T result; - public final int nextOffset; + /** The parsed value, excluding the tag-and-length header. */ + public T result; + + /** + * The offset of the first octet past the end of the parsed value. In other words, the offset to + * continue reading from. + */ + public int nextOffset; } public static ParseDerResult parseDerLength(@NonNull byte[] der, int offset) { @@ -287,46 +329,163 @@ public static ParseDerResult parseDerLength(@NonNull byte[] der, int of } } - private static ParseDerResult parseDerTagged( - @NonNull byte[] der, int offset, byte expectTag) { + private static ParseDerAnyResult parseDerAny(@NonNull byte[] der, int offset) { final int len = der.length - offset; if (len == 0) { throw new IllegalArgumentException( String.format("Empty input at offset %d: 0x%s", offset, BinaryUtil.toHex(der))); } else { final byte tag = der[offset]; - if (tag == expectTag) { - final ParseDerResult contentLen = parseDerLength(der, offset + 1); - final int contentEnd = contentLen.nextOffset + contentLen.result; - return new ParseDerResult<>( - Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), contentEnd); + final ParseDerResult contentLen = parseDerLength(der, offset + 1); + final int contentEnd = contentLen.nextOffset + contentLen.result; + return new ParseDerAnyResult( + DerTagClass.parse(tag), + (tag & 0x20) != 0, + (byte) (tag & 0x1f), + Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), + contentEnd); + } + } + + /** + * Parse a DER header with the given tag value, constructed bit and tag class, and return a copy + * of the value octets. If any of the three criteria do not match, return empty instead. + * + * @param der DER source to read from. + * @param offset The offset in der from which to start reading. + * @param expectTag The expected tag value, excluding the constructed bit and tag class. This is + * the 5 least significant bits of the tag octet. + * @param constructed The expected "constructed" bit. This is bit 6 (the third-most significant + * bit) of the tag octet. + * @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag + * octet. + * @return A copy of the value octets, if the parsed tag matches expectTag, + * constructed and expectTagClass, otherwise empty. {@link + * ParseDerResult#nextOffset} is always returned. + */ + public static ParseDerResult> parseDerTaggedOrSkip( + @NonNull byte[] der, + int offset, + byte expectTag, + boolean constructed, + DerTagClass expectTagClass) { + final ParseDerAnyResult result = parseDerAny(der, offset); + if (result.tagValue == expectTag + && result.constructed == constructed + && result.tagClass == expectTagClass) { + return new ParseDerResult<>(Optional.of(result.content), result.nextOffset); + } else { + return new ParseDerResult<>(Optional.empty(), result.nextOffset); + } + } + + /** + * Parse a DER header with the given tag value, constructed bit and tag class, and return a copy + * of the value octets. If any of the three criteria do not match, throw an {@link + * IllegalArgumentException}. + * + * @param der DER source to read from. + * @param offset The offset in der from which to start reading. + * @param expectTag The expected tag value, excluding the constructed bit and tag class. This is + * the 5 least significant bits of the tag octet. + * @param constructed The expected "constructed" bit. This is bit 6 (the third-most significant + * bit) of the tag octet. + * @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag + * octet. + * @return A copy of the value octets, if the parsed tag matches expectTag, + * constructed and expectTagClass, otherwise empty. {@link + * ParseDerResult#nextOffset} is always returned. + */ + private static ParseDerResult parseDerTagged( + @NonNull byte[] der, + int offset, + byte expectTag, + boolean constructed, + DerTagClass expectTagClass) { + final ParseDerAnyResult result = parseDerAny(der, offset); + if (result.tagValue == expectTag) { + if (result.constructed == constructed) { + if (result.tagClass == expectTagClass) { + return new ParseDerResult<>(result.content, result.nextOffset); + } else { + throw new IllegalArgumentException( + String.format( + "Incorrect tag class: expected %s, found %s at offset %d: 0x%s", + expectTagClass, result.tagClass, offset, BinaryUtil.toHex(der))); + } } else { throw new IllegalArgumentException( String.format( - "Incorrect tag: 0x%02x (expected 0x%02x) at offset %d: 0x%s", - tag, expectTag, offset, BinaryUtil.toHex(der))); + "Incorrect constructed bit: expected %s, found %s at offset %d: 0x%s", + constructed, result.constructed, offset, BinaryUtil.toHex(der))); } + } else { + throw new IllegalArgumentException( + String.format( + "Incorrect tag: expected 0x%02x, found 0x%02x at offset %d: 0x%s", + expectTag, result.tagValue, offset, BinaryUtil.toHex(der))); } } - /** Parse a SEQUENCE and return a copy of the content octets. */ - public static ParseDerResult parseDerSequence(@NonNull byte[] der, int offset) { - return parseDerTagged(der, offset, (byte) 0x30); + /** Function to parse an element of a DER SEQUENCE. */ + @FunctionalInterface + public interface ParseDerSequenceElementFunction { + /** + * Parse an element of a DER SEQUENCE. + * + * @param sequenceDer The content octets of the parent SEQUENCE. This includes ALL elements in + * the sequence. + * @param elementOffset The offset into sequenceDer from where to parse the + * element. + * @return A {@link ParseDerResult} whose {@link ParseDerResult#result} is the parsed element + * and {@link ParseDerResult#nextOffset} is the offset of the first octet past the end of + * the parsed element. + */ + ParseDerResult parse(@NonNull byte[] sequenceDer, int elementOffset); } /** - * Parse an explicitly tagged value of class "context-specific" (bits 8-7 are 0b10), in - * "constructed" encoding (bit 6 is 1), with a prescribed tag value, and return a copy of the - * content octets. + * Parse the elements of a SEQUENCE using the given element parsing function. + * + * @param der DER source array to read from + * @param offset Offset from which to begin reading the first element + * @param endOffset Offset of the first octet past the end of the sequence + * @param parseElement Function to use to parse each element in the sequence. */ - private static ParseDerResult parseDerExplicitlyTaggedContextSpecificConstructed( - @NonNull byte[] der, int offset, byte tagNumber) { - if (tagNumber <= 30 && tagNumber >= 0) { - return parseDerTagged(der, offset, (byte) ((tagNumber & 0x1f) | 0xa0)); - } else { - throw new UnsupportedOperationException( - String.format("Tag number out of range: %d (expected 0 to 30, inclusive)", tagNumber)); + public static ParseDerResult> parseDerSequenceContents( + @NonNull byte[] der, + int offset, + int endOffset, + @NonNull ParseDerSequenceElementFunction parseElement) { + List result = new ArrayList<>(); + int seqOffset = offset; + while (seqOffset < endOffset) { + ParseDerResult elementResult = parseElement.parse(der, seqOffset); + result.add(elementResult.result); + seqOffset = elementResult.nextOffset; } + return new ParseDerResult<>(result, endOffset); + } + + /** + * Parse a SEQUENCE using the given element parsing function. + * + * @param der DER source array to read from + * @param offset Offset from which to begin reading the SEQUENCE + * @param parseElement Function to use to parse each element in the sequence. + */ + public static ParseDerResult> parseDerSequence( + @NonNull byte[] der, int offset, @NonNull ParseDerSequenceElementFunction parseElement) { + final ParseDerResult seq = + parseDerTagged(der, offset, (byte) 0x10, true, DerTagClass.UNIVERSAL); + final ParseDerResult> res = + parseDerSequenceContents(seq.result, 0, seq.result.length, parseElement); + return new ParseDerResult<>(res.result, seq.nextOffset); + } + + /** Parse an Octet String. */ + public static ParseDerResult parseDerOctetString(@NonNull byte[] der, int offset) { + return parseDerTagged(der, offset, (byte) 0x04, false, DerTagClass.UNIVERSAL); } public static byte[] encodeDerObjectId(@NonNull byte[] oid) { diff --git a/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java b/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java index 1e1c72bfe..9703a773c 100755 --- a/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java @@ -26,20 +26,28 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Optional; +import lombok.Value; public class CertificateParser { public static final String ID_FIDO_GEN_CE_AAGUID = "1.3.6.1.4.1.45724.1.1.4"; + public static final String OID_CRL_DISTRIBUTION_POINTS = "2.5.29.31"; private static final Base64.Decoder BASE64_DECODER = Base64.getDecoder(); private static final List FIXSIG = @@ -164,4 +172,147 @@ public static Optional parseFidoAaguidExtension(X509Certificate cert) { }); return result; } + + @Value + public static class ParseCrlDistributionPointsExtensionResult { + /** + * The successfully parsed distribution point URLs. If the CRLDistributionPoints extension is + * not present, this will be an empty list. + */ + Collection distributionPoints; + + /** + * True if and only if the CRLDistributionPoints extension is present and contains anything that + * is not a distributionPoint [0] DistributionPointName containing a + * fullName [0] GeneralNames containing exactly one + * uniformResourceIdentifier [6] IA5String + */ + boolean anyDistributionPointUnsupported; + } + + public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPointsExtension( + X509Certificate cert) { + final byte[] crldpExtension = cert.getExtensionValue(OID_CRL_DISTRIBUTION_POINTS); + if (crldpExtension != null) { + BinaryUtil.ParseDerResult octetString = + BinaryUtil.parseDerOctetString(crldpExtension, 0); + try { + BinaryUtil.ParseDerResult>>>> distributionPoints = + BinaryUtil.parseDerSequence( + octetString.result, + 0, + (outerSequenceDer, distributionPointOffset) -> + BinaryUtil.parseDerSequence( + outerSequenceDer, + distributionPointOffset, + (innerSequenceDer, distributionPointChoiceOffset) -> { + // DistributionPoint ::= SEQUENCE { + // distributionPoint [0] DistributionPointName OPTIONAL, + final BinaryUtil.ParseDerResult> dpElement = + BinaryUtil.parseDerTaggedOrSkip( + innerSequenceDer, + distributionPointChoiceOffset, + (byte) 0, + true, + BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); + if (dpElement.result.isPresent()) { + + // DistributionPointName ::= CHOICE { + // fullName [0] GeneralNames, + final BinaryUtil.ParseDerResult> dpNameElement = + BinaryUtil.parseDerTaggedOrSkip( + dpElement.result.get(), + 0, + (byte) 0, + true, + BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); + + if (dpNameElement.result.isPresent()) { + return BinaryUtil.parseDerSequenceContents( + dpNameElement.result.get(), + 0, + dpNameElement.result.get().length, + (generalNamesDer, generalNamesElementOffset) -> { + // fullName [0] GeneralNames, + // GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName + // GeneralName ::= CHOICE { + // uniformResourceIdentifier [6] IA5String, + // + // GeneralNames is defined in RFC 5280 appendix 2 which uses + // IMPLICIT tagging + // https://datatracker.ietf.org/doc/html/rfc5280#appendix-A.2 + // so the SEQUENCE tag in GeneralNames is implicit. + // The IA5String tag is also implicit from the CHOICE tag. + final BinaryUtil.ParseDerResult> generalName = + BinaryUtil.parseDerTaggedOrSkip( + generalNamesDer, + generalNamesElementOffset, + (byte) 6, + false, + BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); + if (generalName.result.isPresent()) { + String uriString = + new String( + generalName.result.get(), StandardCharsets.US_ASCII); + try { + return new BinaryUtil.ParseDerResult<>( + Optional.of(new URL(uriString)), + generalName.nextOffset); + } catch (MalformedURLException e) { + throw new IllegalArgumentException( + String.format( + "Invalid URL in CRLDistributionPoints: %s", + uriString), + e); + } + } else { + return new BinaryUtil.ParseDerResult<>( + Optional.empty(), generalName.nextOffset); + } + }); + } + } + + // Ignore all other forms of distribution points + return new BinaryUtil.ParseDerResult<>( + Collections.emptyList(), dpElement.nextOffset); + })); + + return distributionPoints.result.stream() + .flatMap(Collection::stream) + .flatMap(Collection::stream) + .reduce( + new ParseCrlDistributionPointsExtensionResult(new ArrayList<>(), false), + (result, next) -> { + if (next.isPresent()) { + List dp = new ArrayList<>(result.distributionPoints); + dp.add(next.get()); + return new ParseCrlDistributionPointsExtensionResult( + dp, result.anyDistributionPointUnsupported); + } else { + return new ParseCrlDistributionPointsExtensionResult( + result.distributionPoints, true); + } + }, + (resultA, resultB) -> { + List dp = new ArrayList<>(resultA.distributionPoints); + dp.addAll(resultB.distributionPoints); + return new ParseCrlDistributionPointsExtensionResult( + dp, + resultA.anyDistributionPointUnsupported + || resultB.anyDistributionPointUnsupported); + }); + + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + String.format( + "X.509 extension %s (id-ce-cRLDistributionPoints) is incorrectly encoded.", + OID_CRL_DISTRIBUTION_POINTS), + e); + } + + } else { + return new ParseCrlDistributionPointsExtensionResult(Collections.emptySet(), false); + } + } } diff --git a/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala b/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala index 53b9d8835..190286649 100644 --- a/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala +++ b/yubico-util/src/test/scala/com/yubico/internal/util/BinaryUtilSpec.scala @@ -204,51 +204,5 @@ class BinaryUtilSpec .parseDerLength(Array(0x84, 0, 1, 33, 7).map(_.toByte), 0) .result should equal(73991) } - - it("encodeDerSequence and parseDerSequenceEnd are (almost) each other's inverse.") { - forAll { (data: Array[Array[Byte]], prefix: Array[Byte]) => - val encoded = BinaryUtil.encodeDerSequence(data: _*) - val decoded = BinaryUtil.parseDerSequence(encoded, 0) - val encodedWithPrefix = BinaryUtil.concat(prefix, encoded) - val decodedWithPrefix = BinaryUtil.parseDerSequence( - encodedWithPrefix, - prefix.length, - ) - - val expectedContent: Array[Byte] = BinaryUtil.concat(data: _*) - decoded.result should equal(expectedContent) - decodedWithPrefix.result should equal(expectedContent) - decoded.nextOffset should equal(encoded.length) - decodedWithPrefix.nextOffset should equal( - prefix.length + encoded.length - ) - } - } - - it("parseDerSequence fails if the first byte is not 0x30.") { - forAll { (tag: Byte, data: Array[Array[Byte]]) => - whenever(tag != 0x30) { - val encoded = BinaryUtil.encodeDerSequence(data: _*) - an[IllegalArgumentException] shouldBe thrownBy { - BinaryUtil.parseDerSequence( - encoded.updated(0, tag), - 0, - ) - } - } - } - } - - it("parseDerSequence fails on empty input.") { - an[IllegalArgumentException] shouldBe thrownBy { - BinaryUtil.parseDerSequence(Array.empty, 0) - } - forAll { data: Array[Byte] => - an[IllegalArgumentException] shouldBe thrownBy { - BinaryUtil.parseDerSequence(data, data.length) - } - } - } } - } From 5b8c7582894962536e13658dcbd2aecccf5ce4af Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 31 Oct 2024 21:00:51 +0100 Subject: [PATCH 10/16] Extract method FidoMetadataDownloader.fetchHeaderCertChain --- .../fido/metadata/FidoMetadataDownloader.java | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java index cf3dfd5cd..0dffab20f 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java @@ -1097,34 +1097,7 @@ private MetadataBLOB verifyBlob(ParseResult parseResult, X509Certificate trustRo InvalidAlgorithmParameterException, FidoMetadataDownloaderException { final MetadataBLOBHeader header = parseResult.blob.getHeader(); - - final List certChain; - if (header.getX5u().isPresent()) { - final URL x5u = header.getX5u().get(); - if (blobUrl != null - && (!(x5u.getHost().equals(blobUrl.getHost()) - && x5u.getProtocol().equals(blobUrl.getProtocol()) - && x5u.getPort() == blobUrl.getPort()))) { - throw new IllegalArgumentException( - String.format( - "x5u in BLOB header must have same origin as the URL the BLOB was downloaded from. Expected origin of: %s ; found: %s", - blobUrl, x5u)); - } - List certs = new ArrayList<>(); - for (String pem : - new String(download(x5u).getBytes(), StandardCharsets.UTF_8) - .trim() - .split("\\n+-----END CERTIFICATE-----\\n+-----BEGIN CERTIFICATE-----\\n+")) { - X509Certificate x509Certificate = CertificateParser.parsePem(pem); - certs.add(x509Certificate); - } - certChain = certs; - } else if (header.getX5c().isPresent()) { - certChain = header.getX5c().get(); - } else { - certChain = Collections.singletonList(trustRootCertificate); - } - + final List certChain = fetchHeaderCertChain(trustRootCertificate, header); final X509Certificate leafCert = certChain.get(0); final Signature signature; @@ -1209,4 +1182,35 @@ private static class ParseResult { private ByteArray jwtPayload; private ByteArray jwtSignature; } + + /** Parse the header cert chain and download any certificates as necessary. */ + List fetchHeaderCertChain( + X509Certificate trustRootCertificate, MetadataBLOBHeader header) + throws IOException, CertificateException { + if (header.getX5u().isPresent()) { + final URL x5u = header.getX5u().get(); + if (blobUrl != null + && (!(x5u.getHost().equals(blobUrl.getHost()) + && x5u.getProtocol().equals(blobUrl.getProtocol()) + && x5u.getPort() == blobUrl.getPort()))) { + throw new IllegalArgumentException( + String.format( + "x5u in BLOB header must have same origin as the URL the BLOB was downloaded from. Expected origin of: %s ; found: %s", + blobUrl, x5u)); + } + List certs = new ArrayList<>(); + for (String pem : + new String(download(x5u).getBytes(), StandardCharsets.UTF_8) + .trim() + .split("\\n+-----END CERTIFICATE-----\\n+-----BEGIN CERTIFICATE-----\\n+")) { + X509Certificate x509Certificate = CertificateParser.parsePem(pem); + certs.add(x509Certificate); + } + return certs; + } else if (header.getX5c().isPresent()) { + return header.getX5c().get(); + } else { + return Collections.singletonList(trustRootCertificate); + } + } } From efa4b009d88c3788fa19acdae407a816aeb6746a Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 31 Oct 2024 21:01:50 +0100 Subject: [PATCH 11/16] Fetch CRLDistributionPoints in FidoMetadataDownloader --- webauthn-server-attestation/build.gradle.kts | 10 ++- ...idoMetadataDownloaderIntegrationTest.scala | 43 ++++++++++- .../fido/metadata/FidoMetadataDownloader.java | 75 ++++++++++++++++++- 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/webauthn-server-attestation/build.gradle.kts b/webauthn-server-attestation/build.gradle.kts index 1748835d2..590e42ad7 100644 --- a/webauthn-server-attestation/build.gradle.kts +++ b/webauthn-server-attestation/build.gradle.kts @@ -48,7 +48,12 @@ dependencies { testImplementation("org.scalatestplus:junit-4-13_2.13") testImplementation("org.scalatestplus:scalacheck-1-16_2.13") - testImplementation("org.slf4j:slf4j-api") + testImplementation("org.slf4j:slf4j-api") { + version { + strictly("[1.7.25,1.8-a)") // Pre-1.8 version required by slf4j-test + } + } + testRuntimeOnly("uk.org.lidalia:slf4j-test") } val integrationTest = task("integrationTest") { @@ -58,9 +63,6 @@ val integrationTest = task("integrationTest") { testClassesDirs = sourceSets["integrationTest"].output.classesDirs classpath = sourceSets["integrationTest"].runtimeClasspath shouldRunAfter(tasks.test) - - // Required for processing CRL distribution points extension - systemProperty("com.sun.security.enableCRLDP", "true") } tasks["check"].dependsOn(integrationTest) diff --git a/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala b/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala index 937a0db8c..a2d01fc09 100644 --- a/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala +++ b/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala @@ -1,5 +1,7 @@ package com.yubico.fido.metadata +import com.yubico.internal.util.CertificateParser +import com.yubico.webauthn.data.ByteArray import org.junit.runner.RunWith import org.scalatest.BeforeAndAfter import org.scalatest.funspec.AnyFunSpec @@ -8,7 +10,8 @@ import org.scalatest.tags.Network import org.scalatest.tags.Slow import org.scalatestplus.junit.JUnitRunner -import java.util.Optional +import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.OptionConverters.RichOption import scala.util.Success import scala.util.Try @@ -21,6 +24,9 @@ class FidoMetadataDownloaderIntegrationTest with BeforeAndAfter { describe("FidoMetadataDownloader with default settings") { + // Cache downloaded items to avoid cause unnecessary load on remote servers + var trustRootCache: Option[ByteArray] = None + var blobCache: Option[ByteArray] = None val downloader = FidoMetadataDownloader .builder() @@ -28,17 +34,46 @@ class FidoMetadataDownloaderIntegrationTest "Retrieval and use of this BLOB indicates acceptance of the appropriate agreement located at https://fidoalliance.org/metadata/metadata-legal-terms/" ) .useDefaultTrustRoot() - .useTrustRootCache(() => Optional.empty(), _ => {}) + .useTrustRootCache( + () => trustRootCache.toJava, + trustRoot => { trustRootCache = Some(trustRoot) }, + ) .useDefaultBlob() - .useBlobCache(() => Optional.empty(), _ => {}) + .useBlobCache( + () => blobCache.toJava, + blob => { blobCache = Some(blob) }, + ) .build() it("downloads and verifies the root cert and BLOB successfully.") { - // This test requires the system property com.sun.security.enableCRLDP=true val blob = Try(downloader.loadCachedBlob) blob shouldBe a[Success[_]] blob.get should not be null } + + it( + "does not encounter any CRLDistributionPoints entries in unknown format." + ) { + val blob = Try(downloader.loadCachedBlob) + blob shouldBe a[Success[_]] + val trustRootCert = + CertificateParser.parseDer(trustRootCache.get.getBytes) + val certChain = downloader + .fetchHeaderCertChain( + trustRootCert, + FidoMetadataDownloader.parseBlob(blobCache.get).getBlob.getHeader, + ) + .asScala :+ trustRootCert + for { cert <- certChain } { + withClue( + s"Unknown CRLDistributionPoints structure in cert [${cert.getSubjectX500Principal}] : ${new ByteArray(cert.getEncoded)}" + ) { + CertificateParser + .parseCrlDistributionPointsExtension(cert) + .isAnyDistributionPointUnsupported should be(false) + } + } + } } } diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java index 0dffab20f..90d550285 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java @@ -29,6 +29,7 @@ import com.yubico.fido.metadata.FidoMetadataDownloaderException.Reason; import com.yubico.internal.util.BinaryUtil; import com.yubico.internal.util.CertificateParser; +import com.yubico.internal.util.OptionalUtil; import com.yubico.webauthn.data.ByteArray; import com.yubico.webauthn.data.exception.Base64UrlException; import com.yubico.webauthn.data.exception.HexException; @@ -54,6 +55,7 @@ import java.security.Signature; import java.security.SignatureException; import java.security.cert.CRL; +import java.security.cert.CRLException; import java.security.cert.CertPath; import java.security.cert.CertPathValidator; import java.security.cert.CertPathValidatorException; @@ -1131,13 +1133,18 @@ private MetadataBLOB verifyBlob(ParseResult parseResult, X509Certificate trustRo if (certStore != null) { pathParams.addCertStore(certStore); } + + // Parse CRLDistributionPoints ourselves so users don't have to set the + // `com.sun.security.enableCRLDP=true` system property + fetchCrlDistributionPoints(certChain, certFactory).ifPresent(pathParams::addCertStore); + pathParams.setDate(Date.from(clock.instant())); cpv.validate(blobCertPath, pathParams); return parseResult.blob; } - private static ParseResult parseBlob(ByteArray jwt) throws IOException, Base64UrlException { + static ParseResult parseBlob(ByteArray jwt) throws IOException, Base64UrlException { Scanner s = new Scanner(new ByteArrayInputStream(jwt.getBytes())).useDelimiter("\\."); final ByteArray jwtHeader = ByteArray.fromBase64Url(s.next()); final ByteArray jwtPayload = ByteArray.fromBase64Url(s.next()); @@ -1176,7 +1183,7 @@ private static ByteArray verifyHash(ByteArray contents, Set acceptedC } @Value - private static class ParseResult { + static class ParseResult { private MetadataBLOB blob; private ByteArray jwtHeader; private ByteArray jwtPayload; @@ -1213,4 +1220,68 @@ List fetchHeaderCertChain( return Collections.singletonList(trustRootCertificate); } } + + /** + * Parse the CRLDistributionPoints extension of each certificate, fetch each distribution point + * and assemble them into a {@link CertStore} ready to be injected into {@link + * PKIXParameters#addCertStore(CertStore)} to provide CRLs for the verification procedure. + * + *

We do this ourselves so that users don't have to set the `com.sun.security.enableCRLDP=true` + * system property. This is required by the default SUN provider in order to enable + * CRLDistributionPoints resolution. + * + *

Any CRLDistributionPoints entries in unknown format are ignored and log a warning. + */ + private Optional fetchCrlDistributionPoints( + List certChain, CertificateFactory certFactory) + throws InvalidAlgorithmParameterException, NoSuchAlgorithmException { + final List crlDistributionPointUrls = + certChain.stream() + .flatMap( + cert -> { + log.debug( + "Attempting to parse CRLDistributionPoints extension of cert: {}", + cert.getSubjectX500Principal()); + try { + return CertificateParser.parseCrlDistributionPointsExtension(cert) + .getDistributionPoints() + .stream(); + } catch (Exception e) { + log.warn( + "Failed to parse CRLDistributionPoints extension of cert: {}", + cert.getSubjectX500Principal(), + e); + return Stream.empty(); + } + }) + .collect(Collectors.toList()); + + if (crlDistributionPointUrls.isEmpty()) { + return Optional.empty(); + + } else { + final List crldpCrls = + crlDistributionPointUrls.stream() + .map( + crldpUrl -> { + log.debug("Attempting to download CRL distribution point: {}", crldpUrl); + try { + return Optional.of( + certFactory.generateCRL( + new ByteArrayInputStream(download(crldpUrl).getBytes()))); + } catch (CRLException e) { + log.warn("Failed to import CRL from distribution point: {}", crldpUrl, e); + return Optional.empty(); + } catch (Exception e) { + log.warn("Failed to download CRL distribution point: {}", crldpUrl, e); + return Optional.empty(); + } + }) + .flatMap(OptionalUtil::stream) + .collect(Collectors.toList()); + + return Optional.of( + CertStore.getInstance("Collection", new CollectionCertStoreParameters(crldpCrls))); + } + } } From 811cd63f5b5e6cbb8ffb84ec282dc3e8c7ae41b2 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 31 Oct 2024 21:30:58 +0100 Subject: [PATCH 12/16] Return offsets instead of byte array copies during DER parsing --- .../com/yubico/internal/util/BinaryUtil.java | 42 ++++++++------ .../internal/util/CertificateParser.java | 56 ++++++++++--------- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java index 037be9dec..c34858dbc 100644 --- a/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java @@ -261,8 +261,8 @@ private static class ParseDerAnyResult { DerTagClass tagClass; boolean constructed; byte tagValue; - byte[] content; - int nextOffset; + int valueStart; + int valueEnd; } @Value @@ -342,14 +342,15 @@ private static ParseDerAnyResult parseDerAny(@NonNull byte[] der, int offset) { DerTagClass.parse(tag), (tag & 0x20) != 0, (byte) (tag & 0x1f), - Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd), + contentLen.nextOffset, contentEnd); } } /** - * Parse a DER header with the given tag value, constructed bit and tag class, and return a copy - * of the value octets. If any of the three criteria do not match, return empty instead. + * Parse a DER header with the given tag value, constructed bit and tag class, and return the + * start and end offsets of the value octets. If any of the three criteria do not match, return + * empty instead. * * @param der DER source to read from. * @param offset The offset in der from which to start reading. @@ -359,11 +360,12 @@ private static ParseDerAnyResult parseDerAny(@NonNull byte[] der, int offset) { * bit) of the tag octet. * @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag * octet. - * @return A copy of the value octets, if the parsed tag matches expectTag, + * @return The start and end offsets of the value octets, if the parsed tag matches + * expectTag, * constructed and expectTagClass, otherwise empty. {@link * ParseDerResult#nextOffset} is always returned. */ - public static ParseDerResult> parseDerTaggedOrSkip( + public static ParseDerResult> parseDerTaggedOrSkip( @NonNull byte[] der, int offset, byte expectTag, @@ -373,16 +375,16 @@ public static ParseDerResult> parseDerTaggedOrSkip( if (result.tagValue == expectTag && result.constructed == constructed && result.tagClass == expectTagClass) { - return new ParseDerResult<>(Optional.of(result.content), result.nextOffset); + return new ParseDerResult<>(Optional.of(result.valueStart), result.valueEnd); } else { - return new ParseDerResult<>(Optional.empty(), result.nextOffset); + return new ParseDerResult<>(Optional.empty(), result.valueEnd); } } /** - * Parse a DER header with the given tag value, constructed bit and tag class, and return a copy - * of the value octets. If any of the three criteria do not match, throw an {@link - * IllegalArgumentException}. + * Parse a DER header with the given tag value, constructed bit and tag class, and return the + * start and end offsets of the value octets. If any of the three criteria do not match, throw an + * {@link IllegalArgumentException}. * * @param der DER source to read from. * @param offset The offset in der from which to start reading. @@ -392,11 +394,12 @@ public static ParseDerResult> parseDerTaggedOrSkip( * bit) of the tag octet. * @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag * octet. - * @return A copy of the value octets, if the parsed tag matches expectTag, + * @return The start and end offsets of the value octets, if the parsed tag matches + * expectTag, * constructed and expectTagClass, otherwise empty. {@link * ParseDerResult#nextOffset} is always returned. */ - private static ParseDerResult parseDerTagged( + private static ParseDerResult parseDerTagged( @NonNull byte[] der, int offset, byte expectTag, @@ -406,7 +409,7 @@ private static ParseDerResult parseDerTagged( if (result.tagValue == expectTag) { if (result.constructed == constructed) { if (result.tagClass == expectTagClass) { - return new ParseDerResult<>(result.content, result.nextOffset); + return new ParseDerResult<>(result.valueStart, result.valueEnd); } else { throw new IllegalArgumentException( String.format( @@ -476,16 +479,19 @@ public static ParseDerResult> parseDerSequenceContents( */ public static ParseDerResult> parseDerSequence( @NonNull byte[] der, int offset, @NonNull ParseDerSequenceElementFunction parseElement) { - final ParseDerResult seq = + final ParseDerResult seq = parseDerTagged(der, offset, (byte) 0x10, true, DerTagClass.UNIVERSAL); final ParseDerResult> res = - parseDerSequenceContents(seq.result, 0, seq.result.length, parseElement); + parseDerSequenceContents(der, seq.result, seq.nextOffset, parseElement); return new ParseDerResult<>(res.result, seq.nextOffset); } /** Parse an Octet String. */ public static ParseDerResult parseDerOctetString(@NonNull byte[] der, int offset) { - return parseDerTagged(der, offset, (byte) 0x04, false, DerTagClass.UNIVERSAL); + ParseDerResult res = + parseDerTagged(der, offset, (byte) 0x04, false, DerTagClass.UNIVERSAL); + return new ParseDerResult<>( + Arrays.copyOfRange(der, res.result, res.nextOffset), res.nextOffset); } public static byte[] encodeDerObjectId(@NonNull byte[] oid) { diff --git a/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java b/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java index 9703a773c..f0314f0c8 100755 --- a/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java +++ b/yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java @@ -208,30 +208,31 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin (innerSequenceDer, distributionPointChoiceOffset) -> { // DistributionPoint ::= SEQUENCE { // distributionPoint [0] DistributionPointName OPTIONAL, - final BinaryUtil.ParseDerResult> dpElement = + final BinaryUtil.ParseDerResult> dpElementOffsets = BinaryUtil.parseDerTaggedOrSkip( innerSequenceDer, distributionPointChoiceOffset, (byte) 0, true, BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); - if (dpElement.result.isPresent()) { + if (dpElementOffsets.result.isPresent()) { // DistributionPointName ::= CHOICE { // fullName [0] GeneralNames, - final BinaryUtil.ParseDerResult> dpNameElement = - BinaryUtil.parseDerTaggedOrSkip( - dpElement.result.get(), - 0, - (byte) 0, - true, - BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); + final BinaryUtil.ParseDerResult> + dpNameElementOffsets = + BinaryUtil.parseDerTaggedOrSkip( + innerSequenceDer, + dpElementOffsets.result.get(), + (byte) 0, + true, + BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); - if (dpNameElement.result.isPresent()) { + if (dpNameElementOffsets.result.isPresent()) { return BinaryUtil.parseDerSequenceContents( - dpNameElement.result.get(), - 0, - dpNameElement.result.get().length, + innerSequenceDer, + dpNameElementOffsets.result.get(), + dpNameElementOffsets.nextOffset, (generalNamesDer, generalNamesElementOffset) -> { // fullName [0] GeneralNames, // GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName @@ -243,21 +244,26 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin // https://datatracker.ietf.org/doc/html/rfc5280#appendix-A.2 // so the SEQUENCE tag in GeneralNames is implicit. // The IA5String tag is also implicit from the CHOICE tag. - final BinaryUtil.ParseDerResult> generalName = - BinaryUtil.parseDerTaggedOrSkip( - generalNamesDer, - generalNamesElementOffset, - (byte) 6, - false, - BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); - if (generalName.result.isPresent()) { + final BinaryUtil.ParseDerResult> + generalNameOffsets = + BinaryUtil.parseDerTaggedOrSkip( + generalNamesDer, + generalNamesElementOffset, + (byte) 6, + false, + BinaryUtil.DerTagClass.CONTEXT_SPECIFIC); + if (generalNameOffsets.result.isPresent()) { String uriString = new String( - generalName.result.get(), StandardCharsets.US_ASCII); + Arrays.copyOfRange( + generalNamesDer, + generalNameOffsets.result.get(), + generalNameOffsets.nextOffset), + StandardCharsets.US_ASCII); try { return new BinaryUtil.ParseDerResult<>( Optional.of(new URL(uriString)), - generalName.nextOffset); + generalNameOffsets.nextOffset); } catch (MalformedURLException e) { throw new IllegalArgumentException( String.format( @@ -267,7 +273,7 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin } } else { return new BinaryUtil.ParseDerResult<>( - Optional.empty(), generalName.nextOffset); + Optional.empty(), generalNameOffsets.nextOffset); } }); } @@ -275,7 +281,7 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin // Ignore all other forms of distribution points return new BinaryUtil.ParseDerResult<>( - Collections.emptyList(), dpElement.nextOffset); + Collections.emptyList(), dpElementOffsets.nextOffset); })); return distributionPoints.result.stream() From fc6d4251a4cb3621c74ddb05c69f9a9d5a7dafcb Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 12 Dec 2024 20:35:03 +0100 Subject: [PATCH 13/16] Update JavaDoc to reflect CRLDistributionPoints improvements --- .../fido/metadata/FidoMetadataDownloader.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java index 90d550285..2a0cfbc0e 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java @@ -545,9 +545,9 @@ public FidoMetadataDownloaderBuilder clock(@NonNull Clock clock) { /** * Use the provided CRLs. * - *

CRLs will also be downloaded from distribution points if the - * com.sun.security.enableCRLDP system property is set to true (assuming the - * use of the {@link CertPathValidator} implementation from the SUN provider). + *

CRLs will also be downloaded from distribution points for any certificates with a + * CRLDistributionPoints extension, if the extension can be successfully interpreted. A warning + * message will be logged CRLDistributionPoints parsing fails. * * @throws InvalidAlgorithmParameterException if {@link CertStore#getInstance(String, * CertStoreParameters)} does. @@ -563,9 +563,9 @@ public FidoMetadataDownloaderBuilder useCrls(@NonNull Collection crls) /** * Use CRLs in the provided {@link CertStore}. * - *

CRLs will also be downloaded from distribution points if the - * com.sun.security.enableCRLDP system property is set to true (assuming the - * use of the {@link CertPathValidator} implementation from the SUN provider). + *

CRLs will also be downloaded from distribution points for any certificates with a + * CRLDistributionPoints extension, if the extension can be successfully interpreted. A warning + * message will be logged CRLDistributionPoints parsing fails. * * @see #useCrls(Collection) */ @@ -693,7 +693,7 @@ public FidoMetadataDownloaderBuilder verifyDownloadsOnly(final boolean verifyDow * @throws InvalidAlgorithmParameterException if certificate path validation fails. * @throws InvalidKeyException if signature verification fails. * @throws NoSuchAlgorithmException if signature verification fails, or if the SHA-256 algorithm - * is not available. + * or the "Collection" type {@link CertStore} is not available. * @throws SignatureException if signature verification fails. * @throws UnexpectedLegalHeader if the downloaded BLOB (if any) contains a "legalHeader" * value not configured in {@link @@ -796,7 +796,7 @@ public MetadataBLOB loadCachedBlob() * @throws InvalidAlgorithmParameterException if certificate path validation fails. * @throws InvalidKeyException if signature verification fails. * @throws NoSuchAlgorithmException if signature verification fails, or if the SHA-256 algorithm - * is not available. + * or the "Collection" type {@link CertStore} is not available. * @throws SignatureException if signature verification fails. * @throws UnexpectedLegalHeader if the downloaded BLOB (if any) contains a "legalHeader" * value not configured in {@link @@ -968,7 +968,8 @@ private X509Certificate retrieveTrustRootCert() * @throws IOException on failure to parse the BLOB contents. * @throws InvalidAlgorithmParameterException if certificate path validation fails. * @throws InvalidKeyException if signature verification fails. - * @throws NoSuchAlgorithmException if signature verification fails. + * @throws NoSuchAlgorithmException if signature verification fails, or if the SHA-256 algorithm + * or the "Collection" type {@link CertStore} is not available. * @throws SignatureException if signature verification fails. * @throws FidoMetadataDownloaderException if the explicitly configured BLOB (if any) has a bad * signature. From 25ccfc2478eb7b248fe58d3202450e65dd225f0d Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 12 Dec 2024 20:35:20 +0100 Subject: [PATCH 14/16] Fix code snippet syntax in JavaDoc --- .../com/yubico/fido/metadata/FidoMetadataDownloader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java index 2a0cfbc0e..4ac9e6641 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java @@ -1227,9 +1227,9 @@ List fetchHeaderCertChain( * and assemble them into a {@link CertStore} ready to be injected into {@link * PKIXParameters#addCertStore(CertStore)} to provide CRLs for the verification procedure. * - *

We do this ourselves so that users don't have to set the `com.sun.security.enableCRLDP=true` - * system property. This is required by the default SUN provider in order to enable - * CRLDistributionPoints resolution. + *

We do this ourselves so that users don't have to set the + * com.sun.security.enableCRLDP=true system property. This is required by the default SUN + * provider in order to enable CRLDistributionPoints resolution. * *

Any CRLDistributionPoints entries in unknown format are ignored and log a warning. */ From 4adf4cae20d394c4f6cb4719f7c31c946793ff86 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 12 Dec 2024 20:36:20 +0100 Subject: [PATCH 15/16] Remove documentation of com.sun.security.enableCRLDP setting --- webauthn-server-attestation/README.adoc | 11 ----------- webauthn-server-demo/build.gradle.kts | 3 --- 2 files changed, 14 deletions(-) diff --git a/webauthn-server-attestation/README.adoc b/webauthn-server-attestation/README.adoc index f0a86db7a..0307e645f 100644 --- a/webauthn-server-attestation/README.adoc +++ b/webauthn-server-attestation/README.adoc @@ -222,17 +222,6 @@ RegistrationResult result = rp.finishRegistration(/* ... */); Set metadata = mds.findEntries(result); ---------- - 5. If you use the SUN provider for the `PKIX` certificate path validation algorithm, which many deployments do by default: - set the `com.sun.security.enableCRLDP` system property to `true`. - This is required for the SUN `PKIX` provider to support the CRL Distribution Points extension, - which is needed in order to verify the BLOB signature. -+ -For example, this can be done on the JVM command line using a `-Dcom.sun.security.enableCRLDP=true` option. -See the https://docs.oracle.com/javase/9/security/java-pki-programmers-guide.htm#GUID-EB250086-0AC1-4D60-AE2A-FC7461374746__SECTION-139-623E860E[Java PKI Programmers Guide] -for details. -+ -This step may not be necessary if you use a different provider for the `PKIX` certificate path validation algorithm. - == Selecting trusted authenticators diff --git a/webauthn-server-demo/build.gradle.kts b/webauthn-server-demo/build.gradle.kts index 82830c46e..7a77b2347 100644 --- a/webauthn-server-demo/build.gradle.kts +++ b/webauthn-server-demo/build.gradle.kts @@ -55,9 +55,6 @@ dependencies { application { mainClass.set("demo.webauthn.EmbeddedServer") - - // Required for processing CRL distribution points extension - applicationDefaultJvmArgs = listOf("-Dcom.sun.security.enableCRLDP=true") } for (task in listOf(tasks.installDist, tasks.distZip, tasks.distTar)) { From b28e10ecb1cc46a5934667751f599914c126e809 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 12 Dec 2024 20:50:55 +0100 Subject: [PATCH 16/16] Add CRLDistributionPoints feature to NEWS --- NEWS | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/NEWS b/NEWS index 341090cb6..365fad2e6 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ == Version 2.6.0 (unreleased) == +`webauthn-server-core`: + New features: * Added method `getParsedPublicKey(): java.security.PublicKey` to @@ -61,6 +63,14 @@ New features: version increase. * (Experimental) Added `credProps` extension to assertion extension outputs. +`webauthn-server-attestation`: + +New features: + +* `FidoMetadataDownloader` now parses the CRLDistributionPoints extension on the + application level, so the `com.sun.security.enableCRLDP=true` system property + setting is no longer necessary. + == Version 2.5.4 ==