From a9fa5efd86e7ce2e5c1b6de113262e58035ca251 Mon Sep 17 00:00:00 2001 From: Ayush Garg Date: Mon, 29 Jul 2024 11:10:26 +0100 Subject: [PATCH] Mark .file case deprecated for NIOSSLPrivateKeySource (#474) swift-nio-ssl expects file to in specific format when loading the PrivateKey from a file baed on the extension of the file. This is prone to errors, since there is no standard agreement on the extension and format of the file. An example of a mismatch is `.key` swift-nio-ssl tries to load such file as DER, though these can be in PEM format too. We could based on the binary contents of the file provided infer the format of the file, but it doesn't seem like a logic this library should own. The clients should be able to load the file themselves. In this PR `.file` case is marked deprecated and relevant deprecation flags and documentation is added to point to the preferred option. --- Sources/NIOSSL/SSLErrors.swift | 2 +- Sources/NIOSSL/TLSConfiguration.swift | 9 +++++++++ Sources/NIOSSLHTTP1Client/main.swift | 2 +- Sources/NIOTLSServer/main.swift | 3 ++- Tests/NIOSSLTests/NIOSSLIntegrationTest.swift | 3 ++- Tests/NIOSSLTests/SSLPrivateKeyTests.swift | 1 + Tests/NIOSSLTests/TLSConfigurationTest.swift | 13 +++++++------ 7 files changed, 23 insertions(+), 10 deletions(-) diff --git a/Sources/NIOSSL/SSLErrors.swift b/Sources/NIOSSL/SSLErrors.swift index c8cb36d3c..e52868527 100644 --- a/Sources/NIOSSL/SSLErrors.swift +++ b/Sources/NIOSSL/SSLErrors.swift @@ -250,7 +250,7 @@ extension NIOSSLExtraError { internal static func unknownPrivateKeyFileType(path: String) -> NIOSSLExtraError { let description = "Unknown private key file type for \(path)" return NIOSSLExtraError(baseError: .unknownPrivateKeyFileType, description: description) - } + } } diff --git a/Sources/NIOSSL/TLSConfiguration.swift b/Sources/NIOSSL/TLSConfiguration.swift index aa1968c09..31cf4764d 100644 --- a/Sources/NIOSSL/TLSConfiguration.swift +++ b/Sources/NIOSSL/TLSConfiguration.swift @@ -32,7 +32,16 @@ public enum NIOSSLCertificateSource: Hashable, Sendable { /// Places NIOSSL can obtain private keys from. public enum NIOSSLPrivateKeySource: Hashable { + /// Path to file in PEM or ASN1 format to load private key from + /// + /// File Extensions | Expected file format + /// --------------- | -------------------- + /// `.pem` | PEM + /// `.der or .key` | ASN1 + @available(*, deprecated, message: "Use 'NIOSSLPrivateKeySource.privateKey(NIOSSLPrivateKey)' to set private key") case file(String) + + /// Loaded Private key case privateKey(NIOSSLPrivateKey) } diff --git a/Sources/NIOSSLHTTP1Client/main.swift b/Sources/NIOSSLHTTP1Client/main.swift index a59911557..602aa4f86 100644 --- a/Sources/NIOSSLHTTP1Client/main.swift +++ b/Sources/NIOSSLHTTP1Client/main.swift @@ -82,7 +82,7 @@ if let c = arguments.dropFirst(2).first { cert.append(contentsOf: try NIOSSLCertificate.fromPEMFile(c).map { .certificate($0) }) } if let k = arguments.dropFirst(3).first { - key = .file(k) + try! key = .privateKey(.init(file: k, format: .pem)) } if let r = arguments.dropFirst(4).first { trustRoot = .file(r) diff --git a/Sources/NIOTLSServer/main.swift b/Sources/NIOTLSServer/main.swift index e281ef482..2d40693a3 100644 --- a/Sources/NIOTLSServer/main.swift +++ b/Sources/NIOTLSServer/main.swift @@ -30,9 +30,10 @@ private final class EchoHandler: ChannelInboundHandler { } let certificateChain = try NIOSSLCertificate.fromPEMFile("cert.pem") +let privateKey = try! NIOSSLPrivateKey(file: "key.pem", format: .pem) let sslContext = try! NIOSSLContext(configuration: TLSConfiguration.makeServerConfiguration( certificateChain: certificateChain.map { .certificate($0) }, - privateKey: .file("key.pem")) + privateKey: .privateKey(privateKey)) ) diff --git a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift index 6ff43839a..ae0dc3059 100644 --- a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift +++ b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift @@ -554,9 +554,10 @@ class NIOSSLIntegrationTest: XCTestCase { private func configuredSSLContext(passphraseCallback: @escaping NIOSSLPassphraseCallback, file: StaticString = #filePath, line: UInt = #line) throws -> NIOSSLContext where T.Element == UInt8 { + let privateKey = try NIOSSLPrivateKey(file: NIOSSLIntegrationTest.encryptedKeyPath, format: .pem) { closure in closure("thisisagreatpassword".utf8) } var config = TLSConfiguration.makeServerConfiguration( certificateChain: [.certificate(NIOSSLIntegrationTest.cert)], - privateKey: .file(NIOSSLIntegrationTest.encryptedKeyPath) + privateKey: .privateKey(privateKey) ) config.trustRoots = .certificates([NIOSSLIntegrationTest.cert]) return try assertNoThrowWithValue(NIOSSLContext(configuration: config, passphraseCallback: passphraseCallback), file: file, line: line) diff --git a/Tests/NIOSSLTests/SSLPrivateKeyTests.swift b/Tests/NIOSSLTests/SSLPrivateKeyTests.swift index 8c22630ab..9dd8d61f0 100644 --- a/Tests/NIOSSLTests/SSLPrivateKeyTests.swift +++ b/Tests/NIOSSLTests/SSLPrivateKeyTests.swift @@ -237,6 +237,7 @@ class SSLPrivateKeyTest: XCTestCase { } } + @available(*, deprecated, message: "`.file` NIOSSLPrivateKeySource option deprecated") func testMissingPassword() { let configuration = TLSConfiguration.makeServerConfiguration( certificateChain: [], diff --git a/Tests/NIOSSLTests/TLSConfigurationTest.swift b/Tests/NIOSSLTests/TLSConfigurationTest.swift index 995ac7d3f..6061df4e0 100644 --- a/Tests/NIOSSLTests/TLSConfigurationTest.swift +++ b/Tests/NIOSSLTests/TLSConfigurationTest.swift @@ -744,7 +744,7 @@ class TLSConfigurationTest: XCTestCase { } func testComputedApplicationProtocols() throws { - var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .file("fake.file")) + var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .privateKey(TLSConfigurationTest.key1)) config.applicationProtocols = ["http/1.1"] XCTAssertEqual(config.applicationProtocols, ["http/1.1"]) XCTAssertEqual(config.encodedApplicationProtocols, [[8, 104, 116, 116, 112, 47, 49, 46, 49]]) @@ -792,7 +792,7 @@ class TLSConfigurationTest: XCTestCase { } func testTheSameHashValue() { - var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .file("fake.file")) + var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .privateKey(TLSConfigurationTest.key1)) config.applicationProtocols = ["http/1.1"] let theSameConfig = config var hasher = Hasher() @@ -804,15 +804,15 @@ class TLSConfigurationTest: XCTestCase { } func testDifferentHashValues() { - var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .file("fake.file")) + var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .privateKey(TLSConfigurationTest.key1)) config.applicationProtocols = ["http/1.1"] var differentConfig = config - differentConfig.privateKey = .file("fake2.file") + differentConfig.privateKey = .privateKey(TLSConfigurationTest.key2) XCTAssertFalse(config.bestEffortEquals(differentConfig)) } func testDifferentCallbacksNotEqual() { - var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .file("fake.file")) + var config = TLSConfiguration.makeServerConfiguration(certificateChain: [], privateKey: .privateKey(TLSConfigurationTest.key1)) config.applicationProtocols = ["http/1.1"] config.keyLogCallback = { _ in } var differentConfig = config @@ -1288,7 +1288,8 @@ class TLSConfigurationTest: XCTestCase { serverConfig.pskHint = "pskHint" try assertHandshakeError(withClientConfig: clientConfig, andServerConfig: serverConfig, errorTextContainsAnyOf: ["SSLV3_ALERT_BAD_RECORD_MAC"]) } - + + @available(*, deprecated, message: "`.file` NIOSSLPrivateKeySource option deprecated") func testUnknownPrivateKeyFileType() throws { var clientConfig = TLSConfiguration.makeClientConfiguration() clientConfig.privateKey = .file("key.invalidExtension")