Skip to content

Commit a4a5aa7

Browse files
authored
Change PackageCollectionSigning API from callback-based to async/await (#6609)
* Change PackageCollectionSigning API from callback-based to async/await Motivation: Package collection signing crashes with code 139 on Linux (Ubuntu Jammy). Adding a signed collection also fails. Modifications: - Change `PackageCollectionSigning` API from callback-based to async/await - Certificate validation is async/await instead of callback-based - Change `PackageCollectionSigning` to actor since accessing `self.observabilityScope` also causes crash * Add import * Use task group * Make test fixture methods async
1 parent 0886bd1 commit a4a5aa7

File tree

9 files changed

+1396
-1634
lines changed

9 files changed

+1396
-1634
lines changed

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

Lines changed: 205 additions & 106 deletions
Large diffs are not rendered by default.

Sources/PackageCollectionsSigning/CertificatePolicy.swift

Lines changed: 47 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
import _Concurrency
1413
import Dispatch
1514
import Foundation
1615

@@ -53,66 +52,58 @@ protocol CertificatePolicy {
5352
/// Validates the given certificate chain.
5453
///
5554
/// - Parameters:
56-
/// - certChainPaths: Paths to each certificate in the chain. The certificate being verified must be the first
57-
/// element of the array, with its issuer the next element and so on, and the root CA
58-
/// certificate is last.
55+
/// - certChain: The certificate being verified must be the first element of the array, with its issuer the next
56+
/// element and so on, and the root CA certificate is last.
5957
/// - validationTime: Overrides the timestamp used for checking certificate expiry (e.g., for testing).
6058
/// By default the current time is used.
61-
/// - callback: The callback to invoke when the result is available.
62-
func validate(certChain: [Certificate], validationTime: Date, callback: @escaping (Result<Void, Error>) -> Void)
59+
func validate(certChain: [Certificate], validationTime: Date) async throws
6360
}
6461

6562
extension CertificatePolicy {
6663
/// Validates the given certificate chain.
6764
///
6865
/// - Parameters:
69-
/// - certChainPaths: Paths to each certificate in the chain. The certificate being verified must be the first
70-
/// element of the array, with its issuer the next element and so on, and the root CA
71-
/// certificate is last.
72-
/// - callback: The callback to invoke when the result is available.
73-
func validate(certChain: [Certificate], callback: @escaping (Result<Void, Error>) -> Void) {
74-
self.validate(certChain: certChain, validationTime: Date(), callback: callback)
66+
/// - certChain: The certificate being verified must be the first element of the array, with its issuer the next
67+
/// element and so on, and the root CA certificate is last.
68+
func validate(certChain: [Certificate]) async throws {
69+
try await self.validate(certChain: certChain, validationTime: Date())
7570
}
7671

7772
func verify(
7873
certChain: [Certificate],
7974
trustedRoots: [Certificate]?,
8075
@PolicyBuilder policies: () -> some VerifierPolicy,
81-
observabilityScope: ObservabilityScope,
82-
callbackQueue: DispatchQueue,
83-
callback: @escaping (Result<Void, Error>) -> Void
84-
) {
85-
let wrappedCallback: (Result<Void, Error>) -> Void = { result in callbackQueue.async { callback(result) } }
86-
76+
observabilityScope: ObservabilityScope
77+
) async throws {
8778
guard !certChain.isEmpty else {
88-
return wrappedCallback(.failure(CertificatePolicyError.emptyCertChain))
79+
throw CertificatePolicyError.emptyCertChain
8980
}
81+
9082
let policies = policies()
91-
Task {
92-
var trustStore = CertificateStores.defaultTrustRoots
93-
if let trustedRoots {
94-
trustStore.append(contentsOf: trustedRoots)
95-
}
9683

97-
guard !trustStore.isEmpty else {
98-
return wrappedCallback(.failure(CertificatePolicyError.noTrustedRootCertsConfigured))
99-
}
84+
var trustStore = CertificateStores.defaultTrustRoots
85+
if let trustedRoots {
86+
trustStore.append(contentsOf: trustedRoots)
87+
}
10088

101-
var verifier = Verifier(rootCertificates: CertificateStore(trustStore)) {
102-
policies
103-
}
104-
let result = await verifier.validate(
105-
leafCertificate: certChain[0],
106-
intermediates: CertificateStore(certChain)
107-
)
89+
guard !trustStore.isEmpty else {
90+
throw CertificatePolicyError.noTrustedRootCertsConfigured
91+
}
10892

109-
switch result {
110-
case .validCertificate:
111-
wrappedCallback(.success(()))
112-
case .couldNotValidate(let failures):
113-
observabilityScope.emit(error: "Failed to validate certificate chain \(certChain): \(failures)")
114-
wrappedCallback(.failure(CertificatePolicyError.invalidCertChain))
115-
}
93+
var verifier = Verifier(rootCertificates: CertificateStore(trustStore)) {
94+
policies
95+
}
96+
let result = await verifier.validate(
97+
leafCertificate: certChain[0],
98+
intermediates: CertificateStore(certChain)
99+
)
100+
101+
switch result {
102+
case .validCertificate:
103+
return
104+
case .couldNotValidate(let failures):
105+
observabilityScope.emit(error: "Failed to validate certificate chain \(certChain): \(failures)")
106+
throw CertificatePolicyError.invalidCertChain
116107
}
117108
}
118109
}
@@ -136,7 +127,6 @@ struct DefaultCertificatePolicy: CertificatePolicy {
136127
let expectedSubjectUserID: String?
137128
let expectedSubjectOrganizationalUnit: String?
138129

139-
private let callbackQueue: DispatchQueue
140130
private let httpClient: HTTPClient
141131
private let observabilityScope: ObservabilityScope
142132

@@ -150,14 +140,12 @@ struct DefaultCertificatePolicy: CertificatePolicy {
150140
/// user configured and dynamic, while this is configured by SwiftPM and static.
151141
/// - expectedSubjectUserID: The subject user ID that must match if specified.
152142
/// - expectedSubjectOrganizationalUnit: The subject organizational unit name that must match if specified.
153-
/// - callbackQueue: The `DispatchQueue` to use for callbacks.
154143
init(
155144
trustedRootCertsDir: URL?,
156145
additionalTrustedRootCerts: [Certificate]?,
157146
expectedSubjectUserID: String? = nil,
158147
expectedSubjectOrganizationalUnit: String? = nil,
159-
observabilityScope: ObservabilityScope,
160-
callbackQueue: DispatchQueue
148+
observabilityScope: ObservabilityScope
161149
) {
162150
var trustedRoots = [Certificate]()
163151
if let trustedRootCertsDir {
@@ -170,19 +158,16 @@ struct DefaultCertificatePolicy: CertificatePolicy {
170158
self.trustedRoots = trustedRoots
171159
self.expectedSubjectUserID = expectedSubjectUserID
172160
self.expectedSubjectOrganizationalUnit = expectedSubjectOrganizationalUnit
173-
self.callbackQueue = callbackQueue
174161
self.httpClient = HTTPClient.makeDefault()
175162
self.observabilityScope = observabilityScope
176163
}
177164

178-
func validate(certChain: [Certificate], validationTime: Date, callback: @escaping (Result<Void, Error>) -> Void) {
179-
let wrappedCallback: (Result<Void, Error>) -> Void = { result in self.callbackQueue.async { callback(result) } }
180-
165+
func validate(certChain: [Certificate], validationTime: Date) async throws {
181166
guard !certChain.isEmpty else {
182-
return wrappedCallback(.failure(CertificatePolicyError.emptyCertChain))
167+
throw CertificatePolicyError.emptyCertChain
183168
}
184169

185-
self.verify(
170+
try await self.verify(
186171
certChain: certChain,
187172
trustedRoots: self.trustedRoots,
188173
policies: {
@@ -202,9 +187,7 @@ struct DefaultCertificatePolicy: CertificatePolicy {
202187
validationTime: validationTime
203188
)
204189
},
205-
observabilityScope: self.observabilityScope,
206-
callbackQueue: self.callbackQueue,
207-
callback: callback
190+
observabilityScope: self.observabilityScope
208191
)
209192
}
210193
}
@@ -218,7 +201,6 @@ struct ADPSwiftPackageCollectionCertificatePolicy: CertificatePolicy {
218201
let expectedSubjectUserID: String?
219202
let expectedSubjectOrganizationalUnit: String?
220203

221-
private let callbackQueue: DispatchQueue
222204
private let httpClient: HTTPClient
223205
private let observabilityScope: ObservabilityScope
224206

@@ -232,14 +214,12 @@ struct ADPSwiftPackageCollectionCertificatePolicy: CertificatePolicy {
232214
/// user configured and dynamic, while this is configured by SwiftPM and static.
233215
/// - expectedSubjectUserID: The subject user ID that must match if specified.
234216
/// - expectedSubjectOrganizationalUnit: The subject organizational unit name that must match if specified.
235-
/// - callbackQueue: The `DispatchQueue` to use for callbacks.
236217
init(
237218
trustedRootCertsDir: URL?,
238219
additionalTrustedRootCerts: [Certificate]?,
239220
expectedSubjectUserID: String? = nil,
240221
expectedSubjectOrganizationalUnit: String? = nil,
241-
observabilityScope: ObservabilityScope,
242-
callbackQueue: DispatchQueue
222+
observabilityScope: ObservabilityScope
243223
) {
244224
var trustedRoots = [Certificate]()
245225
if let trustedRootCertsDir {
@@ -252,19 +232,16 @@ struct ADPSwiftPackageCollectionCertificatePolicy: CertificatePolicy {
252232
self.trustedRoots = trustedRoots
253233
self.expectedSubjectUserID = expectedSubjectUserID
254234
self.expectedSubjectOrganizationalUnit = expectedSubjectOrganizationalUnit
255-
self.callbackQueue = callbackQueue
256235
self.httpClient = HTTPClient.makeDefault()
257236
self.observabilityScope = observabilityScope
258237
}
259238

260-
func validate(certChain: [Certificate], validationTime: Date, callback: @escaping (Result<Void, Error>) -> Void) {
261-
let wrappedCallback: (Result<Void, Error>) -> Void = { result in self.callbackQueue.async { callback(result) } }
262-
239+
func validate(certChain: [Certificate], validationTime: Date) async throws {
263240
guard !certChain.isEmpty else {
264-
return wrappedCallback(.failure(CertificatePolicyError.emptyCertChain))
241+
throw CertificatePolicyError.emptyCertChain
265242
}
266243

267-
self.verify(
244+
try await self.verify(
268245
certChain: certChain,
269246
trustedRoots: self.trustedRoots,
270247
policies: {
@@ -286,9 +263,7 @@ struct ADPSwiftPackageCollectionCertificatePolicy: CertificatePolicy {
286263
validationTime: validationTime
287264
)
288265
},
289-
observabilityScope: self.observabilityScope,
290-
callbackQueue: self.callbackQueue,
291-
callback: callback
266+
observabilityScope: self.observabilityScope
292267
)
293268
}
294269
}
@@ -302,7 +277,6 @@ struct ADPAppleDistributionCertificatePolicy: CertificatePolicy {
302277
let expectedSubjectUserID: String?
303278
let expectedSubjectOrganizationalUnit: String?
304279

305-
private let callbackQueue: DispatchQueue
306280
private let httpClient: HTTPClient
307281
private let observabilityScope: ObservabilityScope
308282

@@ -316,14 +290,12 @@ struct ADPAppleDistributionCertificatePolicy: CertificatePolicy {
316290
/// user configured and dynamic, while this is configured by SwiftPM and static.
317291
/// - expectedSubjectUserID: The subject user ID that must match if specified.
318292
/// - expectedSubjectOrganizationalUnit: The subject organizational unit name that must match if specified.
319-
/// - callbackQueue: The `DispatchQueue` to use for callbacks.
320293
init(
321294
trustedRootCertsDir: URL?,
322295
additionalTrustedRootCerts: [Certificate]?,
323296
expectedSubjectUserID: String? = nil,
324297
expectedSubjectOrganizationalUnit: String? = nil,
325-
observabilityScope: ObservabilityScope,
326-
callbackQueue: DispatchQueue
298+
observabilityScope: ObservabilityScope
327299
) {
328300
var trustedRoots = [Certificate]()
329301
if let trustedRootCertsDir {
@@ -336,19 +308,16 @@ struct ADPAppleDistributionCertificatePolicy: CertificatePolicy {
336308
self.trustedRoots = trustedRoots
337309
self.expectedSubjectUserID = expectedSubjectUserID
338310
self.expectedSubjectOrganizationalUnit = expectedSubjectOrganizationalUnit
339-
self.callbackQueue = callbackQueue
340311
self.httpClient = HTTPClient.makeDefault()
341312
self.observabilityScope = observabilityScope
342313
}
343314

344-
func validate(certChain: [Certificate], validationTime: Date, callback: @escaping (Result<Void, Error>) -> Void) {
345-
let wrappedCallback: (Result<Void, Error>) -> Void = { result in self.callbackQueue.async { callback(result) } }
346-
315+
func validate(certChain: [Certificate], validationTime: Date) async throws {
347316
guard !certChain.isEmpty else {
348-
return wrappedCallback(.failure(CertificatePolicyError.emptyCertChain))
317+
throw CertificatePolicyError.emptyCertChain
349318
}
350319

351-
self.verify(
320+
try await self.verify(
352321
certChain: certChain,
353322
trustedRoots: self.trustedRoots,
354323
policies: {
@@ -370,9 +339,7 @@ struct ADPAppleDistributionCertificatePolicy: CertificatePolicy {
370339
validationTime: validationTime
371340
)
372341
},
373-
observabilityScope: self.observabilityScope,
374-
callbackQueue: self.callbackQueue,
375-
callback: callback
342+
observabilityScope: self.observabilityScope
376343
)
377344
}
378345
}

0 commit comments

Comments
 (0)