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

travis: remove expired DST Root CA X3 cert on trusty #2192

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

dgw
Copy link
Member

@dgw dgw commented Oct 6, 2021

Description

Forcibly remove the expired mozilla/DST_Root_CA_X3.crt file and update CA certificate bundle, only on Trusty distro (used for Python 3.3 tests). Also ignore certifi trust store on trusty+py3.3, since certifi maintainers are stubborn (see comments in diff).

Thanks to @half-duplex for helping with the monkey-patch step!

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches
    • Painstakingly: both myself and @half-duplex ran tests in local VMs, and I pushed promising solutions to a travis- prefixed test branch to see how they performed on the real thing.
    • If this PR build fails despite all of our advance testing, I might actually cry.

Notes

Blocks #2190, which can't pass its CI tests until this is fixed.

Considering an alternative approach that replaces the cp with an environment variable (REQUESTS_CA_BUNDLE).

@dgw dgw added this to the 7.1.5 milestone Oct 6, 2021
@dgw dgw requested a review from a team October 6, 2021 05:06
.travis.yml Outdated Show resolved Hide resolved
Copy link
Member

@half-duplex half-duplex left a comment

Choose a reason for hiding this comment

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

Nits, these and REQUESTS_CA_BUNDLE= are cleaner (and severable) but should be functionally identical and this patch dies with 7.x so 🤷

Other thoughts:

  • If anyone is actually using 3.3 in a venv they'll probably have this problem, if we care about fully supporting 3.3 (I don't) it should be documented.
  • Why doesn't this affect newer python? My py3.9 also uses certifi's store in a venv, but works fine... openssl version?
  • Nobody should be using python 3.3 anyway: I am not aware of any supported distro/release without 3.4+ available

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
Rebuild the system CA certificates bundle without this expired root,
and tell `requests` to use that instead of `certifi`.

Co-authored-by: mal <mal@sec.gd>
@dgw
Copy link
Member Author

dgw commented Oct 7, 2021

I applied the suggestions locally to update the comments all in one step and minimize Travis build time.

  • Why doesn't this affect newer python? My py3.9 also uses certifi's store in a venv, but works fine... openssl version?

Probably this, yes.

@dgw dgw merged commit 91652f0 into 7.1.x Oct 8, 2021
@dgw dgw deleted the trusty-expired-LE-root branch October 8, 2021 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants