Skip to content

Commit 145db19

Browse files
FranzBuschFranzBusch
authored
TimedCertificateReloader prevents non-self-signed certificates from b… (#274)
…eing loaded Fix certificate check and make sure all code paths throw an error. ### Motivation: TimedCertificateReloader currently has an invalid check that accidecanlly enforces self-signed certificates. Additionally some code paths of `reload()` did not throw an error which made it hard for the call site to judge if the reloader was initialized successfully or not. Further, the missing errors also meant that the `run()` method would never be able to log these. ### Modifications: * Updated the certificate check to validate the the public key of the private key matches the cert's public key (instead of checking the signature). * Added three new error cases. ### Result: * A caller can now be sure that their certificate and private key pair are valid after calling `reload()` the first time. * Errors in future reloads will be surfaced using the provided logger and do not go unnoticed. * You can actually use the reloader with normal certificates.
1 parent 24cb15c commit 145db19

File tree

2 files changed

+207
-79
lines changed

2 files changed

+207
-79
lines changed

‎Sources/NIOCertificateReloading/TimedCertificateReloader.swift‎

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,26 @@ public struct TimedCertificateReloader: CertificateReloader {
210210
private enum _Backing: Hashable, CustomStringConvertible {
211211
case certificatePathNotFound(String)
212212
case privateKeyPathNotFound(String)
213+
case certificateLoadingError(reason: String)
214+
case privateKeyLoadingError(reason: String)
215+
case publicKeyMismatch
213216

214217
var description: String {
215218
switch self {
216219
case .certificatePathNotFound(let path):
217220
return "Certificate path not found: \(path)"
218221
case .privateKeyPathNotFound(let path):
219222
return "Private key path not found: \(path)"
223+
case let .certificateLoadingError(reason):
224+
return "Failed to load certificate: \(reason)"
225+
case let .privateKeyLoadingError(reason):
226+
return "Failed to load private key: \(reason)"
227+
case .publicKeyMismatch:
228+
return
229+
"""
230+
The public key derived from the private key does not match the public key in the certificate. \n
231+
This may occur if the certificate and key were reloaded inconsistently or during an update in progress.
232+
"""
220233
}
221234
}
222235
}
@@ -241,6 +254,25 @@ public struct TimedCertificateReloader: CertificateReloader {
241254
Self(.privateKeyPathNotFound(path))
242255
}
243256

257+
/// Failed to load the certificate.
258+
/// - Parameter reason: A description of the error occurred.
259+
/// - Returns: A ``TimedCertificateReloader/Error``.
260+
public static func certificateLoadingError(reason: String) -> Self {
261+
Self(.certificateLoadingError(reason: reason))
262+
}
263+
264+
/// Failed to load the private key.
265+
/// - Parameter reason: A description of the error occurred.
266+
/// - Returns: A ``TimedCertificateReloader/Error``.
267+
public static func privateKeyLoadingError(reason: String) -> Self {
268+
Self(.privateKeyLoadingError(reason: reason))
269+
}
270+
271+
/// The private key does not match the provided certificate.
272+
public static var publicKeyMismatch: Self {
273+
Self(.publicKeyMismatch)
274+
}
275+
244276
public var description: String {
245277
self._backing.description
246278
}
@@ -350,13 +382,19 @@ public struct TimedCertificateReloader: CertificateReloader {
350382
public func reload() throws {
351383
let certificateBytes = try self.loadCertificate()
352384
let keyBytes = try self.loadPrivateKey()
353-
if let certificates = try self.parseCertificates(from: certificateBytes),
354-
let key = try self.parsePrivateKey(from: keyBytes),
355-
let firstCertificate = certificates.first,
356-
key.publicKey.isValidSignature(firstCertificate.signature, for: firstCertificate)
357-
{
358-
try self.attemptToUpdatePair(certificates: certificates, key: key)
385+
386+
let certificates = try self.parseCertificates(from: certificateBytes)
387+
let key = try self.parsePrivateKey(from: keyBytes)
388+
389+
guard let firstCertificate = certificates.first else {
390+
throw Error.certificateLoadingError(reason: "The provided file does not contain any certificates.")
391+
}
392+
393+
guard key.publicKey == firstCertificate.publicKey else {
394+
throw Error.publicKeyMismatch
359395
}
396+
397+
try self.attemptToUpdatePair(certificates: certificates, key: key)
360398
}
361399

362400
private func loadCertificate() throws -> [UInt8] {
@@ -389,28 +427,34 @@ public struct TimedCertificateReloader: CertificateReloader {
389427
return keyBytes
390428
}
391429

392-
private func parseCertificates(from certificateBytes: [UInt8]) throws -> [Certificate]? {
393-
let certificates: [Certificate]?
430+
private func parseCertificates(from certificateBytes: [UInt8]) throws -> [Certificate] {
431+
let certificates: [Certificate]
394432
switch self.certificateSource.format._backing {
395433
case .der:
396434
certificates = [try Certificate(derEncoded: certificateBytes)]
397435

398436
case .pem:
399-
certificates = try String(bytes: certificateBytes, encoding: .utf8)
400-
.flatMap { try PEMDocument.parseMultiple(pemString: $0).map { try Certificate(pemDocument: $0) } }
437+
guard let pemString = String(bytes: certificateBytes, encoding: .utf8) else {
438+
throw Error.certificateLoadingError(reason: "Certificate data is not valid UTF-8.")
439+
}
440+
441+
certificates = try PEMDocument.parseMultiple(pemString: pemString)
442+
.map { try Certificate(pemDocument: $0) }
401443
}
402444
return certificates
403445
}
404446

405-
private func parsePrivateKey(from keyBytes: [UInt8]) throws -> Certificate.PrivateKey? {
406-
let key: Certificate.PrivateKey?
447+
private func parsePrivateKey(from keyBytes: [UInt8]) throws -> Certificate.PrivateKey {
448+
let key: Certificate.PrivateKey
407449
switch self.privateKeySource.format._backing {
408450
case .der:
409451
key = try Certificate.PrivateKey(derBytes: keyBytes)
410452

411453
case .pem:
412-
key = try String(bytes: keyBytes, encoding: .utf8)
413-
.flatMap { try Certificate.PrivateKey(pemEncoded: $0) }
454+
guard let pemString = String(bytes: keyBytes, encoding: .utf8) else {
455+
throw Error.privateKeyLoadingError(reason: "Private Key data is not valid UTF-8.")
456+
}
457+
key = try Certificate.PrivateKey(pemEncoded: pemString)
414458
}
415459
return key
416460
}

‎Tests/NIOCertificateReloadingTests/TimedCertificateReloaderTests.swift‎

Lines changed: 149 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ final class TimedCertificateReloaderTests: XCTestCase {
6161
}
6262
}
6363

64+
func testNonSelfSignedCert() async throws {
65+
try await runTimedCertificateReloaderTest(
66+
certificate: .init(
67+
location: .memory(provider: { try Self.sampleCertNotSelfSigned.serializeAsPEM().derBytes }),
68+
format: .der
69+
),
70+
privateKey: .init(
71+
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
72+
format: .der
73+
),
74+
validateSources: true
75+
) { reloader in
76+
let override = reloader.sslContextConfigurationOverride
77+
XCTAssertNotNil(override.certificateChain)
78+
XCTAssertNotNil(override.privateKey)
79+
}
80+
}
81+
6482
func testKeyPathDoesNotExist() async throws {
6583
try await runTimedCertificateReloaderTest(
6684
certificate: .init(
@@ -102,19 +120,23 @@ final class TimedCertificateReloaderTests: XCTestCase {
102120
}
103121

104122
func testCertificateIsInUnexpectedFormat_FromMemory() async throws {
105-
try await runTimedCertificateReloaderTest(
106-
certificate: .init(
107-
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
108-
format: .pem
109-
),
110-
privateKey: .init(
111-
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
112-
format: .der
113-
)
114-
) { reloader in
115-
let override = reloader.sslContextConfigurationOverride
116-
XCTAssertNil(override.certificateChain)
117-
XCTAssertNil(override.privateKey)
123+
do {
124+
try await runTimedCertificateReloaderTest(
125+
certificate: .init(
126+
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
127+
format: .pem
128+
),
129+
privateKey: .init(
130+
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
131+
format: .der
132+
)
133+
) { reloader in
134+
XCTFail("Certificate reloader loaded correctly.")
135+
}
136+
} catch let error as TimedCertificateReloader.Error {
137+
XCTAssert(error == .certificateLoadingError(reason: "Certificate data is not valid UTF-8."))
138+
} catch {
139+
XCTFail("Encountered unexpected error \(error)")
118140
}
119141
}
120142

@@ -131,72 +153,111 @@ final class TimedCertificateReloaderTests: XCTestCase {
131153
func testCertificateIsInUnexpectedFormat_FromFile() async throws {
132154
let certBytes = try Self.sampleCert.serializeAsPEM().derBytes
133155
let file = try self.createTempFile(contents: Data(certBytes))
134-
try await runTimedCertificateReloaderTest(
135-
certificate: .init(
136-
location: .file(path: file.path),
137-
format: .pem
138-
),
139-
privateKey: .init(
140-
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
141-
format: .der
142-
)
143-
) { reloader in
144-
let override = reloader.sslContextConfigurationOverride
145-
XCTAssertNil(override.certificateChain)
146-
XCTAssertNil(override.privateKey)
156+
157+
do {
158+
try await runTimedCertificateReloaderTest(
159+
certificate: .init(
160+
location: .file(path: file.path),
161+
format: .pem
162+
),
163+
privateKey: .init(
164+
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
165+
format: .der
166+
)
167+
) { reloader in
168+
XCTFail("Certificate reloader loaded correctly.")
169+
}
170+
} catch let error as TimedCertificateReloader.Error {
171+
XCTAssert(error == .certificateLoadingError(reason: "Certificate data is not valid UTF-8."))
172+
} catch {
173+
XCTFail("Encountered unexpected error \(error)")
147174
}
148175
}
149176

150177
func testKeyIsInUnexpectedFormat_FromMemory() async throws {
151-
try await runTimedCertificateReloaderTest(
152-
certificate: .init(
153-
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
154-
format: .der
155-
),
156-
privateKey: .init(
157-
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
158-
format: .pem
159-
)
160-
) { reloader in
161-
let override = reloader.sslContextConfigurationOverride
162-
XCTAssertNil(override.certificateChain)
163-
XCTAssertNil(override.privateKey)
178+
do {
179+
try await runTimedCertificateReloaderTest(
180+
certificate: .init(
181+
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
182+
format: .der
183+
),
184+
privateKey: .init(
185+
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
186+
format: .pem
187+
)
188+
) { reloader in
189+
XCTFail("Certificate reloader loaded correctly.")
190+
}
191+
} catch let error as TimedCertificateReloader.Error {
192+
XCTAssert(error == .privateKeyLoadingError(reason: "Private Key data is not valid UTF-8."))
193+
} catch {
194+
XCTFail("Encountered unexpected error \(error)")
164195
}
165196
}
166197

167198
func testKeyIsInUnexpectedFormat_FromFile() async throws {
168199
let keyBytes = Self.samplePrivateKey1.derRepresentation
169200
let file = try self.createTempFile(contents: keyBytes)
170-
try await runTimedCertificateReloaderTest(
171-
certificate: .init(
172-
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
173-
format: .der
174-
),
175-
privateKey: .init(
176-
location: .file(path: file.path),
177-
format: .pem
178-
)
179-
) { reloader in
180-
let override = reloader.sslContextConfigurationOverride
181-
XCTAssertNil(override.certificateChain)
182-
XCTAssertNil(override.privateKey)
201+
202+
do {
203+
try await runTimedCertificateReloaderTest(
204+
certificate: .init(
205+
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
206+
format: .der
207+
),
208+
privateKey: .init(
209+
location: .file(path: file.path),
210+
format: .pem
211+
)
212+
) { reloader in
213+
XCTFail("Certificate reloader loaded correctly.")
214+
}
215+
} catch let error as TimedCertificateReloader.Error {
216+
XCTAssert(error == .privateKeyLoadingError(reason: "Private Key data is not valid UTF-8."))
217+
} catch {
218+
XCTFail("Encountered unexpected error \(error)")
183219
}
184220
}
185221

186222
func testCertificateAndKeyDoNotMatch() async throws {
187-
try await runTimedCertificateReloaderTest(
188-
certificate: .init(
189-
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
190-
format: .der
191-
),
192-
privateKey: .init(
193-
location: .memory(provider: { Array(P384.Signing.PrivateKey().derRepresentation) }),
194-
format: .der
195-
)
196-
) { reloader in
197-
let override = reloader.sslContextConfigurationOverride
198-
XCTAssertNil(override.certificateChain)
199-
XCTAssertNil(override.privateKey)
223+
do {
224+
try await runTimedCertificateReloaderTest(
225+
certificate: .init(
226+
location: .memory(provider: { try Self.sampleCert.serializeAsPEM().derBytes }),
227+
format: .der
228+
),
229+
privateKey: .init(
230+
location: .memory(provider: { Array(P384.Signing.PrivateKey().derRepresentation) }),
231+
format: .der
232+
)
233+
) { reloader in
234+
XCTFail("Certificate reloader loaded correctly.")
235+
}
236+
} catch let error as TimedCertificateReloader.Error {
237+
XCTAssert(error == .publicKeyMismatch)
238+
} catch {
239+
XCTFail("Encountered unexpected error \(error)")
240+
}
241+
}
242+
243+
func testEmptyCertificateChain() async throws {
244+
do {
245+
try await runTimedCertificateReloaderTest(
246+
certificate: .init(
247+
location: .memory(provider: { [] }),
248+
format: .pem
249+
),
250+
privateKey: .init(
251+
location: .memory(provider: { Array(Self.samplePrivateKey1.derRepresentation) }),
252+
format: .der
253+
)
254+
) { reloader in
255+
XCTFail("Certificate reloader loaded correctly.")
256+
}
257+
} catch let error as TimedCertificateReloader.Error {
258+
XCTAssert(error == .certificateLoadingError(reason: "The provided file does not contain any certificates."))
259+
} catch {
260+
XCTFail("Encountered unexpected error \(error)")
200261
}
201262
}
202263

@@ -563,6 +624,11 @@ final class TimedCertificateReloaderTests: XCTestCase {
563624
OrganizationName("Apple")
564625
CommonName("Swift Certificate Test")
565626
}
627+
static let issuerCertName = try! DistinguishedName {
628+
CountryName("US")
629+
OrganizationName("Apple")
630+
CommonName("Swift Certificate Test Issuer")
631+
}
566632
static let sampleCert: Certificate = {
567633
try! Certificate(
568634
version: .v3,
@@ -581,6 +647,24 @@ final class TimedCertificateReloaderTests: XCTestCase {
581647
issuerPrivateKey: .init(samplePrivateKey1)
582648
)
583649
}()
650+
static let sampleCertNotSelfSigned: Certificate = {
651+
try! Certificate(
652+
version: .v3,
653+
serialNumber: .init(),
654+
publicKey: .init(samplePrivateKey1.publicKey),
655+
notValidBefore: startDate.advanced(by: -60 * 60 * 24 * 360),
656+
notValidAfter: startDate.advanced(by: 60 * 60 * 24 * 360),
657+
issuer: issuerCertName,
658+
subject: sampleCertName,
659+
signatureAlgorithm: .ecdsaWithSHA384,
660+
extensions: Certificate.Extensions {
661+
Critical(
662+
BasicConstraints.isCertificateAuthority(maxPathLength: nil)
663+
)
664+
},
665+
issuerPrivateKey: .init(samplePrivateKey2)
666+
)
667+
}()
584668
static let sampleCertChain: [Certificate] = {
585669
[
586670
try! Certificate(

0 commit comments

Comments
 (0)