Use installed Android cacerts for URLSession #5163
Conversation
|
This is ready for review. I've tested it out against the swift-6.0.3-RELEASE-android-24-0.1.artifactbundle.tar.gz artifact from https://github.com/marcprux/swift-android-sdk/actions/runs/12960699464 and loading an https site works on Android. |
* Add swift-android-foundation-cacerts.patch * Update cache key for PR * Update swift-android-foundation-cacerts.patch * Update patch to use CFURLSessionOptionCAPATH instead of CFURLSessionOptionCAINFO * New cache key for github workflow * Fix check for isDirectory * Revert "Fix check for isDirectory" This reverts commit ff12e16. * Revert "Update patch to use CFURLSessionOptionCAPATH instead of CFURLSessionOptionCAINFO" This reverts commit 94e355e. * Go back to using CFURLSessionOptionCAINFO instead of CFURLSessionOptionCAPATH, since the latter does not work * Update patch to match swiftlang/swift-corelibs-foundation#5163
|
|
||
| let aggregateCertPath = NSTemporaryDirectory() + "/cacerts-\(UUID().uuidString).pem" | ||
|
|
||
| if FileManager.default.createFile(atPath: aggregateCertPath, contents: nil) == false { |
There was a problem hiding this comment.
Does this file have to be cleaned up at some point?
There was a problem hiding this comment.
Good question. I'm not sure exactly when libcurl will read this file, and so we don't know when we can delete it. In the case of an Android app, NSTemporaryDirectory() will be the app's caches folder, and so will eventually be cleaned up, but still leaving stay files around like this is not good form.
I think the best solution would be rather than using CURLOPT_CAINFO (as exposed through CFURLSessionOptionCAINFO), we would instead use CURLOPT_CAINFO_BLOB, which would mean we wouldn't need to create any temporary file at all. But not only does this not currently have any equivalent CFURLSessionOption* property, it would also require bringing in the curl_blob struct from libcurl, which seemed a bit much just for this Android feature. OTOH, I don't see any other way to avoid the issue of leaving behind old aggregate certificate files.
If you think it is worth investigating, I can look into it…
| // write a header | ||
| fs.write(""" | ||
| ## Bundle of CA Root Certificates | ||
| ## Auto-generated on \(Date()) |
There was a problem hiding this comment.
You may want to give this a specific format instead of just relying on the debug description.
|
|
||
| try! fs.close() | ||
|
|
||
| aggregateCertPath.withCString { pathPtr in |
There was a problem hiding this comment.
aggregateCertPath should probably be a URL and you can call withUnsafeFileSystemRepresentation here
| if certURL.pathExtension != "0" { continue } | ||
| do { | ||
| if try certURL.resourceValues(forKeys: [.isRegularFileKey]).isRegularFile != true { continue } | ||
| if try certURL.resourceValues(forKeys: [.isReadableKey]).isReadable != true { continue } |
There was a problem hiding this comment.
It's generally preferred to get these resource values in a single call to avoid multiple filesystem calls. You could do something like
guard certURL.pathExtension == "0",
let values = try? certURL.resourceValues(forKeys: [.isRegularFileKey, .isReadableKey]),
values.isRegularFile == true,
values.isReadable == true,
let data = try? Data(contentsOf: certURL) else {
continue
}
try? fs.write(contentsOf: data)
There was a problem hiding this comment.
Actually, Data(contentsOf: certURL) will fail anyway if the URL isn't a regular file or if it isn't readable, so this can be consolidated to a single let data = try? Data(contentsOf: certURL)
There was a problem hiding this comment.
Good idea. I've updated it in bda9bd4
|
@swift-ci test |
This PR will cause
URLSessionto use the installed Android cacerts folders (/apex/com.android.conscrypt/cacertsand/system/etc/security/cacerts), similar to how swift-nio supports TLS on Android (apple/swift-nio-ssl#453). If no customURLSessionCertificateAuthorityInfoFileenvironment variable is set, it will dynamically assemble a certificate file from the contents of those folders, and then instruct curl to use it with the curl_easy_setopt parameter CURLOPT_CAINFO.This has the benefits of: