Deprecate NIOSSLCertificate(file:format:) (#550)

Motivation:

NIOSSLCertificate has an 'init' which loads a single cert from PEM or
DER encoded file. This, obviously, only loads the first PEM which is
very rarely what users actually want to do. This is a common source of
error when loading cert chains and is easy to miss in review.

Modifications:

- Deprecate the init and point users to the `fromPEMBytes` method for
  PEM which returns an array of certs, and to a new `fromDERBytes` which
  returns a single cert.

Result:

Harder to misuse API

Author: glbrntt

Sources/NIOSSL/SSLCertificate.swift

@@ -80,10 +80,33 @@ public final class NIOSSLCertificate {
     ///
     /// Note that this method will only ever load the first certificate from a given file.
     ///
+    /// If you want to load certificates from a PEM file use ``fromPEMFile(_:)``. To load
+    /// a certificate from a DER file use ``fromDERFile(_:)``.
+    ///
     /// - parameters:
     ///     - file: The path to the file to load the certificate from.
     ///     - format: The format to use to parse the file.
+    @available(
+        *,
+        deprecated,
+        message: """
+            Use 'fromPEMFile(_:)' to load all certificates from a PEM file or 'fromDERFile(_:)' \
+            to load a single certificate from a DER file.
+            """
+    )
     public convenience init(file: String, format: NIOSSLSerializationFormats) throws {
+        try self.init(_file: file, format: format)
+    }
+
+    /// Create a ``NIOSSLCertificate`` from a file at a given path in either PEM or
+    /// DER format.
+    ///
+    /// Note that this method will only ever load the first certificate from a given file.
+    ///
+    /// - parameters:
+    ///     - file: The path to the file to load the certificate from.
+    ///     - format: The format to use to parse the file.
+    private convenience init(_file file: String, format: NIOSSLSerializationFormats) throws {
         let fileObject = try Posix.fopen(file: file, mode: "rb")
         defer {
             fclose(fileObject)
@@ -335,6 +358,14 @@ extension NIOSSLCertificate {
         return try readCertificatesFromBIO(bio)
     }
 
+    /// Create a ``NIOSSLCertificate`` from a DER file at a given path.
+    ///
+    /// - parameters:
+    ///     - path: The path to the file to load the certificate from.
+    public static func fromDERFile(_ path: String) throws -> NIOSSLCertificate {
+        try NIOSSLCertificate(_file: path, format: .der)
+    }
+
     /// Returns the timestamp before which this certificate is not valid.
     ///
     /// The value is in seconds since the UNIX epoch.

Sources/NIOSSL/SSLContext.swift

@@ -688,8 +688,9 @@ extension NIOSSLContext {
             // Load only the certificates that resolve to an existing certificate in the directory.
             for symPath in certificateFilePaths {
                 // c_rehash only support pem files.
-                let cert = try NIOSSLCertificate(file: symPath, format: .pem)
-                try addCACertificateNameToList(context: context, certificate: cert)
+                if let cert = try NIOSSLCertificate.fromPEMFile(symPath).first {
+                    try addCACertificateNameToList(context: context, certificate: cert)
+                }
             }
         }
     }

Tests/NIOSSLTests/SSLCertificateTest.swift

@@ -220,10 +220,8 @@ class SSLCertificateTest: XCTestCase {
 
     func testLoadingPemCertFromFile() throws {
         let (cert1, cert2) = try Self.withPemCertPath {
-            (
-                try NIOSSLCertificate(file: $0, format: .pem),
-                try NIOSSLCertificate(file: $0, format: .pem)
-            )
+            let cert = try NIOSSLCertificate.fromPEMFile($0).first!
+            return (cert, cert)
         }
 
         XCTAssertEqual(cert1, cert2)
@@ -234,10 +232,8 @@ class SSLCertificateTest: XCTestCase {
 
     func testLoadingDerCertFromFile() throws {
         let (cert1, cert2) = try Self.withDerCertPath {
-            (
-                try NIOSSLCertificate(file: $0, format: .der),
-                try NIOSSLCertificate(file: $0, format: .der)
-            )
+            let cert = try NIOSSLCertificate.fromDERFile($0)
+            return (cert, cert)
         }
 
         XCTAssertEqual(cert1, cert2)
@@ -248,10 +244,10 @@ class SSLCertificateTest: XCTestCase {
 
     func testDerAndPemAreIdentical() throws {
         let cert1 = try Self.withPemCertPath {
-            try NIOSSLCertificate(file: $0, format: .pem)
+            try NIOSSLCertificate.fromPEMFile($0).first!
         }
         let cert2 = try Self.withDerCertPath {
-            try NIOSSLCertificate(file: $0, format: .der)
+            try NIOSSLCertificate.fromDERFile($0)
         }
 
         XCTAssertEqual(cert1, cert2)
@@ -334,7 +330,7 @@ class SSLCertificateTest: XCTestCase {
             _ = tempFile.withCString { unlink($0) }
         }
 
-        XCTAssertThrowsError(try NIOSSLCertificate(file: tempFile, format: .pem)) { error in
+        XCTAssertThrowsError(try NIOSSLCertificate.fromPEMFile(tempFile)) { error in
             XCTAssertEqual(.failedToLoadCertificate, error as? NIOSSLError)
         }
     }
@@ -356,11 +352,12 @@ class SSLCertificateTest: XCTestCase {
             _ = tempFile.withCString { unlink($0) }
         }
 
-        XCTAssertThrowsError(try NIOSSLCertificate(file: tempFile, format: .der)) { error in
+        XCTAssertThrowsError(try NIOSSLCertificate.fromDERFile(tempFile)) { error in
             XCTAssertEqual(.failedToLoadCertificate, error as? NIOSSLError)
         }
     }
 
+    @available(*, deprecated, message: "Deprecated to test deprecated functionality")
     func testLoadingNonexistentFileAsPem() throws {
         XCTAssertThrowsError(try NIOSSLCertificate(file: "/nonexistent/path", format: .pem)) { error in
             guard let error = error as? IOError else {
@@ -382,7 +379,7 @@ class SSLCertificateTest: XCTestCase {
     }
 
     func testLoadingNonexistentFileAsDer() throws {
-        XCTAssertThrowsError(try NIOSSLCertificate(file: "/nonexistent/path", format: .der)) { error in
+        XCTAssertThrowsError(try NIOSSLCertificate.fromDERFile("/nonexistent/path")) { error in
             guard let error = error as? IOError else {
                 return XCTFail("unexpected error \(error)")
             }

Tests/NIOSSLTests/TLSConfigurationTest.swift

@@ -439,9 +439,9 @@ class TLSConfigurationTest: XCTestCase {
     func getRehashFilename(path: String, testName: String, numericExtension: Int) -> String {
         var cert: NIOSSLCertificate!
         if path.suffix(4) == ".pem" {
-            XCTAssertNoThrow(cert = try NIOSSLCertificate(file: path, format: .pem))
+            XCTAssertNoThrow(cert = try NIOSSLCertificate.fromPEMFile(path).first)
         } else {
-            XCTAssertNoThrow(cert = try NIOSSLCertificate(file: path, format: .der))
+            XCTAssertNoThrow(cert = try NIOSSLCertificate.fromDERFile(path))
         }
         // Create a rehash format filename to symlink the hard file above to.
         let originalSubjectName = cert.getSubjectNameHash()