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

Remove expired Let's Encrypt root certificate from Mono containers #3457

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

DasSkelett
Copy link
Member

@DasSkelett DasSkelett commented Sep 30, 2021

Problem

We are seeing hundreds of SSL errors from our indexer for SpaceDock mods, and metadata CI is broken for those as well.

errors

Cause

An old root certificate used by Let's Encrypt (DST Root CA X3) has expired. It has largely been replaced by the ISRG Root X1, which is in the certificate trust stores of most modern systems.
However, to enhance compatibility with older clients (especially Android phones, which don't check whether root certs have expired), the X1 cert has a cross-signature from the old X3.

More information:

(The below is my best guess at where the exact issue with Mono is, I might be wrong)
Most clients don't care about this (e.g. OpenSSL as used by e.g. cURL), they go the certificate chain backwards and stop as soon as they see the X1 cert, which is valid and they trust. Mono however also looks at the cross-signature from X3, and throws an error, because it has expired.

Similar to older, broken OpenSSL versions:

Unfortunately this does not apply to OpenSSL 1.0.2 which always prefers the untrusted chain and if that chain contains a path that leads to an expired trusted root certificate (DST Root CA X3), it will be selected for the certificate verification and the expiration will be reported.

This does not affect my normal Linux installation, as the X3 cert has already been removed from Ubuntu's trust store. Thus Mono "doesn't see" the expired cert and is happy with the valid X1.

Workaround

If my above assumption is correct, it is a bug in Mono that needs fixing. And/or Debian should remove X3 from its trust store.

In the meantime, we can blocklist X3 in the Mono images, which forces Mono and other software to ignore it and only look at the X1 cert instead.
This works by prepending a ! to the relevant line in /etc/ca-certificates.conf and running update-ca-certificates to apply the change.

We should monitor upstream changes and remove the line again when it is no longer needed. FWIW, sed doesn't fail when it didn't change something because the line is no longer present in the file.

@DasSkelett DasSkelett added Bug Something is not working as intended Pull request Infrastructure Issues affecting everything around CKAN (the GitHub repos, build process, CI, ...) Mono Issues specific for Mono Network Issues affecting internet connections of CKAN labels Sep 30, 2021
@HebaruSan
Copy link
Member

Can we add a test somewhere that will exercise this (succeed if the cert is removed, fail if it's present)? Is there one already?

@DasSkelett
Copy link
Member Author

Can we add a test somewhere that will exercise this (succeed if the cert is removed, fail if it's present)? Is there one already?

sed itself doesn't really have a "fail if not present" option (looked that up just yesterday for something else), but we could grep for it, which exits with 1 if it doesn't find anything.

@HebaruSan
Copy link
Member

HebaruSan commented Sep 30, 2021

No, I meant a basic "smoke test" to see whether ckan.exe is able to access a letsencrypt-signed site, such as SpaceDock.

@DasSkelett
Copy link
Member Author

DasSkelett commented Sep 30, 2021

Ah I see. No, I don't think we run any tests in the containers yet. We only run some network tests in the .NET Core CI builds.
We could try to set something up, but it needs some work.

Edit: actually, since the whole CI basically runs inside mono containers, it might not be that hard to do, give me a minute...

@DasSkelett DasSkelett force-pushed the fix/inflator-expired-cert branch from 4c944a2 to e2b8ad2 Compare September 30, 2021 21:29
@DasSkelett DasSkelett force-pushed the fix/inflator-expired-cert branch 3 times, most recently from 9ecb9fb to 3ee130a Compare September 30, 2021 22:54
@DasSkelett DasSkelett force-pushed the fix/inflator-expired-cert branch from 3ee130a to 1973999 Compare September 30, 2021 22:57
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

Let's try it! 🩹

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Infrastructure Issues affecting everything around CKAN (the GitHub repos, build process, CI, ...) Mono Issues specific for Mono Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants