diff options
author | Amir Omidi <amir@aaomidi.com> | 2022-06-29 07:24:24 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-29 07:24:24 +0300 |
commit | dedbdea1d9854761df9ba28d26e368bdd78d72c9 (patch) | |
tree | 87d50a456684bc6a512b8723b22053810a5949e1 | |
parent | b9f9952660ac63acf7ea5abe729f0d378362e5b4 (diff) |
Update generated CSRs to create V1 CSRs (#9334)
* Update generated CSRs to create V1 CSRs
Per the RFC: https://datatracker.ietf.org/doc/html/rfc2986#section-4
Version 3 CSRs, as far as I can tell, are not a thing (yet).
Relevant code in Go, for example: https://cs.opensource.google/go/go/+/refs/tags/go1.18.3:src/crypto/x509/x509.go;l=1979
* Update AUTHORS.md
* Unit test for PR #9334
* Add a small comment explaining this line for future readers.
* Add info to changelog
Co-authored-by: Paul Buonopane <paul@namepros.com>
-rw-r--r-- | AUTHORS.md | 2 | ||||
-rw-r--r-- | acme/acme/crypto_util.py | 3 | ||||
-rw-r--r-- | acme/tests/crypto_util_test.py | 8 | ||||
-rw-r--r-- | certbot/CHANGELOG.md | 2 |
4 files changed, 14 insertions, 1 deletions
diff --git a/AUTHORS.md b/AUTHORS.md index 9629b1135..d5cb664cb 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -17,6 +17,7 @@ Authors * [Alex Halderman](https://github.com/jhalderm) * [Alex Jordan](https://github.com/strugee) * [Alex Zorin](https://github.com/alexzorin) +* [Amir Omidi](https://github.com/aaomidi) * [Amjad Mashaal](https://github.com/TheNavigat) * [amplifi](https://github.com/amplifi) * [Andrew Murray](https://github.com/radarhere) @@ -200,6 +201,7 @@ Authors * [osirisinferi](https://github.com/osirisinferi) * Patrick Figel * [Patrick Heppler](https://github.com/PatrickHeppler) +* [Paul Buonopane](https://github.com/Zenexer) * [Paul Feitzinger](https://github.com/pfeyz) * [Pavan Gupta](https://github.com/pavgup) * [Pavel Pavlov](https://github.com/ghost355) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index f81d0e592..956366469 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -258,7 +258,8 @@ def make_csr(private_key_pem: bytes, domains: Optional[Union[Set[str], List[str] value=b"DER:30:03:02:01:05")) csr.add_extensions(extensions) csr.set_pubkey(private_key) - csr.set_version(2) + # RFC 2986 Section 4.1 only defines version 0 + csr.set_version(0) csr.sign(private_key, 'sha256') return crypto.dump_certificate_request( crypto.FILETYPE_PEM, csr) diff --git a/acme/tests/crypto_util_test.py b/acme/tests/crypto_util_test.py index 5c2eebc07..0244bf835 100644 --- a/acme/tests/crypto_util_test.py +++ b/acme/tests/crypto_util_test.py @@ -314,6 +314,14 @@ class MakeCSRTest(unittest.TestCase): def test_make_csr_without_hostname(self): self.assertRaises(ValueError, self._call_with_key) + def test_make_csr_correct_version(self): + csr_pem = self._call_with_key(["a.example"]) + csr = OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_PEM, csr_pem) + + self.assertEqual(csr.get_version(), 0, + "Expected CSR version to be v1 (encoded as 0), per RFC 2986, section 4") + class DumpPyopensslChainTest(unittest.TestCase): """Test for dump_pyopenssl_chain.""" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 9327dd9d6..8a54bf94e 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -18,6 +18,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). data, so it doesn't rely on the locally stored account URL. This fixes situations where Certbot would use old ACMEv1 registration info with non-functional account URLs. +* The generated Certificate Signing Requests are now generated as version 1 instead of version 3. This resolves situations in where strict enforcement of PKCS#10 meant that CSRs that were generated as version 3 were rejected. + More details about these changes can be found on our GitHub repo. ## 1.28.0 - 2022-06-07 |