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

finding corrupted certificate on MacOS Catalina #15

Closed
marekyggdrasil opened this issue Jan 14, 2021 · 11 comments
Closed

finding corrupted certificate on MacOS Catalina #15

marekyggdrasil opened this issue Jan 14, 2021 · 11 comments

Comments

@marekyggdrasil
Copy link

Hi! First of all, I'm no rust programmer or anything, I just want a thing that depends on a thing that depends on your thing to start to work so...

There was this problem mimblewimble/grin-wallet#554 and I traced it down to this https://github.com/ctz/hyper-rustls/blob/5a30ca520ab382bdeb06ba37a1401b6f5aeb971f/src/connector.rs#L42

I suspect it is one of my certificates that breaks it. I wanted to make something similar to what has been done here #4 (comment) which is, write a code snipped that shows which certificate breaks, I just have one that reproduces the problem

use rustls_native_certs::load_native_certs;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let _dafuck = match rustls_native_certs::load_native_certs() {
        Ok(store) => store,
        Err((Some(store), err)) => {
            println!("Could not load all certificates: {:?}", err);
            store
        }
        Err((None, err)) => Err(err).expect("cannot access native cert store"),
    };
   Ok(())
}

is there any chance I can detect which certificate breaks it, then I can remove it / update it / whatever and world can become a happy place once again?

Thanks in advance!

@marekyggdrasil marekyggdrasil changed the title finding corrupted certificate on OSX Catalina finding corrupted certificate on MacOS Catalina Jan 14, 2021
@Shnatsel
Copy link

I have also received a report of the same error in my project when run in Mac OS 10.14.6: Could not load platform certs: "The Trust Settings Record was corrupted."

My project uses rustls-native-certs via ureq. Details here: rust-secure-code/cargo-supply-chain#44

@Matthias247
Copy link

is there any chance I can detect which certificate breaks it, then I can remove it / update it / whatever and world can become a happy place once again?

You can use a forked version of the library and add some extra debug printing here to figure out which cert is rejected.

@mefellows
Copy link

I think we just encountered this error, but on Windows.

From what I can see, if there is any invalid certificates on your system it simply bails out with an error. This seems unintuitive because this is exactly the kind of library likely to be buried deep in other frameworks, where the end user will probably not understand the problem nor have any means to rectify. But perhaps I'm missing something?

I'd be open to submitting a PR if there were suggestions around dealing with this.

@adamrodger
Copy link

Looking into the Rustls code, it appears it's essentially expected that some certs in the store will be invalid. The parsing code contains a method with the comments:

/// Parse the given DER-encoded certificates and add all that can be parsed
/// in a best-effort fashion.
///
/// This is because large collections of root certificates often
/// include ancient or syntactically invalid certificates.
///
/// Returns the number of certificates added, and the number that were ignored.

https://github.com/rustls/rustls/blob/633bf4ba9d9521a95f68766d04c22e2b01e68318/rustls/src/anchors.rs#L105-L134

That's the method that should probably be used internally so that invalid certs are skipped.

@adamrodger
Copy link

I've written a sample app to parse my OS cert store in two different ways: one the way that rustls-native-certs will do it and another via x509-parser.

The x509-parser version was able to parse one of the certs that rustls wasn't, although it did appear to have parsed it slightly incorrectly as the subject was incorrect. The rustls version gives the error MissingOrMalformedExtensions:

[2021-10-07T20:01:10Z ERROR rustls_native_test] Failed to parse cert
[2021-10-07T20:01:10Z ERROR rustls_native_test]     Subject: CN=0043004F00520050005C007300720076002D006200750069006C0064002D00630064
[2021-10-07T20:01:10Z ERROR rustls_native_test]     Error: MissingOrMalformedExtensions

The sample app is here: https://github.com/adamrodger/rustls-native-certs-error

I've attached the cert here just in case you want to use it for testing:

-----BEGIN CERTIFICATE-----
MIIB3TCCAUagAwIBAgIQNy/WjTnu4KBEfyDeF6mOLjANBgkqhkiG9w0BAQUFADAt
MSswKQYDVQQDHiIAQwBPAFIAUABcAHMAcgB2AC0AYgB1AGkAbABkAC0AYwBkMB4X
DTE5MDUwMTE1NTMyMFoXDTIwMDQzMDIxNTMyMFowLTErMCkGA1UEAx4iAEMATwBS
AFAAXABzAHIAdgAtAGIAdQBpAGwAZAAtAGMAZDCBnzANBgkqhkiG9w0BAQEFAAOB
jQAwgYkCgYEAtFWjgGjMbSVOCMCWHury61UJZBbESsoM+39j7OBEj8oqDACZW2qG
g9fFpXYcKRinHCb6Xte6YQDs5MxWQAQWLYXyGIHGu+drkS5YioyMo9M4LPIH4h+e
0cDBUW9vHCkM5xguWWRMysgIPqhV0Gly8RRxx8qyCurS4cGjZcSvU/0CAwEAATAN
BgkqhkiG9w0BAQUFAAOBgQBPkkAOKAqCdrIiPELB2qRC67GjVtcNG9jQUBDVPv2g
l1f3/V/nJDa3GKufhb+b3allHmcS+p1ZOgyNC75BuPabFGfIbUw3HgVaCUFExI9k
3aECHiAiX1UtC60H0UyUQOaIoet80ciDwzBg6ou2DNKLp9m5PKYxsnr5ye9+ODlo
7g==
-----END CERTIFICATE-----

@djc
Copy link
Member

djc commented Oct 11, 2021

In current main, rustls-native-certs no longer tries to parse the trust roots it finds, so parsing the roots and handling any errors is left to user code.

@mefellows
Copy link

I just looked at the code and if I'm not mistaken load_native_certs still returns a populated store.

Can you please point me to what you mean?

(I'm on mobile so perhaps I've mislooked).

@djc
Copy link
Member

djc commented Oct 11, 2021

It returns a Vec<Certificate>, where Certificate is just a Vec<u8> newtype.

@mefellows
Copy link

Ah I see, it loads the platform specific function (not the one in lib.rs).

So I think we can resolve in reqwest directly then.

@djc
Copy link
Member

djc commented Oct 11, 2021

I think so, yes.

@cpu
Copy link
Member

cpu commented Mar 31, 2023

Based on the discussion above I believe this issue can be closed. If I'm mistaken please comment and we can reopen for further discussion.

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

No branches or pull requests

7 participants