From ae33e5941bb88d88538d0a6b19ca0b01e6c76dcf Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Tue, 14 Jan 2025 14:18:46 +0000 Subject: [PATCH] Merge commit from fork * Throw error instead of halting when the encoding is invalid Signed-off-by: Cory Benfield * Avoid extra precondition failure --------- Signed-off-by: Cory Benfield Co-authored-by: Nicolas Bachschmidt --- .../Basic ASN1 Types/ASN1BitString.swift | 2 +- .../Basic ASN1 Types/ASN1Integer.swift | 4 +- .../Basic ASN1 Types/ASN1OctetString.swift | 2 +- .../Basic ASN1 Types/GeneralizedTime.swift | 12 +--- .../Basic ASN1 Types/ObjectIdentifier.swift | 2 +- .../SwiftASN1/Basic ASN1 Types/UTCTime.swift | 12 +--- Sources/SwiftASN1/DER.swift | 5 +- Tests/SwiftASN1Tests/ASN1Tests.swift | 61 +++++++++++++++++++ 8 files changed, 75 insertions(+), 25 deletions(-) diff --git a/Sources/SwiftASN1/Basic ASN1 Types/ASN1BitString.swift b/Sources/SwiftASN1/Basic ASN1 Types/ASN1BitString.swift index dbe1525..3613e14 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/ASN1BitString.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/ASN1BitString.swift @@ -57,7 +57,7 @@ public struct ASN1BitString: DERImplicitlyTaggable, BERImplicitlyTaggable { } guard case .primitive(let content) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") + throw ASN1Error.invalidASN1Object(reason: "ASN1BitString encoded with constructed encoding") } // The initial octet explains how many of the bits in the _final_ octet are not part of the bitstring. diff --git a/Sources/SwiftASN1/Basic ASN1 Types/ASN1Integer.swift b/Sources/SwiftASN1/Basic ASN1 Types/ASN1Integer.swift index ef850e5..3104f25 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/ASN1Integer.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/ASN1Integer.swift @@ -53,7 +53,7 @@ extension ASN1IntegerRepresentable { } guard case .primitive(var dataBytes) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") + throw ASN1Error.invalidASN1Object(reason: "INTEGER encoded with constructed encoding") } // Zero bytes of integer is not an acceptable encoding. @@ -93,7 +93,7 @@ extension ASN1IntegerRepresentable { } guard case .primitive(var dataBytes) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") + throw ASN1Error.invalidASN1Object(reason: "INTEGER encoded with constructed encoding") } // Zero bytes of integer is not an acceptable encoding. diff --git a/Sources/SwiftASN1/Basic ASN1 Types/ASN1OctetString.swift b/Sources/SwiftASN1/Basic ASN1 Types/ASN1OctetString.swift index 4e797d7..996ddf3 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/ASN1OctetString.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/ASN1OctetString.swift @@ -30,7 +30,7 @@ public struct ASN1OctetString: DERImplicitlyTaggable, BERImplicitlyTaggable { } guard case .primitive(let content) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") + throw ASN1Error.invalidASN1Object(reason: "ASN1OctetString encoded with constructed encoding") } self.bytes = content diff --git a/Sources/SwiftASN1/Basic ASN1 Types/GeneralizedTime.swift b/Sources/SwiftASN1/Basic ASN1 Types/GeneralizedTime.swift index 1cb426e..bff2b32 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/GeneralizedTime.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/GeneralizedTime.swift @@ -206,21 +206,15 @@ public struct GeneralizedTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Has @inlinable public init(derEncoded node: ASN1Node, withIdentifier identifier: ASN1Identifier) throws { - guard node.identifier == identifier else { - throw ASN1Error.unexpectedFieldType(node.identifier) - } - - guard case .primitive(let content) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") - } - + let content = try ASN1OctetString(derEncoded: node, withIdentifier: identifier).bytes self = try TimeUtilities.generalizedTimeFromBytes(content) } @inlinable public init(berEncoded node: ASN1Node, withIdentifier identifier: ASN1Identifier) throws { // TODO: BER supports relaxed timestamp parsing, which is not yet supported - self = try .init(derEncoded: node, withIdentifier: identifier) + let content = try ASN1OctetString(berEncoded: node, withIdentifier: identifier).bytes + self = try TimeUtilities.generalizedTimeFromBytes(content) } @inlinable diff --git a/Sources/SwiftASN1/Basic ASN1 Types/ObjectIdentifier.swift b/Sources/SwiftASN1/Basic ASN1 Types/ObjectIdentifier.swift index ae96932..ed79b33 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/ObjectIdentifier.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/ObjectIdentifier.swift @@ -42,7 +42,7 @@ public struct ASN1ObjectIdentifier: DERImplicitlyTaggable, BERImplicitlyTaggable } guard case .primitive(let content) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") + throw ASN1Error.invalidASN1Object(reason: "OID encoded with constructed encoding") } try Self.validateObjectIdentifierInEncodedForm(content) diff --git a/Sources/SwiftASN1/Basic ASN1 Types/UTCTime.swift b/Sources/SwiftASN1/Basic ASN1 Types/UTCTime.swift index 550e7ad..ae79b8a 100644 --- a/Sources/SwiftASN1/Basic ASN1 Types/UTCTime.swift +++ b/Sources/SwiftASN1/Basic ASN1 Types/UTCTime.swift @@ -128,20 +128,14 @@ public struct UTCTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Hashable, S @inlinable public init(derEncoded node: ASN1Node, withIdentifier identifier: ASN1Identifier) throws { - guard node.identifier == identifier else { - throw ASN1Error.unexpectedFieldType(node.identifier) - } - - guard case .primitive(let content) = node.content else { - preconditionFailure("ASN.1 parser generated primitive node with constructed content") - } - + let content = try ASN1OctetString(derEncoded: node, withIdentifier: identifier).bytes self = try TimeUtilities.utcTimeFromBytes(content) } @inlinable public init(berEncoded node: ASN1Node, withIdentifier identifier: ASN1Identifier) throws { - self = try .init(derEncoded: node, withIdentifier: identifier) + let content = try ASN1OctetString(berEncoded: node, withIdentifier: identifier).bytes + self = try TimeUtilities.utcTimeFromBytes(content) } @inlinable diff --git a/Sources/SwiftASN1/DER.swift b/Sources/SwiftASN1/DER.swift index aa6e69a..7374888 100644 --- a/Sources/SwiftASN1/DER.swift +++ b/Sources/SwiftASN1/DER.swift @@ -233,8 +233,9 @@ extension DER { // We expect a single child. guard case .constructed(let nodes) = node.content else { - // This error is an internal parser error: the tag above is always constructed. - preconditionFailure("Explicit tags are always constructed") + throw ASN1Error.invalidASN1Object( + reason: "Explicit tags should always be constructed, got \(node.identifier) which is not." + ) } var nodeIterator = nodes.makeIterator() diff --git a/Tests/SwiftASN1Tests/ASN1Tests.swift b/Tests/SwiftASN1Tests/ASN1Tests.swift index 29e9315..9cce268 100644 --- a/Tests/SwiftASN1Tests/ASN1Tests.swift +++ b/Tests/SwiftASN1Tests/ASN1Tests.swift @@ -967,6 +967,23 @@ class ASN1Tests: XCTestCase { } } + func testPrimitiveTaggedObject() throws { + // We should error if primitive encoding is used for an explicitly tagged object. + let weirdASN1: [UInt8] = [ + 0x30, 0x05, // Sequence, containing... + 0x82, 0x03, // Context specific tag 2, 3 byte body, containing... + 0x02, 0x01, 0x00, // Integer 0 + ] + let parsed = try DER.parse(weirdASN1) + try DER.sequence(parsed, identifier: .sequence) { nodes in + XCTAssertThrowsError( + try DER.optionalExplicitlyTagged(&nodes, tagNumber: 2, tagClass: .contextSpecific, { _ in }) + ) { error in + XCTAssertEqual((error as? ASN1Error)?.code, .invalidASN1Object) + } + } + } + func testSPKIWithUnexpectedKeyTypeOID() throws { // This is an SPKI object for RSA instead of EC. This is a 1024-bit RSA key, so hopefully no-one will think to use it. let rsaSPKI = @@ -1366,4 +1383,48 @@ class ASN1Tests: XCTestCase { XCTAssertEqual(asn1OctetString.bytes, [0xFE, 0xED, 0xFA, 0xCE]) XCTAssertThrowsError(try DER.parse(berOctetString)) } + + func testConstructedBoolean() throws { + let weirdASN1: [UInt8] = [0x21, 0x00] + let node = try DER.parse(weirdASN1) + XCTAssertThrowsError(try Bool(berEncoded: node)) + XCTAssertThrowsError(try Bool(derEncoded: node)) + } + + func testConstructedInteger() throws { + let weirdASN1: [UInt8] = [0x22, 0x00] + let node = try DER.parse(weirdASN1) + XCTAssertThrowsError(try Int(berEncoded: node)) + XCTAssertThrowsError(try Int(derEncoded: node)) + } + + func testConstructedBitString() throws { + let weirdASN1: [UInt8] = [0x23, 0x08, 0x03, 0x02, 0x00, 0xAB, 0x03, 0x02, 0x04, 0xC] + let node = try DER.parse(weirdASN1) + // Not yet supported + // XCTAssertEqual(try ASN1BitString(berEncoded: node), ASN1BitString(bytes: [0xAB, 0xC], paddingBits: 4)) + XCTAssertThrowsError(try ASN1BitString(berEncoded: node)) + XCTAssertThrowsError(try ASN1BitString(derEncoded: node)) + } + + func testConstructedOctetString() throws { + let weirdASN1: [UInt8] = [0x24, 0x06, 0x04, 0x01, 0xAB, 0x04, 0x01, 0xCD] + let node = try DER.parse(weirdASN1) + XCTAssertEqual(try ASN1OctetString(berEncoded: node), ASN1OctetString(contentBytes: [0xAB, 0xCD])) + XCTAssertThrowsError(try ASN1OctetString(derEncoded: node)) + } + + func testConstructedNull() throws { + let weirdASN1: [UInt8] = [0x25, 0x00] + let node = try DER.parse(weirdASN1) + XCTAssertThrowsError(try ASN1Null(berEncoded: node)) + XCTAssertThrowsError(try ASN1Null(derEncoded: node)) + } + + func testConstructedOID() throws { + let weirdASN1: [UInt8] = [0x26, 0x03, 0x02, 0x01, 0x00] + let node = try DER.parse(weirdASN1) + XCTAssertThrowsError(try ASN1ObjectIdentifier(berEncoded: node)) + XCTAssertThrowsError(try ASN1ObjectIdentifier(derEncoded: node)) + } }