Skip to content

Commit

Permalink
Mark .file case deprecated for NIOSSLPrivateKeySource (#474)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ayush1794 authored Jul 29, 2024
1 parent 62c9086 commit a9fa5ef
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Sources/NIOSSL/SSLErrors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}


Expand Down
9 changes: 9 additions & 0 deletions Sources/NIOSSL/TLSConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOSSLHTTP1Client/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion Sources/NIOTLSServer/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)


Expand Down
3 changes: 2 additions & 1 deletion Tests/NIOSSLTests/NIOSSLIntegrationTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -554,9 +554,10 @@ class NIOSSLIntegrationTest: XCTestCase {
private func configuredSSLContext<T: Collection>(passphraseCallback: @escaping NIOSSLPassphraseCallback<T>,
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)
Expand Down
1 change: 1 addition & 0 deletions Tests/NIOSSLTests/SSLPrivateKeyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ class SSLPrivateKeyTest: XCTestCase {
}
}

@available(*, deprecated, message: "`.file` NIOSSLPrivateKeySource option deprecated")
func testMissingPassword() {
let configuration = TLSConfiguration.makeServerConfiguration(
certificateChain: [],
Expand Down
13 changes: 7 additions & 6 deletions Tests/NIOSSLTests/TLSConfigurationTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]])
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit a9fa5ef

Please sign in to comment.