rustls / rustls-platform-verifier

A certificate verification library for rustls that uses the operating system's verifier
Apache License 2.0
57 stars 18 forks source link

Possible FreeBSD certificate detection failure #104

Closed rami3l closed 4 weeks ago

rami3l commented 1 month ago

Hello there! After the Rustup team's recent attempt to migrate to rustls-platform-verifier (https://github.com/rust-lang/rustup/pull/3903) we have noticed a CI regression as follows:

error: could not download file from 'https://static.rust-lang.org/rustup/release-stable.toml' to '/tmp/rustup-update68roIr/release-stable.toml': failed to make network request: error sending request for url (https://static.rust-lang.org/rustup/release-stable.toml): client error (Connect): received fatal alert: DecodeError

I'm not sure what this DecodeError is. Could it be X.509-related? https://github.com/rust-lang/rustup/issues/3908#issue-2372437430

Interestingly enough, before the migration we have used reqwest's integration of rustls-native-certs v0.7.0 and it didn't fail:

Before (d85502ca):

> cargo tree -i webpki-roots --target=x86_64-unknown-freebsd
error: package ID specification `webpki-roots` did not match any packages

> cargo tree -i rustls-native-certs --target=x86_64-unknown-freebsd
rustls-native-certs v0.7.0
└── reqwest v0.12.4
    └── download v1.27.1
        └── rustup v1.27.1

After (f48df22e):

> cargo tree -i webpki-roots --target=x86_64-unknown-freebsd
warning: nothing to print.

> cargo tree -i rustls-native-certs --target=x86_64-unknown-freebsd
rustls-native-certs v0.7.0
└── rustls-platform-verifier v0.3.1
    └── download v1.27.1
        └── rustup v1.27.1

https://github.com/rust-lang/rustup/issues/3908#issuecomment-2191355535


Comparing the usage of rustls_native_certs::load_native_certs in reqwest and rustls-platform-verifier, I have noticed that here this function is under a guard that is disabled on FreeBSD (https://github.com/rust-lang/rustup/issues/3908#issuecomment-2191368278):

https://github.com/rustls/rustls-platform-verifier/blob/2b3bfbe9789b15748364e5b8c1b7f93848dea26e/rustls-platform-verifier/src/verification/others.rs#L118-L119

Could this be a mistake of some sort? Does it have something to do with the snippet below?

https://github.com/rustls/rustls-platform-verifier/blob/2b3bfbe9789b15748364e5b8c1b7f93848dea26e/rustls-platform-verifier/Cargo.toml#L51-L53

Many thanks in advance!

ctz commented 1 month ago

https://github.com/rustls/rustls-platform-verifier/blob/2b3bfbe9789b15748364e5b8c1b7f93848dea26e/rustls-platform-verifier/src/verification/others.rs#L118-L119

I guess this should be equivalent to the cfg expression that adds the dependency on rustls-native-certs in the first place.

https://github.com/rustls/rustls-platform-verifier/blob/2b3bfbe9789b15748364e5b8c1b7f93848dea26e/rustls-platform-verifier/Cargo.toml#L51-L53

We should probably remove this once rustls/rustls-native-certs#28 is addressed, and also alter our freebsd CI job so it doesn't accidentally work around that bug by installing curl.

rami3l commented 4 weeks ago

I guess this should be equivalent to the cfg expression that adds the dependency on rustls-native-certs in the first place.

@ctz Thanks! I've started an experiment in https://github.com/rust-lang/rustup/pull/3912 to see whether your proposed changes do the job.

Update: It works! I've made #105.