Skip to content

[Networking] Search for CA roots if libcurl doesn't know where they are. #4877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 2, 2024
Merged

[Networking] Search for CA roots if libcurl doesn't know where they are. #4877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c62fb45
[Networking] Search for CA roots if libcurl doesn't know where they are.
al45tair Feb 22, 2024
e55999a
[Networking] Don't search for roots if we don't have CURLINFO_CAINFO.
al45tair Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CoreFoundation/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,10 @@ if(NOT CMAKE_SYSTEM_NAME STREQUAL Darwin)
if((NS_CURL_ASSUME_FEATURES_MISSING) OR (CURL_VERSION_STRING VERSION_LESS "7.30.0"))
add_compile_definitions($<$<COMPILE_LANGUAGE:C>:NS_CURL_MISSING_MAX_HOST_CONNECTIONS>)
endif()

if((NS_CURL_ASSUME_FEATURES_MISSING) OR (CURL_VERSION_STRING VERSION_LESS "7.84.0"))
add_compile_definitions($<$<COMPILE_LANGUAGE:C>:NS_CURL_MISSING_CURLINFO_CAINFO>)
endif()
endif()

add_framework(CFURLSessionInterface
Expand Down
3 changes: 3 additions & 0 deletions CoreFoundation/URL.subproj/CFURLSessionInterface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,9 @@ CFURLSessionInfo const CFURLSessionInfoFTP_ENTRY_PATH = { CURLINFO_FTP_ENTRY_PAT
CFURLSessionInfo const CFURLSessionInfoREDIRECT_URL = { CURLINFO_REDIRECT_URL };
CFURLSessionInfo const CFURLSessionInfoPRIMARY_IP = { CURLINFO_PRIMARY_IP };
CFURLSessionInfo const CFURLSessionInfoAPPCONNECT_TIME = { CURLINFO_APPCONNECT_TIME };
#if !NS_CURL_MISSING_CURLINFO_CAINFO
CFURLSessionInfo const CFURLSessionInfoCAINFO = { CURLINFO_CAINFO };
#endif
CFURLSessionInfo const CFURLSessionInfoCERTINFO = { CURLINFO_CERTINFO };
CFURLSessionInfo const CFURLSessionInfoCONDITION_UNMET = { CURLINFO_CONDITION_UNMET };
CFURLSessionInfo const CFURLSessionInfoRTSP_SESSION_ID = { CURLINFO_RTSP_SESSION_ID };
Expand Down
1 change: 1 addition & 0 deletions CoreFoundation/URL.subproj/CFURLSessionInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ CF_EXPORT CFURLSessionInfo const CFURLSessionInfoFTP_ENTRY_PATH; // CURLINFO_FTP
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoREDIRECT_URL; // CURLINFO_REDIRECT_URL
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoPRIMARY_IP; // CURLINFO_PRIMARY_IP
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoAPPCONNECT_TIME; // CURLINFO_APPCONNECT_TIME
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoCAINFO; // CURLINFO_CAINFO
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoCERTINFO; // CURLINFO_CERTINFO
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoCONDITION_UNMET; // CURLINFO_CONDITION_UNMET
CF_EXPORT CFURLSessionInfo const CFURLSessionInfoRTSP_SESSION_ID; // CURLINFO_RTSP_SESSION_ID
Expand Down
4 changes: 4 additions & 0 deletions Sources/FoundationNetworking/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ if(NOT CMAKE_SYSTEM_NAME STREQUAL Darwin)
if((NS_CURL_ASSUME_FEATURES_MISSING) OR (CURL_VERSION_STRING VERSION_LESS "7.30.0"))
add_compile_definitions(NS_CURL_MISSING_MAX_HOST_CONNECTIONS)
endif()

if((NS_CURL_ASSUME_FEATURES_MISSING) OR (CURL_VERSION_STRING VERSION_LESS "7.84.0"))
add_compile_definitions(NS_CURL_MISSING_CURLINFO_CAINFO)
endif()
endif()

add_library(FoundationNetworking
Expand Down
71 changes: 61 additions & 10 deletions Sources/FoundationNetworking/URLSession/libcurl/EasyHandle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,17 @@ extension _EasyHandle {
_config = config
}

/// Set allowed protocols
/// Set the CA bundle path automatically if it isn't set
///
/// - Note: This has security implications. Not limiting this, someone could
/// redirect a HTTP request into one of the many other protocols that libcurl
/// supports.
/// - SeeAlso: https://curl.haxx.se/libcurl/c/CURLOPT_PROTOCOLS.html
/// - SeeAlso: https://curl.haxx.se/libcurl/c/CURLOPT_REDIR_PROTOCOLS.html
func setAllowedProtocolsToHTTPAndHTTPS() {
let protocols = (CFURLSessionProtocolHTTP | CFURLSessionProtocolHTTPS)
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionPROTOCOLS, protocols).asError()
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionREDIR_PROTOCOLS, protocols).asError()
/// Curl does not necessarily know where to find the CA root bundle,
/// and in that case we need to specify where it is. There was a hack
/// to do this automatically for Android but allowing an environment
/// variable to control the location of the CA root bundle seems like
/// a security issue in general.
///
/// Rather than doing that, we have a list of places we might expect
/// to find it, and search those until we locate a suitable file.
func setCARootBundlePath() {
#if os(Android)
// See https://curl.haxx.se/docs/sslcerts.html
// For SSL on Android you need a "cacert.pem" to be
Expand All @@ -205,8 +205,58 @@ extension _EasyHandle {
else {
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionCAINFO, caInfo).asError()
}
return
}
#endif

#if !NS_CURL_MISSING_CURLINFO_CAINFO
#if !os(Windows) && !os(macOS) && !os(iOS) && !os(watchOS) && !os(tvOS)
// Check if there is a default path; if there is, it will already
// be set, so leave things alone
var p: UnsafeMutablePointer<Int8>? = nil

try! CFURLSession_easy_getinfo_charp(rawHandle, CFURLSessionInfoCAINFO, &p).asError()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tested with curl < 7.84.0 to make sure the NS_CURL_MISSING_CURLINFO_CAINFO properly guards this code? I think it should, but I'd just want to make sure, otherwise I think this will always fatalError since curl will return CURLE_UNKNOWN_OPTION. It might be good to just catch the error here and return instead of force trying.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The original version of this PR actually failed because of this problem, so the guard definitely works.


if p != nil {
return
}

// Otherwise, search a list of known paths
let paths = [
"/etc/ssl/certs/ca-certificates.crt",
"/etc/pki/tls/certs/ca-bundle.crt",
"/usr/share/ssl/certs/ca-bundle.crt",
"/usr/local/share/certs/ca-root-nss.crt",
"/etc/ssl/cert.pem"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, is there reasoning or precedence for this search order? (I don't see any issue with it, I'm just not as familiar.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the list from curl's build system (I assumed that was a good place to look, because it already knows how to do this, just during configuration rather than at runtime).

]

for path in paths {
var isDirectory: ObjCBool = false
if FileManager.default.fileExists(atPath: path,
isDirectory: &isDirectory)
&& !isDirectory.boolValue {
path.withCString { pathPtr in
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionCAINFO, UnsafeMutablePointer(mutating: pathPtr)).asError()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same idea as above making sure with don't get CURLE_UNKNOWN_OPTION.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the we failed in one of the previous iterations because of this, so yes, it's been tested.

}
return
}
}
#endif // !os(Windows) && !os(macOS) && !os(iOS) && !os(watchOS) && !os(tvOS)
#endif // !NS_CURL_MISSING_CURLINFO_CAINFO
}

/// Set allowed protocols
///
/// - Note: This has security implications. Not limiting this, someone could
/// redirect a HTTP request into one of the many other protocols that libcurl
/// supports.
/// - SeeAlso: https://curl.haxx.se/libcurl/c/CURLOPT_PROTOCOLS.html
/// - SeeAlso: https://curl.haxx.se/libcurl/c/CURLOPT_REDIR_PROTOCOLS.html
func setAllowedProtocolsToHTTPAndHTTPS() {
let protocols = (CFURLSessionProtocolHTTP | CFURLSessionProtocolHTTPS)
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionPROTOCOLS, protocols).asError()
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionREDIR_PROTOCOLS, protocols).asError()
setCARootBundlePath()
//TODO: Added in libcurl 7.45.0
//TODO: Set default protocol for schemeless URLs
//CURLOPT_DEFAULT_PROTOCOL available only in libcurl 7.45.0
Expand All @@ -217,6 +267,7 @@ extension _EasyHandle {
let redirectProtocols = (CFURLSessionProtocolHTTP | CFURLSessionProtocolHTTPS)
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionPROTOCOLS, protocols).asError()
try! CFURLSession_easy_setopt_long(rawHandle, CFURLSessionOptionREDIR_PROTOCOLS, redirectProtocols).asError()
setCARootBundlePath()
}

//TODO: Proxy setting, namely CFURLSessionOptionPROXY, CFURLSessionOptionPROXYPORT,
Expand Down