Skip to content

fix(cpp): honor SSL_CERT_FILE via CURLOPT_CAINFO for TLS on Android - #866

Open
sheetalarkadam wants to merge 5 commits into
mainfrom
android-ssl-cainfo-fix
Open

fix(cpp): honor SSL_CERT_FILE via CURLOPT_CAINFO for TLS on Android#866
sheetalarkadam wants to merge 5 commits into
mainfrom
android-ssl-cainfo-fix

Conversation

@sheetalarkadam

@sheetalarkadam sheetalarkadam commented Jul 8, 2026

Copy link
Copy Markdown

Problem

TLS verification on Android failed for every HTTPS request to Azure, with two different misleading errors:

  • self-signed certificate in certificate chain — catalog requests
  • unable to get local issuer certificate — blob downloads

Both have the same root cause: the trust store was empty, because neither of the two mechanisms that should have supplied it works in this build.

  1. The compiled-in default is wrong. OpenSSL's --openssldir is baked in at build time as a string constant. vcpkg cross-compiles for Android on the host, so the baked path does not exist on a device (Android's store lives at /system/etc/security/cacerts/ as hash-named .0 files).

  2. SSL_CERT_FILE never gets read. It is an OpenSSL-level fallback consulted only by SSL_CTX_set_default_verify_paths() — the "no explicit CA configured" path. libcurl always configures an explicit CA (its own build-time default, via SSL_CTX_load_verify_locations()), so that fallback never runs and the environment variable is silently ignored.

The self-signed certificate in certificate chain wording is a red herring: with an empty store the chain walks up to a root, and every root is self-signed — with nothing to validate it against, that is exactly what OpenSSL reports.

Isolated on-device: passing the device's own trust store via SSL_CERT_FILE failed, while the same bundle passed explicitly as a CA file (openssl s_client -CAfile, equivalent to CURLOPT_CAINFO) verified ai.azure.com with Verify return code: 0 (ok). Same bytes, same device, same endpoint — only the delivery mechanism differed.

Fix

Keep the familiar SSL_CERT_FILE interface, but stop relying on OpenSSL to read it. A shared http::CaBundleFile() helper reads the variable, and each libcurl transport receives it explicitly as CurlTransportOptions.CAInfo (CURLOPT_CAINFO), which libcurl always honors.

The third is the non-obvious one: the Azure Storage SDK builds its own libcurl transport internally rather than reusing ours, so fixing the first two left blob downloads still failing — with the different unable to get local issuer certificate message. MakeBlobClientOptions() injects a preconfigured CurlTransport into options.Transport.Transport.

Every site is guarded by #if !defined(FOUNDRY_LOCAL_USE_WINHTTP_TRANSPORT), so desktop Windows is unaffected — it uses WinHTTP and the OS trust store.

Callers still set SSL_CERT_FILE exactly as before; its effect now comes from the Core forwarding it to CAInfo, not from OpenSSL auto-reading the environment.

Validation

  • Verified on-device: catalog browse + model download over HTTPS to Azure now succeed.

@sheetalarkadam
sheetalarkadam requested a review from Copilot July 8, 2026 02:07
@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview Aug 6, 2026 11:38pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes TLS certificate verification failures on Android (and any platform where the bundled statically-linked libcurl has a non-existent compiled-in default CA path). Previously the caller-provided trust store set via SSL_CERT_FILE was silently ignored, causing every HTTPS request to Azure to fail with "self-signed certificate in certificate chain". The fix reads SSL_CERT_FILE in the SDK core and forwards it explicitly to libcurl through CurlTransportOptions.CAInfo (CURLOPT_CAINFO), which libcurl always honors. The change is additive and guarded to the libcurl (non-WinHTTP) transport path, so desktop Windows (WinHTTP/SChannel) is unaffected.

Changes:

  • In both HttpRequestRaw and HttpDownloadFile, construct CurlTransport with CurlTransportOptions whose CAInfo is populated from SSL_CERT_FILE (only when set and non-empty).
  • Add <cstdlib> include for std::getenv in both files.
  • Correct the AndroidBuildPlan.md SSL section to reflect that neither SSL_CERT_DIR nor SSL_CERT_FILE is auto-consulted, and that the Core forwards SSL_CERT_FILE to CAInfo.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
sdk_v2/cpp/src/http/http_client.cc Forwards SSL_CERT_FILE to CurlTransportOptions.CAInfo for the request path; adds <cstdlib>.
sdk_v2/cpp/src/http/http_download.cc Applies the same CAInfo forwarding for the file-download path; adds <cstdlib>.
sdk_v2/cpp/docs/AndroidBuildPlan.md Updates SSL documentation to describe the corrected mechanism and rationale.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

The bundled libcurl does not consult the SSL_CERT_FILE (or SSL_CERT_DIR)
environment variable at request time: it was built with a compiled-in default
CA path that does not exist on Android, and without the fallback that would
otherwise read SSL_CERT_FILE. As a result the caller-provided trust store was
never used and HTTPS requests to Azure failed verification ('self-signed
certificate in certificate chain' for the catalog, 'unable to get local issuer
certificate' for blob downloads).

Every libcurl-based transport must therefore pass the bundle explicitly via
CURLOPT_CAINFO, which libcurl always honors. Add a shared http::CaBundleFile()
helper (reads SSL_CERT_FILE) and route all four transports through it:
  - http_client.cc  (HttpRequestRaw)      -> CurlTransportOptions.CAInfo
  - http_download.cc (HttpDownloadFile)    -> CurlTransportOptions.CAInfo
  - blob_downloader.cc (ListBlobs +        -> MakeBlobClientOptions() installs a
    DownloadBlob, via the Azure Storage       CurlTransport(CAInfo) on the SDK's
    SDK's own transport)                       BlobClientOptions

The WinHTTP (desktop Windows) path uses the OS trust store and is unaffected.
Also corrects the AndroidBuildPlan.md SSL section, which previously claimed
SSL_CERT_FILE was honored on its own.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@skottmckay

Copy link
Copy Markdown
Collaborator

Suggestions in #955

@sheetalarkadam

sheetalarkadam commented Aug 6, 2026

Copy link
Copy Markdown
Author

Merged Scott McKay (@skottmckay)'s suggestions from #955 into this branch (f1e18314, 5383d24e, 5bc6af8a) — applied as-is, no modifications.

Device-validated. Built arm64-v8a and confirmed the fix works on a physical Android device

On the process-lifetime cache: I raised a concern that a failed CA export would memoize an empty path for the process lifetime, defeating a retry. Having dug into it, I'm dropping that — Scott is right that it's rare, and it doesn't warrant the change:

  • On Android the export failing is unlikely, and the exporter already logs at ERROR when it does.
  • Process death is the common case and it self-heals: the static, the environment variable, and the loaded library all die together, so a bad cached value can never outlive the process that produced it.
  • Across an AIDL disconnect where the process survives, the cached value stays correct — the cache holds a path, not certificate bytes, and that path is a pure function of package + UID, so it is identical before and after.

Keeping the cache as written. It also lets the function keep returning const std::string& safely, since the static is immutable.

<cstdlib> and <utility> were added for a draft that called std::getenv
directly and moved the result. The final version reads the path through
Utils::GetEnv in http_client.cc, so neither header is used here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 24ef037b-246f-4fea-a1a3-4fae5223cb0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants