sfackler / rust-native-tls

Apache License 2.0
473 stars 197 forks source link

Newer pkcs12 file format reverses cert chain order #281

Open srstites75 opened 12 months ago

srstites75 commented 12 months ago

I see in many of the Identity::from_pkcs12 functions the comment:

// > The stack is the reverse of what you might expect due to the way

But it appears in the newer pkcs12 file format the chain is coming out reversed. I verified that with the older format, the reversal in the code is necessary, but in the newer format, the reversal is causing the chain to be present in opposite order and thus causing the server to not recognize the client cert/chain.

Note: This is diferent from the other issue rgd the newer pkcs12 format where the cipher used is no longer supported.

sfackler commented 12 months ago

Are you sure the certs aren't in the opposite order in the PKCS#12 archive?

srstites75 commented 12 months ago

yes, I imported the same ca chain into pkcs12 using older and newer openssl and observed the opposite behavior with the following test:

use openssl::pkcs12::Pkcs12;

fn main() -> Result<(),Box>{ let buf = include_bytes!("../../keystore.p12"); let pkcs12 = Pkcs12::from_der(buf)?;

let pass = include_bytes!("../../pass.txt");
let parsed = pkcs12.parse2(String::from_utf8(pass.to_vec())?.as_str())?;
let chain: Vec<_> = parsed.ca.into_iter().flatten().rev().collect();

for c in chain {
    println!("got cert: {:?}", c);
}

Ok(())

}

sunmy2019 commented 7 months ago

Yes. openssl "fixed" the reverse behavior in 3.0 beta2. They now treat the original format as a legacy.

https://github.com/openssl/openssl/issues/6698


I found a PR in other crates to handle this situation. https://github.com/ancwrd1/pki-rs/pull/1

sfackler commented 7 months ago

"Very cool"

It seems like the pki-rs approach is the least-bad option here.