Skip to content
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.

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

Error using nextclade dataset get behind proxy: CURLE_PEER_FAILED_VERIFICATION: SSL peer certificate or SSH remote key was not OK #726

Closed
EricFournier3 opened this issue Feb 9, 2022 · 22 comments · Fixed by #1529
Labels
nextclade_data Concerning dataset part: reference tree, qc definitions etc package: nextclade_cli t:feat Type: request of a new feature, functionality, enchancement v2 To bear in mind when implementing v2

Comments

@EricFournier3
Copy link

EricFournier3 commented Feb 9, 2022

nextclade version 1.10.2 on centos-release-7-4.1708.el7.centos.x86_64

Hi,

when I execute the following command,
nextclade dataset get --name sars-cov-2 --output-dir /home/[email protected]/temp/20220209/TESTNEXTLADE
I get the following error
[ERROR] Nextclade: When fetching a file "https://data.clades.nextstrain.org/index.json": CURLE_PEER_FAILED_VERIFICATION: SSL peer certificate or SSH remote key was not OK
I got the same error when I append insecure in my ~/.curlrc

Actually, our process run under a proxy with ssl full inspection. The non-recognized certificate is on our side. Is there a way like curl to activate a switch to ignore the certificate ?

Can you please help me with this
Best,
Eric

@corneliusroemer
Copy link
Member

I'm not very familiar with proxy servers and how they might interact with curl here. Have you tried setting the following flags?

CURLOPT_SSL_VERIFYPEER=0
CURLOPT_PROXY_SSL_VERIFYPEER=0
CURLOPT_SSL_VERIFYHOST=0
CURLOPT_PROXY_SSL_VERIFYHOST=0
nextclade get ...

I found them here and through StackOverflow: https://www.php.net/manual/en/function.curl-setopt.php

@ivan-aksamentov may know better things to try.

In any case, you can download in any way you like, using wget or curl, that is without using the convenience command nextclade dataset get. The implementation could however change at some point in the future, so it's best to sort out nextclade dataset get. But for now to be able to use datasets go ahead as follows.

All information is contained in this json: https://data.clades.nextstrain.org/index.json

The latest dataset has the field latest: true. The URL to the zip folder is:
wget https://data.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-02-07T12:00:00Z/zip-bundle/nextclade_dataset_sars-cov-2_MN908947_2022-02-07T12:00:00Z.zip

Unzip that into a folder and you're ready to go. You may still run into proxy issues, but at least the problem is simplified to "download a zip file" and you don't need to worry about how Nextclade invokes curl.

@corneliusroemer corneliusroemer added nextclade_data Concerning dataset part: reference tree, qc definitions etc package: nextclade_cli t:ask Type: question, request of information 1 t:talk Type: discussion of the application or the science behind it labels Feb 10, 2022
@corneliusroemer
Copy link
Member

corneliusroemer commented Feb 10, 2022

@stefandiederich had a similar problem being behind a proxy before. Ivan gave some tips back then, but I'm not sure how it got resolved, see here: #552 #532

Maybe he can comment if he sees this. Watching the https://github.com/nextstrain/nextclade_data repo for new releases (maybe twice per month) and manually updating is the worst case. You could script something together that automatically uses the latest datset though.

@corneliusroemer corneliusroemer changed the title Error with nextclade dataset get : nextclade version 1.10.2 on centos-release-7-4.1708.el7.centos.x86_64 Error using nextclade dataset get behind proxy: CURLE_PEER_FAILED_VERIFICATION: SSL peer certificate or SSH remote key was not OK Feb 10, 2022
@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Feb 10, 2022

Hi Eric @EricFournier3,

Sorry, Nextclade does not have the proxy networking functionality and it seems to be out of scope of the project.

You should bring that up with your IT team and tell them that their technical implementation prevents you from doing your job. It's their responsibility to ensure you can do your work effectively. Otherwise why they exist?

Also note that these restrictions might be done on purpose. I.e. they don't want you to download any files from the internet. In that case, you are about to violate some rules, and it's no longer a technical problem - you should seek support of your superiors instead.

If that does not work, you have a few options (from simple and sane to complicated and ridiculous, depending on how bad your situation is):

  • use Nextclade Web
  • use a proxifier - put a special piece of software in between the proxy and nextclade, which will translate whatever is needed to make https requests to the outside world work
  • download the dataset as usual on another machine and carry it over to your worker machine
  • download the dataset from GitHub
  • implement a dataset download script using the tools that work (and are allowed). Start with the index.json file, pick one of the dataset versions and look at the 'files' array. That's what Nextclade does.
  • use a VPN, WireGuard or Tor to bypass the restrictions - this is technically very involved, and is likely to get you into troubles if you are caught. But this is what unreasonably brave people might use in countries with state-sanctioned firewalls.
  • use a bastion host - if you can get access to another machine on the same network as your worker machine, but also has access to the internet, you might be able pipe your traffic through that machine, i.e. using an SSH tunnel, bypassing the proxy

Unfortunately, we don't have bandwidth to support any of these use-cases officially, but feel free to reach out with questions.

If you ever find a solution or a workaround, please post here so that other users could also benefit.

If you or your colleagues have some time, you could also take a look at how proxy support might be implemented in nextclade. Nextclade is currently using libcurl for networking, and I know that curl has a --proxy flag, so pushing through HTTP proxies might be doable. I don't have enough knowledge in this area, and neither I have information on what kind of different proxy configurations you or other users might have. As I don't have any of these proxies handy, I also will not be able to test if it works even if I implement it. Contributions in this area are welcome.

We are planning some mid-term changes in Nextclade which may or may not help to resolve this - the network implementation will be different and it might have a better support for proxies (or not).

@corneliusroemer corneliusroemer added the wontfix This will not be worked on label Feb 10, 2022
@corneliusroemer
Copy link
Member

I'll close this as wontfix but you're of course still welcome to comment @EricFournier3

@EricFournier3
Copy link
Author

Thank you. I will try one of your suggestions

@corneliusroemer
Copy link
Member

It looks like a lot of people have this problem. So while we may not want to fix it in the current Nextclade version 1, maybe this is something to bear in mind when moving to Nextclade v2?
@rneher @ivan-aksamentov
If we rustify the CLI, this may be easier, since we just use a rust crate for http requests instead of C++ interfacing to CURL etc.
I'll therefore reopen, so this doesn't get forgotten.

@corneliusroemer
Copy link
Member

@jacaravas has written a (workaround) shell script to replace nextclade dataset get when the latter doesn't work: https://github.com/jacaravas/update-nextclade-dataset

@tsibley
Copy link
Member

tsibley commented Mar 3, 2022

I don't think the issue here is lack of proxy support. My understanding is that libcurl respects standard proxy environment variables unless they've been explicitly disabled/overridden by libcurl init options (which I assume Nextclade wouldn't be doing).

Instead, I think nextclade's libcurl either…

  1. …isn't using the system CA store (because libcurl is statically compiled into nextclade?), or…
  2. @EricFournier3's proxy's cert's CA isn't in their system CA store (in which case it's likely only in their browsers' local CA stores).

Situation 1 seems most likely to me given the info so far. To fix this and retain the use of the proxy, @EricFournier3 should be able to set the CURL_CA_BUNDLE environment variable to a CA bundle file that includes the proxy's cert's CA. Curl's CA store handling is described a bit more on these two pages:

This situation is fairly common. As an example of this happening elsewhere, @jacaravas ran into a similar issue in nextstrain/ncov with the Python requests library and was able to resolve it by setting request's corresponding env var (REQUESTS_CA_BUNDLE).

@pilem
Copy link

pilem commented Jun 2, 2022

Hi,
It looks like this problem is coming from the openssl crate: https://docs.rs/openssl/latest/openssl/

The vendored copy will not be configured to automatically find the system’s root certificates, but the openssl-probe crate can be used to do that instead.

This could explain why nextclade dataset fails on a properly configured system, where curl and wget work flawlessly in a shell, with https requests going through a transparent proxy (for web filtering). i.e. With the proxy's cert's CA in the system CA store or with environment variables and a CA bundle file.

I'll let you do your excellent development work from here, I'm back to my questionable existence as a sysadmin in an IT team. ;-)

@corneliusroemer
Copy link
Member

Thanks for your idea @pilem!
This bug report is related to v1 which isn't on master anymore. We used to use c++ back then, so no rust at all Impossible to know when reading this issue of course 🙃

@pilem
Copy link

pilem commented Jun 2, 2022

Indeed @corneliusroemer! I will open a new bug report then, since we see the same behaviour with the current master, tag 2.0.0-alpha3.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Jun 2, 2022

Hi @pilem thanks for trying Nextclade v2! How do you invoke the command?

Is there an easy way for us to emulate your environment to be able to reproduce the error? (i.e. a dockerfile, a provisioning script for a VM etc.).

My understanding is that Nextclade via reqwest should use rustls instead of openssl, with the current configuration:

reqwest = { version = "0.11.10", default-features = false, features = ["blocking", "deflate", "gzip", "brotli", "socks", "rustls-tls"]}

Perhaps flipping it to openssl could work?

Alternatively, I think reqwest also has a possibility to pass a blob of CA certs during initialization. If so, how could I obtain it on a typical proxied system? Perhaps I could add a flag and/or to read an env var?

Tangentially related: can you also help us to test Nextclade v2 with SOCKS proxies?

@pilem
Copy link

pilem commented Jun 2, 2022

Hi @ivan-aksamentov,

I did my tests with your instructions for local dev with rustup and cargo, then:

$ cargo run --bin=nextclade -- dataset get --name sars-cov-2 --output-dir /tmp
[2022-06-02 15:14:36.966][nextclade][WARN ] /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/rustls-0.20.6/src/conn.rs:1285: Sending fatal alert BadCertificate
Error: 
   0: When downloading dataset index
   1: error sending request for url (https://data.master.clades.nextstrain.org/index_v2.json): error trying to connect: invalid peer certificate contents: invalid peer certificate: UnknownIssuer
   2: error trying to connect: invalid peer certificate contents: invalid peer certificate: UnknownIssuer
   3: invalid peer certificate contents: invalid peer certificate: UnknownIssuer

Location:
   packages_rs/nextclade-cli/src/io/http_client.rs:91

For info: It works fine when outside our corporate network and also curl works normally on our servers with our own root CA and certs (used by the proxy) configured on the system.

It might be difficult to create a similar setup, you'll need a transparent web filtering device, or software, with custom trusted CA, and port forwarding on the default route. It's all feasible, but it would be a lot easier to look for system calls with strace. Simply grep etc to see system's calls to the server configs:

$ strace ./target/debug/nextclade dataset get --name sars-cov-2 --output-dir /tmp 2>&1 | grep etc
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
access("/etc/sysconfig/strcasecmp-nonascii", F_OK) = -1 ENOENT (No such file or directory)
access("/etc/sysconfig/strcasecmp-nonascii", F_OK) = -1 ENOENT (No such file or directory)

As you can see, no calls to ssl/tls/cert/pki configs are made.

With curl, as an example, on the same server:

$ strace curl -o /dev/null  https://data.master.clades.nextstrain.org/index_v2.json  2>&1 | grep etc | egrep "pki|ssl|cert"
open("/etc/pki/tls/legacy-settings", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/etc/pki/nssdb", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/etc/pki/nssdb/pkcs11.txt", O_RDONLY) = 4
... (many more)

As I understand it, the problem comes with the vendoring features of cargo. When vendored, rustls or native-tlsare replaced by a statically linked build of openssl, as documented here: https://docs.rs/native-tls/latest/native_tls/. And without openssl-probe functions, only the embedded root CA certs are used, not the system's one.

I can give a hand to test SOCKS proxy. This is easy to setup, only a container and environment variables are required, like https://hub.docker.com/r/serjs/go-socks5-proxy/.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Jun 2, 2022

@pilem Have you tried the new --proxy* command line flags? I think that reqwest does not look for proxies by default and needs to be told explicitly, so I added the flags and pass this information on to reqwest. (And in order to improve on that feature, is there a best practice for automated detection of HTTP proxies? i.e. perhaps through env vars? Perhaps the one that curl uses?)

Can you tell me more about your config? In particular:

  • is it an HTTPS proxy where just the connetion to the proxy server is encrypted
  • or is this some sort of MITM config, where you force HTTP clients to use your local crt authority for requests to the destination server
  • or both

I am really no expert on either, but I believe is that the implementation of client software is quite different for these two cases.

If Nextclade still does not work with the proxy flags, then how can I detect the path to your custom cert file? If you have an env var with the path, or can provide it through a flag, then I might try to add a call to reqwest::ClientBuilder::add_root_certificate(). Do you think it may work?

Update: I now see that I can use the https://github.com/alexcrichton/openssl-probe crate to find out the cert paths.

Update2: And perhaps also the https://github.com/inejge/env_proxy to detect proxies.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Jun 2, 2022

When vendored, rustls or native-tls are replaced by a statically linked build of openssl

I don't believe we use native-tls vendoring. That would require turning on this feature:
https://github.com/seanmonstar/reqwest/blob/28840afd46fe3b81b7c77dde4537ad702826c7f7/Cargo.toml#L38

In fact, if I understand correctly, we don't use native-tls at all and it should always be rustls, which does not have vendoring and not using openssl library.

@ivan-aksamentov ivan-aksamentov removed wontfix This will not be worked on t:ask Type: question, request of information 1 t:talk Type: discussion of the application or the science behind it labels Jun 2, 2022
@ivan-aksamentov
Copy link
Member

I will be away for a few days, but if someone wants to take a look during that time, then the relevant code is:

  • HttpClient::new() configures the undelrying implementation (reqwest), including proxy params and I guess the certs can be added there as well.
    impl HttpClient {
    pub fn new(root: Url, proxy_conf: ProxyConfig, verbose: bool) -> Result<Self, Report> {
    let mut client_builder = Client::builder();
    client_builder = if let Some(proxy_url) = proxy_conf.proxy {
    let proxy = match (proxy_conf.proxy_user, proxy_conf.proxy_pass) {
    (Some(proxy_user), Some(proxy_pass)) => {
    let proxy = Proxy::all(proxy_url)?.basic_auth(&proxy_user, &proxy_pass);
    Ok(proxy)
    }
    (None, None) => {
    let proxy = Proxy::all(proxy_url)?;
    Ok(proxy)
    }
    _ => make_internal_error!("`--proxy-user` and `--proxy-pass` must be either both specified or both omitted"),
    }?;
    client_builder.proxy(proxy)
    } else {
    client_builder
    };
    let user_agent = format!("{} {}", getenv!("CARGO_PKG_NAME"), getenv!("CARGO_PKG_VERSION"));
    let client = client_builder
    .connection_verbose(verbose)
    .connect_timeout(Some(std::time::Duration::from_secs(60)))
    .user_agent(user_agent)
    .build()?;
    Ok(Self { client, root })
    }
  • The new command line args can be added just above it in the struct ProxyConfig. The new fields will become CLI args for dataset get and dataset list commands automagically.
    #[derive(Parser, Debug)]
    #[clap(verbatim_doc_comment)]
    pub struct ProxyConfig {
    /// Pass all traffic over proxy server. HTTP, HTTPS, and SOCKS5 proxies are supported.
    #[clap(long, short = 'x')]
    #[clap(value_hint = ValueHint::Url)]
    pub proxy: Option<Url>,
    /// Username for basic authentication on proxy server, if applicable. Only valid when `--proxy` is also supplied. `--proxy-user` and `--proxy-pass` must be either both specified or both omitted
    #[clap(long)]
    #[clap(value_hint = ValueHint::Other)]
    pub proxy_user: Option<String>,
    /// Password for basic authentication on proxy server, if applicable. Only valid when `--proxy` is also supplied. `--proxy-user` and `--proxy-pass` must be either both specified or both omitted
    #[clap(long)]
    #[clap(value_hint = ValueHint::Other)]
    pub proxy_pass: Option<String>,
    }
  • reqwest crate config. Here we disable default features, including native-tls and then only enable rustls-tls
    reqwest = { version = "0.11.10", default-features = false, features = ["blocking", "deflate", "gzip", "brotli", "socks", "rustls-tls"]}

@pilem
Copy link

pilem commented Jun 2, 2022

Our setup is a transparent MITM web filtering configured on the network equipment. So no need for --proxy flag in our case. http/https requests use the default route with the default ports.

The ssl/tls chain is broken by this filtering as https requests are remade by the transparent proxy, with its private certificate for internal communication.

We use standard system path, but you've already found that.

@pilem
Copy link

pilem commented Jun 3, 2022

I've just found this proxy tool, mitmproxy, that does https with a private cert with basically no config. It could be useful for testing as it is easy to reproduce the same error as above with it.

Installation and launch, use a separate xterm or a tmux/screen session as mitmproxy opens a text console.

$ wget https://snapshots.mitmproxy.org/8.1.0/mitmproxy-8.1.0-linux.tar.gz
$ tar xf mitmproxy-8.1.0-linux.tar.gz
$ ./mitmproxy

Import the mitmproxy generated CA cert in the host CA trusted store. Assuming Debian based distribution:

$ sudo cp ~/.mitmproxy/mitmproxy-ca-cert.cer /usr/local/share/ca-certificates/mitmproxy-ca-cert.crt
$ sudo update-ca-certificates

Test with curl, look at the mitmproxy console for confirmation:

$ https_proxy=https://localhost:8080 curl https://pilem.info
ok

Test with nextclade:

$ cd nextclade
$ https_proxy=https://localhost:8080 ./target/debug/nextclade dataset get --name sars-cov-2 --output-dir /tmp
[2022-06-03 09:41:17.621][nextclade][WARN ] /home/pierre/.cargo/registry/src/github.com-1ecc6299db9ec823/rustls-0.20.6/src/conn.rs:1285: Sending fatal alert BadCertificate
Error: 
   0: When downloading dataset index
   1: error sending request for url (https://data.master.clades.nextstrain.org/index_v2.json): error trying to connect: invalid peer certificate contents: invalid peer certificate: UnknownIssuer
   2: error trying to connect: invalid peer certificate contents: invalid peer certificate: UnknownIssuer
   3: invalid peer certificate contents: invalid peer certificate: UnknownIssuer

Location:
   packages_rs/nextclade-cli/src/io/http_client.rs:91

Look at mitmproxy event log («E» on the text console):

warn: 127.0.0.1:33724: Client TLS handshake failed. The client does not trust the proxy's certificate for localhost (sslv3 alert bad certificate)

@pilem
Copy link

pilem commented Jun 7, 2022

Using reqwest's default-features fix this issue @ivan-aksamentov, @corneliusroemer. I don't know if it breaks something else.

diff --git a/packages_rs/nextclade-cli/Cargo.toml b/packages_rs/nextclade-cli/Cargo.toml
index 0f933e6c..659fefb5 100644
--- a/packages_rs/nextclade-cli/Cargo.toml
+++ b/packages_rs/nextclade-cli/Cargo.toml
@@ -33,7 +33,7 @@ owo-colors = "3.3.0"
 pretty_assertions = "1.2.1"
 rayon = "1.5.2"
 regex = "1.5.5"
-reqwest = { version = "0.11.10", default-features = false, features = ["blocking", "deflate", "gzip", "brotli", "socks", "rustls-tls"]}
+reqwest = { version = "0.11.10", default-features = true, features = ["blocking", "deflate", "gzip", "brotli", "socks", "rustls-tls"]}
 semver = "1.0.9"
 serde = { version = "1.0.136", features = ["derive"] }
 url = { version = "2.2.2", features = ["serde"] }

This request works with mitmproxy and a self signed certificate:

https_proxy=https://localhost:8080 ./target/debug/nextclade dataset get --name sars-cov-2 --output-dir /tmp

mitmproxy logs:

127.0.0.1:48190: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/genemap.gff
              << 200 OK 417b
127.0.0.1:48200: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/qc.json
              << 200 OK 760b
127.0.0.1:48222: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/virus_properties.json
              << 200 OK 4.3k
127.0.0.1:48240: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/reference.fasta
              << 200 OK 9.3k
127.0.0.1:48252: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/sequences.fasta
              << 200 OK 741k
127.0.0.1:48190: GET https://data.master.clades.nextstrain.org/datasets/sars-cov-2/references/MN908947/versions/2022-06-03T12:00:00Z/files/tree.json
              << 200 OK 303k

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Jun 8, 2022

@pilem Thanks for checking! Does it also work with your production hardware setup you mentioned previously?

Enabling default features in reqwest means on Linux it will use openssl (even if rustls feature is also enabled). So it requires openssl installed during build and perhaps even on runtime.

There is a few effects:

  • one cannot cross-compile from macOS to Linux (there is this use-case in the team internally), without openssl Linux binaries, which is a major pain
  • devs need to install openssl before building
  • not all Linux distros have openssl installed by default, and some specifically avoid it as their feature, plus there may be discrepancies in versions. So things may go bad if it requires openssl shared lib on runtime

So I cannot decide how to proceed.

We could:

  1. try and tweak rustls manually until it sees the certs and proxies
  2. add a cargo feature to Nextclade which allows to select ssl implementation before build (and which one will be the default? should we then ship 2 versions of binaries? and what about the existing gnu vs musl flavors? 2 * 2 = 4 Linux binaries then?)

I'll think about it and I think I am mostly up for solution (1). However it was never my intent to write and maintain this functionality for a bioinformatics tool. It's way out of scope.

@pilem
Copy link

pilem commented Jun 8, 2022

It works well with our production hardware @ivan-aksamentov.
I understand for the cross-compile trouble, but that's one of the things VM and containers are for. And I wouldn't worry about the two other points. There maybe are better alternatives, but openssl is not going away soon. And it is actually rustls/reqwest/cargo/rust responsibility to make working feature builds, and they do with the default features.

tsibley added a commit to nextstrain/docs.nextstrain.org that referenced this issue Oct 9, 2024
This information is a distillation of guidance and assistance I've
provided to CDC and other downstream users over the years.  I extracted
it from my various notes and recollections and verified some specific
behaviour anew.  An official reference page will be useful for pointing
users to in the future, and serves as a collection point for future
recommendations/information uncovered in troubleshooting sessions.

Related-to: <nextstrain/ncov#1055>
Related-to: <nextstrain/nextclade#726>
tsibley added a commit to nextstrain/docs.nextstrain.org that referenced this issue Oct 10, 2024
This information is a distillation of guidance and assistance I've
provided to CDC and other downstream users over the years.  I extracted
it from my various notes and recollections and verified some specific
behaviour anew.  An official reference page will be useful for pointing
users to in the future, and serves as a collection point for future
recommendations/information uncovered in troubleshooting sessions.

Related-to: <nextstrain/ncov#774>
Related-to: <nextstrain/ncov#1055>
Related-to: <nextstrain/nextclade#726>
tsibley added a commit to nextstrain/docs.nextstrain.org that referenced this issue Oct 10, 2024
This information is a distillation of guidance and assistance I've
provided to CDC and other downstream users over the years.  I extracted
it from my various notes and recollections and verified some specific
behaviour anew.  An official reference page will be useful for pointing
users to in the future, and serves as a collection point for future
recommendations/information uncovered in troubleshooting sessions.

Related-to: <nextstrain/ncov#774>
Related-to: <nextstrain/ncov#1055>
Related-to: <nextstrain/nextclade#726>
tsibley added a commit that referenced this issue Oct 14, 2024
Use rustls-platform-verifier to verify certificates with the
OS/platform's standard methods, including the system's standard CA trust
store.  See rustls-platform-verifier's documentation for details on each
platform.¹

Previously, the system's trust store was ignored in favor of a baked in
and unconfigurable trust store provided by webpki-roots.

Updates reqwest as necessary to ensure a single compatible rustls
version remains in use.  Updates wasm-bindgen as necessary for the newer
reqwest.

¹ <https://github.com/rustls/rustls-platform-verifier/blob/v/0.3.4/README.md>

Resolves: <#726>
@tsibley
Copy link
Member

tsibley commented Oct 14, 2024

@pilem Check out #1527. If your private CA certificate is in the system's trust store, it should Just Work™. Or you can configure it as an additional trusted CA certificate by setting NEXTCLADE_EXTRA_CA_CERTS=/path/to/certs.pem when running nextclade.

tsibley added a commit that referenced this issue Oct 15, 2024
Use rustls-platform-verifier to verify certificates with the
OS/platform's standard methods, including the system's standard CA trust
store.  See rustls-platform-verifier's documentation for details on each
platform.¹

Previously, the system's trust store was ignored in favor of a baked in
and unconfigurable trust store provided by webpki-roots.

Updates reqwest as necessary to ensure a single compatible rustls
version remains in use.  Updates wasm-bindgen as necessary for the newer
reqwest.

¹ <https://github.com/rustls/rustls-platform-verifier/blob/v/0.3.4/README.md>

Resolves: <#726>
tsibley added a commit that referenced this issue Oct 15, 2024
Use rustls-platform-verifier to verify certificates with the
OS/platform's standard methods, including the system's standard CA trust
store.  See rustls-platform-verifier's documentation for details on each
platform.¹

Previously, the system's trust store was ignored in favor of a baked in
and unconfigurable trust store provided by webpki-roots.

Updates reqwest as necessary to ensure a single compatible rustls
version remains in use.  Updates wasm-bindgen as necessary for the newer
reqwest.

¹ <https://github.com/rustls/rustls-platform-verifier/blob/v/0.3.4/README.md>

Resolves: <#726>
Related-to: <#1527>
tsibley added a commit that referenced this issue Oct 15, 2024
Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

Currently requires using landed but unreleased changes to
rustls-platform-verifier so that macOS and Windows have the needed
Verifier::new_with_extra_certs() API.  Notably, that API also changes
signature between the latest released version (0.3.4) and the Git
revision we're using.

Related-to: <#1529>
Related-to: <#726>
tsibley added a commit that referenced this issue Oct 15, 2024
Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

Currently requires using landed unreleased changes to
rustls-platform-verifier so that macOS and Windows have the needed
Verifier::new_with_extra_certs() API.¹  That API also has signature
changes proposed² which, while unmerged, seem likely to land, so use
those in anticipation.  They actually bring the signature back closer to
the latest release (0.3.4).

¹ <rustls/rustls-platform-verifier#58>
² <rustls/rustls-platform-verifier#145>

Related-to: <#1529>
Related-to: <#726>
tsibley added a commit that referenced this issue Oct 15, 2024
Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

Currently requires using landed but unreleased changes to
rustls-platform-verifier so that macOS and Windows have the needed
Verifier::new_with_extra_certs() API.¹  That API also has signature
changes proposed² which, while unmerged, seem likely to land, so use
those in anticipation.  They actually bring the signature back closer to
the latest release (0.3.4).

¹ <rustls/rustls-platform-verifier#58>
² <rustls/rustls-platform-verifier#145>

Related-to: <#1529>
Related-to: <#726>
tsibley added a commit that referenced this issue Oct 16, 2024
…L trust store

Previously, the platform's trust store was ignored in favor of a baked
in and unconfigurable trust store provided by webpki-roots.  Now the
reqwest trust store will contain both certs obtained from the platform
at run time as well as certs baked in via webpki-roots.

Obtaining certs from the platform means that Nextclade will respect
OS-level configuration to trust private CAs / self-signed certs.
Keeping webpki-roots for all platforms is a precaution that makes this
change merely additive for backwards compatibility, in case of platforms
which lack a trust store (like some Linux containers) or platforms with
out-of-date trust stores.  It means that Nextclade binaries should
continue to Just Work™.

reqwest uses rustls-native-roots to obtain trusted CA certificates from
the standard trust stores for the OS/platform.  See the crate's
documentation for details on each platform.¹  Notably, this does not use
the platform's standard certificate verification methods like
rustls-platform-verifier; it just extracts certificates.  We may in the
future want to switch to rustls-platform-verifier (ourselves or by
waiting for reqwest to do so).

Updates reqwest because an earlier (but problematic and now reverted²)
change did so and there were some public API changes I'd like to use.
Updates wasm-bindgen as necessary for the newer reqwest (≥0.2.89) and
then a little further (0.2.93) to avoid Clippy warnings.³

¹ <https://docs.rs/crate/rustls-native-certs/0.8.0>
² <#1529 (comment)>.
³ <rustwasm/wasm-bindgen#3985>

Resolves: <#726>
Related-to: <#1529>
Related-to: <#1527>
tsibley added a commit that referenced this issue Oct 16, 2024
…d --extra-ca-certs option

Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

An option may be preferred to an env var by some users or in some
invocations.  It also provides a convenient place for documentation of
default CA cert handling and point of discovery for users.

Co-authored-by: ivan-aksamentov <[email protected]>
Supersedes: <#1527>
Related-to: <#1529>
Related-to: <#726>
tsibley added a commit that referenced this issue Oct 16, 2024
…d --extra-ca-certs option

Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

An option may be preferred to an env var by some users or in some
invocations.  It also provides a convenient place for documentation of
default CA cert handling and point of discovery for users.

Co-authored-by: ivan-aksamentov <[email protected]>
Supersedes: <#1527>
Related-to: <#1529>
Related-to: <#726>
tsibley added a commit that referenced this issue Oct 16, 2024
…d --extra-ca-certs option

Allows adding additional CA certificates to the trust store specifically
for Nextclade, for when modifying the system's trust store isn't
desirable/possible.  Works across all platforms.

An option may be preferred to an env var by some users or in some
invocations.  It also provides a convenient place for documentation of
default CA cert handling and point of discovery for users.

Co-authored-by: ivan-aksamentov <[email protected]>
Supersedes: <#1527>
Related-to: <#1529>
Related-to: <#726>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nextclade_data Concerning dataset part: reference tree, qc definitions etc package: nextclade_cli t:feat Type: request of a new feature, functionality, enchancement v2 To bear in mind when implementing v2
Projects
No open projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

5 participants