Merge commit from fork

* Throw error instead of halting when the encoding is invalid Signed-off-by: Cory Benfield <[email protected]> * Avoid extra precondition failure --------- Signed-off-by: Cory Benfield <[email protected]> Co-authored-by: Nicolas Bachschmidt <[email protected]>
apple · Jan 14, 2025 · ae33e59 · ae33e59
1 parent c27bcbd
commit ae33e59
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))
+    }
 }