Make NIOSSLCertificate._subjectAlternativeNames() return type non-o…

…ptiona (#389)
apple · Aug 1, 2022 · 0265283 · 0265283
1 parent c37f562
commit 0265283
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 35 deletions.
diff --git a/Sources/NIOSSL/IdentityVerification.swift b/Sources/NIOSSL/IdentityVerification.swift
@@ -135,25 +135,23 @@ private func validIdentityForService(serverHostname: Array<UInt8>?,
     // them, and then refuse to check the commonName field. If there are no SAN fields to
     // validate against, we'll check commonName.
     var checkedMatch = false
-    if let alternativeNames = leafCertificate._subjectAlternativeNames() {
-        for name in alternativeNames {
-            checkedMatch = true
-
-            switch name.nameType {
-            case .dnsName:
-                let dnsName = Array(name.contents)
-                if matchHostname(ourHostname: serverHostnameSlice, firstPeriodIndex: firstPeriodIndex, dnsName: dnsName) {
-                    return true
-                }
-            case .ipAddress:
-                if let ip = _SubjectAlternativeName.IPAddress(name),
-                   matchIpAddress(socketAddress: socketAddress, certificateIP: ip)
-                {
-                    return true
-                }
-            default:
-                continue
+    for name in leafCertificate._subjectAlternativeNames() {
+        checkedMatch = true
+
+        switch name.nameType {
+        case .dnsName:
+            let dnsName = Array(name.contents)
+            if matchHostname(ourHostname: serverHostnameSlice, firstPeriodIndex: firstPeriodIndex, dnsName: dnsName) {
+                return true
+            }
+        case .ipAddress:
+            if let ip = _SubjectAlternativeName.IPAddress(name),
+               matchIpAddress(socketAddress: socketAddress, certificateIP: ip)
+            {
+                return true
             }
+        default:
+            continue
         }
     }
 

diff --git a/Sources/NIOSSL/SSLCertificate.swift b/Sources/NIOSSL/SSLCertificate.swift
@@ -165,11 +165,9 @@ public class NIOSSLCertificate {
     }
 
     /// Get a collection of the alternative names in the certificate.
-    public func _subjectAlternativeNames() -> _SubjectAlternativeNames? {
-        guard let sanExtension = CNIOBoringSSL_X509_get_ext_d2i(self.ref, NID_subject_alt_name, nil, nil) else {
-            return nil
-        }
-        return _SubjectAlternativeNames(nameStack: OpaquePointer(sanExtension))
+    public func _subjectAlternativeNames() -> _SubjectAlternativeNames {
+        let sanExtension = CNIOBoringSSL_X509_get_ext_d2i(self.ref, NID_subject_alt_name, nil, nil)
+        return _SubjectAlternativeNames(nameStack: sanExtension.map(OpaquePointer.init))
     }
 
     /// Extracts the SHA1 hash of the subject name before it has been truncated.
@@ -406,7 +404,8 @@ extension NIOSSLCertificate: CustomStringConvertible {
             let commonName = String(decoding: commonNameBytes, as: UTF8.self)
             desc += ";common_name=" + commonName
         }
-        if let alternativeName = self._subjectAlternativeNames() {
+        let alternativeName = self._subjectAlternativeNames()
+        if !alternativeName.isEmpty {
             let altNames = alternativeName.compactMap { name in
                 switch name.nameType {
                 case .dnsName:

diff --git a/Sources/NIOSSL/SubjectAlternativeName.swift b/Sources/NIOSSL/SubjectAlternativeName.swift
@@ -29,16 +29,20 @@ public struct _SubjectAlternativeNames {
     @usableFromInline
     internal final class Storage {
 
-        fileprivate let nameStack: OpaquePointer
+        fileprivate let nameStack: OpaquePointer?
         @usableFromInline internal let stackSize: Int
-
-        internal init(nameStack: OpaquePointer) {
+        
+        internal init(nameStack: OpaquePointer?) {
             self.nameStack = nameStack
-            self.stackSize = CNIOBoringSSLShims_sk_GENERAL_NAME_num(nameStack)
+            if let nameStack = nameStack {
+                self.stackSize = CNIOBoringSSLShims_sk_GENERAL_NAME_num(nameStack)
+            } else {
+                self.stackSize = 0
+            }
         }
 
         public subscript(position: Int) -> Element {
-            guard let name = CNIOBoringSSLShims_sk_GENERAL_NAME_value(self.nameStack, position) else {
+            guard let name = CNIOBoringSSLShims_sk_GENERAL_NAME_value(self.nameStack!, position) else {
                 fatalError("Unexpected null pointer when unwrapping SAN value")
             }
 
@@ -50,14 +54,16 @@ public struct _SubjectAlternativeNames {
         }
 
         deinit {
-            CNIOBoringSSL_GENERAL_NAMES_free(self.nameStack)
+            if let nameStack = self.nameStack {
+                CNIOBoringSSL_GENERAL_NAMES_free(nameStack)
+            }
         }
     }
 
 
     @usableFromInline internal var storage: Storage
 
-    internal init(nameStack: OpaquePointer) {
+    internal init(nameStack: OpaquePointer?) {
         self.storage = .init(nameStack: nameStack)
     }
 }

diff --git a/Tests/NIOSSLTests/SSLCertificateTest.swift b/Tests/NIOSSLTests/SSLCertificateTest.swift
@@ -379,9 +379,7 @@ class SSLCertificateTest: XCTestCase {
         precondition(inet_pton(AF_INET6, "2001:db8::1", &v6addr) == 1)
 
         let cert = try NIOSSLCertificate(bytes: .init(certWithAllSupportedSANTypes.utf8), format: .pem)
-        guard let sans = cert._subjectAlternativeNames() else {
-            return XCTFail("could not get subject alternative names")
-        }
+        let sans = cert._subjectAlternativeNames()
         XCTAssertEqual(sans.count, 7)
         XCTAssertEqual(sans[0].nameType, .dnsName)
         XCTAssertEqual(String(decoding: sans[0].contents, as: UTF8.self), "localhost")
@@ -405,7 +403,7 @@ class SSLCertificateTest: XCTestCase {
 
     func testNonexistentSan() throws {
         let cert = try NIOSSLCertificate(bytes: .init(samplePemCert.utf8), format: .pem)
-        XCTAssertNil(cert._subjectAlternativeNames())
+        XCTAssertTrue(cert._subjectAlternativeNames().isEmpty)
     }
 
     func testCommonName() throws {