From 81916ba1b9ca55133adfc233980e3a80b60addff Mon Sep 17 00:00:00 2001 From: "pyca-boringbot[bot]" <106132319+pyca-boringbot[bot]@users.noreply.github.com> Date: Mon, 8 May 2023 23:03:06 -0400 Subject: [PATCH 1/7] Bump BoringSSL and/or OpenSSL in CI (#8893) Co-authored-by: pyca-boringbot[bot] --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a97b41f1d492..e066969acb8dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,10 +41,10 @@ jobs: - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "libressl", VERSION: "3.7.2"}} - {VERSION: "3.11", NOXSESSION: "tests-randomorder"} - {VERSION: "3.12-dev", NOXSESSION: "tests"} - # Latest commit on the BoringSSL master branch, as of May 06, 2023. - - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "boringssl", VERSION: "b1c6f45f1fe6d808555d04a41bb44b322e4f4c1d"}} - # Latest commit on the OpenSSL master branch, as of May 06, 2023. - - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "openssl", VERSION: "6aeb42eca97227c8235af0986d1525ee4a916504"}} + # Latest commit on the BoringSSL master branch, as of May 09, 2023. + - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "boringssl", VERSION: "2aae3f58b42e75690f28853f712a2e204857b7f6"}} + # Latest commit on the OpenSSL master branch, as of May 09, 2023. + - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "openssl", VERSION: "3868807d2fe5a72aa897ce5f7f7ba7e9cc3c09cb"}} timeout-minutes: 15 steps: - uses: actions/checkout@v3.5.2 From 6bb05529a49a0ce532b7e4bf65ac6246d4d57e91 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 9 May 2023 03:37:12 +0000 Subject: [PATCH 2/7] Bump quote from 1.0.26 to 1.0.27 in /src/rust (#8894) Bumps [quote](https://github.com/dtolnay/quote) from 1.0.26 to 1.0.27. - [Release notes](https://github.com/dtolnay/quote/releases) - [Commits](https://github.com/dtolnay/quote/compare/1.0.26...1.0.27) --- updated-dependencies: - dependency-name: quote dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- src/rust/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index d76c498485ba5..9fdd2313155b2 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -354,9 +354,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.26" +version = "1.0.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" +checksum = "8f4f29d145265ec1c483c7c654450edde0bfe043d3938d6972630663356d9500" dependencies = [ "proc-macro2", ] From 3b8cb2b7337a7dc3a004555fd4bfa032e35a34af Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 9 May 2023 02:29:16 -0400 Subject: [PATCH 3/7] Don't install coverage, it's not needed (#8895) --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e066969acb8dc..b06ae9771f570 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,7 +105,7 @@ jobs: # pypy3-3.8 and pypy3-3.9 -- both of them show up as 7.3.11. key: ${{ matrix.PYTHON.VERSION }}-${{ steps.setup-python.outputs.python-version }}-${{ matrix.PYTHON.NOXSESSION }}-${{ env.OPENSSL_HASH }} - - run: python -m pip install -c ci-constraints-requirements.txt 'nox' coverage[toml] + - run: python -m pip install -c ci-constraints-requirements.txt 'nox' - name: Create nox environment run: | nox -v --install-only @@ -182,7 +182,7 @@ jobs: - run: | echo "OPENSSL_FORCE_FIPS_MODE=1" >> $GITHUB_ENV if: matrix.IMAGE.FIPS - - run: /venv/bin/python -m pip install -c ci-constraints-requirements.txt 'nox' coverage + - run: /venv/bin/python -m pip install -c ci-constraints-requirements.txt 'nox' - run: '/venv/bin/nox -v --install-only -s tests' env: RUSTUP_HOME: /root/.rustup @@ -229,7 +229,7 @@ jobs: - name: Clone wycheproof timeout-minutes: 2 uses: ./.github/actions/wycheproof - - run: python -m pip install -c ci-constraints-requirements.txt 'nox' coverage[toml] + - run: python -m pip install -c ci-constraints-requirements.txt 'nox' - name: Create nox environment run: nox -v --install-only -s tests env: @@ -304,7 +304,7 @@ jobs: - name: Clone wycheproof timeout-minutes: 2 uses: ./.github/actions/wycheproof - - run: python -m pip install -c ci-constraints-requirements.txt 'nox' coverage[toml] cffi + - run: python -m pip install -c ci-constraints-requirements.txt 'nox' cffi - name: Create nox environment run: nox -v --install-only -s tests env: @@ -379,7 +379,7 @@ jobs: python-version: ${{ matrix.PYTHON.VERSION }} architecture: 'x64' # we force this right now so that it will install the universal2 on arm64 - - run: python -m pip install -c ci-constraints-requirements.txt 'nox' coverage[toml] + - run: python -m pip install -c ci-constraints-requirements.txt 'nox' - name: Clone wycheproof timeout-minutes: 2 @@ -442,7 +442,7 @@ jobs: timeout-minutes: 2 with: key: ${{ matrix.PYTHON.NOXSESSION }}-${{ matrix.WINDOWS.ARCH }}-${{ steps.setup-python.outputs.python-version }} - - run: python -m pip install -c ci-constraints-requirements.txt "nox" coverage[toml] + - run: python -m pip install -c ci-constraints-requirements.txt "nox" - uses: dawidd6/action-download-artifact@246dbf436b23d7c49e21a7ab8204ca9ecd1fe615 with: From 1f883568e5ab88fa34a6d041195d368bff5dc702 Mon Sep 17 00:00:00 2001 From: "pyca-boringbot[bot]" <106132319+pyca-boringbot[bot]@users.noreply.github.com> Date: Wed, 10 May 2023 00:17:59 +0000 Subject: [PATCH 4/7] Bump BoringSSL and/or OpenSSL in CI (#8897) Co-authored-by: pyca-boringbot[bot] --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b06ae9771f570..41fac5d7beffd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,8 +43,8 @@ jobs: - {VERSION: "3.12-dev", NOXSESSION: "tests"} # Latest commit on the BoringSSL master branch, as of May 09, 2023. - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "boringssl", VERSION: "2aae3f58b42e75690f28853f712a2e204857b7f6"}} - # Latest commit on the OpenSSL master branch, as of May 09, 2023. - - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "openssl", VERSION: "3868807d2fe5a72aa897ce5f7f7ba7e9cc3c09cb"}} + # Latest commit on the OpenSSL master branch, as of May 10, 2023. + - {VERSION: "3.11", NOXSESSION: "tests", OPENSSL: {TYPE: "openssl", VERSION: "8c63b14296f117b07781509ced529a8955d78fb9"}} timeout-minutes: 15 steps: - uses: actions/checkout@v3.5.2 From 9dfb1200948523046d6996f0cd81c7fec2060ab6 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 9 May 2023 20:25:05 -0400 Subject: [PATCH 5/7] Added a missing rerun-if stanza (#8899) --- src/rust/cryptography-cffi/build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rust/cryptography-cffi/build.rs b/src/rust/cryptography-cffi/build.rs index 9a93b50bc4380..4a40990b9da47 100644 --- a/src/rust/cryptography-cffi/build.rs +++ b/src/rust/cryptography-cffi/build.rs @@ -25,6 +25,7 @@ fn main() { let out_dir = env::var("OUT_DIR").unwrap(); // FIXME: maybe pyo3-build-config should provide a way to do this? let python = env::var("PYO3_PYTHON").unwrap_or_else(|_| "python3".to_string()); + println!("cargo:rerun-if-env-changed=PYO3_PYTHON"); println!("cargo:rerun-if-changed=../../_cffi_src/"); let output = Command::new(&python) .env("OUT_DIR", &out_dir) From c6887af98236de1343def4544282812b60b3a383 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Wed, 10 May 2023 09:25:18 +0900 Subject: [PATCH 6/7] update cache key to reflect all rust files, not just cargo.lock (#8898) rust uses mtime to determine if files are fresh or not. However, if the mtime of a file in main is newer than the mtime of a commit in a PR then it will load the cache and there will be weird errors since it thinks the cache is new enough but in reality the code has changed. This change ties our cache keys to all our rust files, not just our cargo.lock, and should resolve this issue. --- .github/actions/cache/action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/cache/action.yml b/.github/actions/cache/action.yml index 3e8c300d03e1c..37b9cc81bd37e 100644 --- a/.github/actions/cache/action.yml +++ b/.github/actions/cache/action.yml @@ -43,11 +43,11 @@ runs: ~/.cargo/registry/cache/ src/rust/target/ ${{ inputs.additional-paths }} - key: cargo-pip-${{ runner.os }}-${{ runner.arch }}-${{ inputs.key }}-3-${{ hashFiles('**/Cargo.lock') }}-${{ steps.rust-version.version }} + key: cargo-pip-${{ runner.os }}-${{ runner.arch }}-${{ inputs.key }}-3-${{ hashFiles('**/Cargo.lock', '**/*.rs') }}-${{ steps.rust-version.version }} - name: Size of cache items run: | du -sh ~/.cargo/registry/index/ du -sh ~/.cargo/registry/cache/ du -sh src/rust/target/ shell: bash - if: ${{ steps.cache.outputs.cache-hit }} \ No newline at end of file + if: ${{ steps.cache.outputs.cache-hit }} From 1ff6208ec739b27ae2826d866f4d2bd3db77fd87 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Wed, 10 May 2023 07:14:49 -0400 Subject: [PATCH 7/7] certificate: add a `get_extension` helper (#8892) * certificate: add a `get_extension` helper Signed-off-by: William Woodruff * certificate: OID by ref Signed-off-by: William Woodruff * certificate: syntax Signed-off-by: William Woodruff * x509, src: `check_duplicate_extensions` Signed-off-by: William Woodruff * src: simplify Signed-off-by: William Woodruff * src: everyone loves newtypes Signed-off-by: William Woodruff * rust: refactor-o-rama Signed-off-by: William Woodruff * src: look upon my works Signed-off-by: William Woodruff * src: continue blasting the code Signed-off-by: William Woodruff * src/rust: actually commit my changes Signed-off-by: William Woodruff * src: clippage Signed-off-by: William Woodruff * relocate Signed-off-by: William Woodruff * src: dedupe Signed-off-by: William Woodruff * src: cleanup Signed-off-by: William Woodruff * clippage Signed-off-by: William Woodruff * src: dedupe Signed-off-by: William Woodruff * common: cleanup Signed-off-by: William Woodruff * src: unused impls Signed-off-by: William Woodruff * more deletion Signed-off-by: William Woodruff * clippage Signed-off-by: William Woodruff * extensions: add a `get_extension` test Signed-off-by: William Woodruff * extensions: unused derives Signed-off-by: William Woodruff * tests/x509: dup ext check for tbs_precertificate_bytes Signed-off-by: William Woodruff * certificate: remove `extensions()` Signed-off-by: William Woodruff * extensions: docs Signed-off-by: William Woodruff * extensions: newtype Signed-off-by: William Woodruff * rust: better error types, dedupe Signed-off-by: William Woodruff extensions: unwrap -> expect Signed-off-by: William Woodruff * Revert "rust: better error types, dedupe" This reverts commit 212b75ff2f69a3b3cfc9d6a55949f23877f8f618. --------- Signed-off-by: William Woodruff --- src/rust/cryptography-x509/src/certificate.rs | 9 +- src/rust/cryptography-x509/src/crl.rs | 10 ++- src/rust/cryptography-x509/src/csr.rs | 2 +- src/rust/cryptography-x509/src/extensions.rs | 83 ++++++++++++++++++- src/rust/cryptography-x509/src/ocsp_req.rs | 10 ++- src/rust/cryptography-x509/src/ocsp_resp.rs | 10 ++- src/rust/src/x509/certificate.rs | 28 ++++--- src/rust/src/x509/common.rs | 31 +++---- src/rust/src/x509/crl.rs | 10 ++- src/rust/src/x509/csr.rs | 11 ++- src/rust/src/x509/ocsp_req.rs | 6 +- src/rust/src/x509/ocsp_resp.rs | 29 ++++--- tests/x509/test_x509.py | 14 ++++ 13 files changed, 193 insertions(+), 60 deletions(-) diff --git a/src/rust/cryptography-x509/src/certificate.rs b/src/rust/cryptography-x509/src/certificate.rs index bb9a666f5f78a..59960242b2025 100644 --- a/src/rust/cryptography-x509/src/certificate.rs +++ b/src/rust/cryptography-x509/src/certificate.rs @@ -4,6 +4,7 @@ use crate::common; use crate::extensions; +use crate::extensions::Extensions; use crate::name; #[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, PartialEq, Clone)] @@ -31,7 +32,13 @@ pub struct TbsCertificate<'a> { #[implicit(2)] pub subject_unique_id: Option>, #[explicit(3)] - pub extensions: Option>, + pub raw_extensions: Option>, +} + +impl<'a> TbsCertificate<'a> { + pub fn extensions(&'a self) -> Result>, asn1::ObjectIdentifier> { + Extensions::from_raw_extensions(self.raw_extensions.as_ref()) + } } #[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, PartialEq, Clone)] diff --git a/src/rust/cryptography-x509/src/crl.rs b/src/rust/cryptography-x509/src/crl.rs index 3a47e0a377274..c81a3c4a95fda 100644 --- a/src/rust/cryptography-x509/src/crl.rs +++ b/src/rust/cryptography-x509/src/crl.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{common, extensions, name}; +use crate::{ + common, + extensions::{self}, + name, +}; pub type ReasonFlags<'a> = Option, asn1::OwnedBitString>>; @@ -31,14 +35,14 @@ pub struct TBSCertList<'a> { pub next_update: Option, pub revoked_certificates: RevokedCertificates<'a>, #[explicit(0)] - pub crl_extensions: Option>, + pub raw_crl_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Hash, Clone)] pub struct RevokedCertificate<'a> { pub user_certificate: asn1::BigUint<'a>, pub revocation_date: common::Time, - pub crl_entry_extensions: Option>, + pub raw_crl_entry_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/cryptography-x509/src/csr.rs b/src/rust/cryptography-x509/src/csr.rs index c23d22d0fd11d..d2cf9b5e27399 100644 --- a/src/rust/cryptography-x509/src/csr.rs +++ b/src/rust/cryptography-x509/src/csr.rs @@ -26,7 +26,7 @@ pub struct CertificationRequestInfo<'a> { impl CertificationRequestInfo<'_> { pub fn get_extension_attribute( &self, - ) -> Result>, asn1::ParseError> { + ) -> Result>, asn1::ParseError> { for attribute in self.attributes.unwrap_read().clone() { if attribute.type_id == oid::EXTENSION_REQUEST || attribute.type_id == oid::MS_EXTENSION_REQUEST diff --git a/src/rust/cryptography-x509/src/extensions.rs b/src/rust/cryptography-x509/src/extensions.rs index 0728633d4adbe..b1138fec206e7 100644 --- a/src/rust/cryptography-x509/src/extensions.rs +++ b/src/rust/cryptography-x509/src/extensions.rs @@ -2,16 +2,62 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. +use std::collections::HashSet; + use crate::common; use crate::crl; use crate::name; -pub type Extensions<'a> = common::Asn1ReadableOrWritable< +pub type RawExtensions<'a> = common::Asn1ReadableOrWritable< 'a, asn1::SequenceOf<'a, Extension<'a>>, asn1::SequenceOfWriter<'a, Extension<'a>, Vec>>, >; +/// An invariant-enforcing wrapper for `RawExtensions`. +/// +/// In particular, an `Extensions` cannot be constructed from a `RawExtensions` +/// that contains duplicated extensions (by OID). +pub struct Extensions<'a>(RawExtensions<'a>); + +impl<'a> Extensions<'a> { + /// Create an `Extensions` from the given `RawExtensions`. + /// + /// Returns an `Err` variant containing the first duplicated extension's + /// OID, if there are any duplicates. + pub fn from_raw_extensions( + raw: Option<&RawExtensions<'a>>, + ) -> Result, asn1::ObjectIdentifier> { + match raw { + Some(raw_exts) => { + let mut seen_oids = HashSet::new(); + + for ext in raw_exts.unwrap_read().clone() { + if !seen_oids.insert(ext.extn_id.clone()) { + return Err(ext.extn_id); + } + } + + Ok(Some(Self(raw_exts.clone()))) + } + None => Ok(None), + } + } + + /// Retrieves the extension identified by the given OID, + /// or None if the extension is not present (or no extensions are present). + pub fn get_extension(&self, oid: &asn1::ObjectIdentifier) -> Option { + let mut extensions = self.0.unwrap_read().clone(); + + extensions.find(|ext| &ext.extn_id == oid) + } + + /// Returns a reference to the underlying extensions. + pub fn as_raw(&self) -> &RawExtensions<'_> { + &self.0 + } +} + #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone)] pub struct Extension<'a> { pub extn_id: asn1::ObjectIdentifier, @@ -174,3 +220,38 @@ pub struct BasicConstraints { pub ca: bool, pub path_length: Option, } + +#[cfg(test)] +mod tests { + use asn1::SequenceOfWriter; + + use crate::oid::{AUTHORITY_KEY_IDENTIFIER_OID, BASIC_CONSTRAINTS_OID}; + + use super::{BasicConstraints, Extension, Extensions}; + + #[test] + fn test_get_extension() { + let extension_value = BasicConstraints { + ca: true, + path_length: Some(3), + }; + let extension = Extension { + extn_id: BASIC_CONSTRAINTS_OID, + critical: true, + extn_value: &asn1::write_single(&extension_value).unwrap(), + }; + let extensions = SequenceOfWriter::new(vec![extension]); + + let der = asn1::write_single(&extensions).unwrap(); + + let extensions: Extensions = + Extensions::from_raw_extensions(Some(&asn1::parse_single(&der).unwrap())) + .unwrap() + .unwrap(); + + assert!(&extensions.get_extension(&BASIC_CONSTRAINTS_OID).is_some()); + assert!(&extensions + .get_extension(&AUTHORITY_KEY_IDENTIFIER_OID) + .is_none()); + } +} diff --git a/src/rust/cryptography-x509/src/ocsp_req.rs b/src/rust/cryptography-x509/src/ocsp_req.rs index 1e096e71f1dab..ba54d391f5065 100644 --- a/src/rust/cryptography-x509/src/ocsp_req.rs +++ b/src/rust/cryptography-x509/src/ocsp_req.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{common, extensions, name}; +use crate::{ + common, + extensions::{self}, + name, +}; #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct TBSRequest<'a> { @@ -17,14 +21,14 @@ pub struct TBSRequest<'a> { asn1::SequenceOfWriter<'a, Request<'a>>, >, #[explicit(2)] - pub request_extensions: Option>, + pub raw_request_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct Request<'a> { pub req_cert: CertID<'a>, #[explicit(0)] - pub single_request_extensions: Option>, + pub single_request_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/cryptography-x509/src/ocsp_resp.rs b/src/rust/cryptography-x509/src/ocsp_resp.rs index f7620f6aa6016..21f01e2c7375f 100644 --- a/src/rust/cryptography-x509/src/ocsp_resp.rs +++ b/src/rust/cryptography-x509/src/ocsp_resp.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::{certificate, common, crl, extensions, name, ocsp_req}; +use crate::{ + certificate, common, crl, + extensions::{self}, + name, ocsp_req, +}; #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct OCSPResponse<'a> { @@ -47,7 +51,7 @@ pub struct ResponseData<'a> { asn1::SequenceOfWriter<'a, SingleResponse<'a>, Vec>>, >, #[explicit(1)] - pub response_extensions: Option>, + pub raw_response_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] @@ -66,7 +70,7 @@ pub struct SingleResponse<'a> { #[explicit(0)] pub next_update: Option, #[explicit(1)] - pub single_extensions: Option>, + pub raw_single_extensions: Option>, } #[derive(asn1::Asn1Read, asn1::Asn1Write)] diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs index 03d8ae883256e..3784b1c9a4b07 100644 --- a/src/rust/src/x509/certificate.rs +++ b/src/rust/src/x509/certificate.rs @@ -9,13 +9,13 @@ use crate::error::{CryptographyError, CryptographyResult}; use crate::x509::{extensions, sct, sign}; use crate::{exceptions, x509}; use cryptography_x509::common::Asn1ReadableOrWritable; +use cryptography_x509::extensions::Extension; use cryptography_x509::extensions::{ AuthorityKeyIdentifier, BasicConstraints, DisplayText, DistributionPoint, DistributionPointName, MSCertificateTemplate, NameConstraints, PolicyConstraints, - PolicyInformation, PolicyQualifierInfo, Qualifier, SequenceOfAccessDescriptions, + PolicyInformation, PolicyQualifierInfo, Qualifier, RawExtensions, SequenceOfAccessDescriptions, SequenceOfSubtrees, UserNotice, }; -use cryptography_x509::extensions::{Extension, Extensions}; use cryptography_x509::{common, name, oid}; use once_cell::sync::Lazy; use pyo3::{IntoPy, ToPyObject}; @@ -193,9 +193,9 @@ impl Certificate { let val = self.raw.borrow_value(); let mut tbs_precert = val.tbs_cert.clone(); // Remove the SCT list extension - match tbs_precert.extensions { - Some(extensions) => { - let readable_extensions = extensions.unwrap_read().clone(); + match val.tbs_cert.extensions() { + Ok(Some(extensions)) => { + let readable_extensions = extensions.as_raw().unwrap_read().clone(); let ext_count = readable_extensions.len(); let filtered_extensions: Vec> = readable_extensions .filter(|x| x.extn_id != oid::PRECERT_SIGNED_CERTIFICATE_TIMESTAMPS_OID) @@ -207,18 +207,26 @@ impl Certificate { ), )); } - let filtered_extensions: Extensions<'_> = Asn1ReadableOrWritable::new_write( + let filtered_extensions: RawExtensions<'_> = Asn1ReadableOrWritable::new_write( asn1::SequenceOfWriter::new(filtered_extensions), ); - tbs_precert.extensions = Some(filtered_extensions); + tbs_precert.raw_extensions = Some(filtered_extensions); let result = asn1::write_single(&tbs_precert)?; Ok(pyo3::types::PyBytes::new(py, &result)) } - None => Err(CryptographyError::from( + Ok(None) => Err(CryptographyError::from( pyo3::exceptions::PyValueError::new_err( "Could not find any extensions in TBS certificate", ), )), + Err(oid) => { + let oid_obj = oid_to_py_oid(py, &oid)?; + Err(exceptions::DuplicateExtension::new_err(( + format!("Duplicate {} extension found", oid), + oid_obj.into_py(py), + )) + .into()) + } } } @@ -360,7 +368,7 @@ impl Certificate { x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.raw.borrow_value().tbs_cert.extensions, + &self.raw.borrow_value().tbs_cert.raw_extensions, |oid, ext_data| match *oid { oid::PRECERT_POISON_OID => { asn1::parse_single::<()>(ext_data)?; @@ -1035,7 +1043,7 @@ fn create_x509_certificate( spki: asn1::parse_single(spki_bytes)?, issuer_unique_id: None, subject_unique_id: None, - extensions: x509::common::encode_extensions( + raw_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/src/rust/src/x509/common.rs b/src/rust/src/x509/common.rs index 571963e36b636..94ae58d386c5c 100644 --- a/src/rust/src/x509/common.rs +++ b/src/rust/src/x509/common.rs @@ -6,11 +6,10 @@ use crate::asn1::{oid_to_py_oid, py_oid_to_oid}; use crate::error::{CryptographyError, CryptographyResult}; use crate::{exceptions, x509}; use cryptography_x509::common::{Asn1ReadableOrWritable, AttributeTypeValue, RawTlv}; -use cryptography_x509::extensions::{AccessDescription, Extension, Extensions}; +use cryptography_x509::extensions::{AccessDescription, Extension, Extensions, RawExtensions}; use cryptography_x509::name::{GeneralName, Name, OtherName, UnvalidatedIA5String}; use pyo3::types::IntoPyDict; use pyo3::{IntoPy, ToPyObject}; -use std::collections::HashSet; /// Parse all sections in a PEM file and return the first matching section. /// If no matching sections are found, return an error. @@ -391,27 +390,30 @@ pub(crate) fn parse_and_cache_extensions< >( py: pyo3::Python<'p>, cached_extensions: &mut Option, - raw_exts: &Option>, + raw_extensions: &Option>, parse_ext: F, ) -> pyo3::PyResult { if let Some(cached) = cached_extensions { return Ok(cached.clone_ref(py)); } + let extensions = match Extensions::from_raw_extensions(raw_extensions.as_ref()) { + Ok(extensions) => extensions, + Err(oid) => { + let oid_obj = oid_to_py_oid(py, &oid)?; + return Err(exceptions::DuplicateExtension::new_err(( + format!("Duplicate {} extension found", oid), + oid_obj.into_py(py), + ))); + } + }; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; let exts = pyo3::types::PyList::empty(py); - let mut seen_oids = HashSet::new(); - if let Some(raw_exts) = raw_exts { - for raw_ext in raw_exts.unwrap_read().clone() { + if let Some(extensions) = extensions { + for raw_ext in extensions.as_raw().unwrap_read().clone() { let oid_obj = oid_to_py_oid(py, &raw_ext.extn_id)?; - if seen_oids.contains(&raw_ext.extn_id) { - return Err(exceptions::DuplicateExtension::new_err(( - format!("Duplicate {} extension found", raw_ext.extn_id), - oid_obj.into_py(py), - ))); - } - let extn_value = match parse_ext(&raw_ext.extn_id, raw_ext.extn_value)? { Some(e) => e, None => x509_module.call_method1( @@ -424,7 +426,6 @@ pub(crate) fn parse_and_cache_extensions< (oid_obj, raw_ext.critical, extn_value), )?; exts.append(ext_obj)?; - seen_oids.insert(raw_ext.extn_id); } } let extensions = x509_module @@ -445,7 +446,7 @@ pub(crate) fn encode_extensions< py: pyo3::Python<'p>, py_exts: &'p pyo3::PyAny, encode_ext: F, -) -> pyo3::PyResult>> { +) -> pyo3::PyResult>> { let unrecognized_extension_type: &pyo3::types::PyType = py .import(pyo3::intern!(py, "cryptography.x509"))? .getattr(pyo3::intern!(py, "UnrecognizedExtension"))? diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index e2c4b9c09b9e3..6bb08779a0a20 100644 --- a/src/rust/src/x509/crl.rs +++ b/src/rust/src/x509/crl.rs @@ -260,11 +260,13 @@ impl CertificateRevocationList { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { + let tbs_cert_list = &self.owned.borrow_value().tbs_cert_list; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.owned.borrow_value().tbs_cert_list.crl_extensions, + &tbs_cert_list.raw_crl_extensions, |oid, ext_data| match *oid { oid::CRL_NUMBER_OID => { let bignum = asn1::parse_single::>(ext_data)?; @@ -498,7 +500,7 @@ impl RevokedCertificate { x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.owned.borrow_value().crl_entry_extensions, + &self.owned.borrow_value().raw_crl_entry_extensions, |oid, ext_data| parse_crl_entry_ext(py, oid.clone(), ext_data), ) } @@ -594,7 +596,7 @@ fn create_x509_crl( user_certificate: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, serial_number)?) .unwrap(), revocation_date: x509::certificate::time_from_py(py, py_revocation_date)?, - crl_entry_extensions: x509::common::encode_extensions( + raw_crl_entry_extensions: x509::common::encode_extensions( py, py_revoked_cert.getattr(pyo3::intern!(py, "extensions"))?, extensions::encode_extension, @@ -618,7 +620,7 @@ fn create_x509_crl( asn1::SequenceOfWriter::new(revoked_certs), )) }, - crl_extensions: x509::common::encode_extensions( + raw_crl_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/src/rust/src/x509/csr.rs b/src/rust/src/x509/csr.rs index 35aee5c9e5013..7ceed3511daad 100644 --- a/src/rust/src/x509/csr.rs +++ b/src/rust/src/x509/csr.rs @@ -211,7 +211,7 @@ impl CertificateSigningRequest { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { - let exts = self + let raw_exts = self .raw .borrow_value() .csr_info @@ -222,9 +222,12 @@ impl CertificateSigningRequest { ) })?; - x509::parse_and_cache_extensions(py, &mut self.cached_extensions, &exts, |oid, ext_data| { - certificate::parse_cert_ext(py, oid.clone(), ext_data) - }) + x509::parse_and_cache_extensions( + py, + &mut self.cached_extensions, + &raw_exts, + |oid, ext_data| certificate::parse_cert_ext(py, oid.clone(), ext_data), + ) } #[getter] diff --git a/src/rust/src/x509/ocsp_req.rs b/src/rust/src/x509/ocsp_req.rs index 235ac6ee10c5c..bd5aecad0ec74 100644 --- a/src/rust/src/x509/ocsp_req.rs +++ b/src/rust/src/x509/ocsp_req.rs @@ -108,11 +108,13 @@ impl OCSPRequest { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { + let tbs_request = &self.raw.borrow_value().tbs_request; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self.raw.borrow_value().tbs_request.request_extensions, + &tbs_request.raw_request_extensions, |oid, value| { match *oid { oid::NONCE_OID => { @@ -228,7 +230,7 @@ fn create_ocsp_request( request_list: common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new( &reqs, )), - request_extensions: extensions, + raw_request_extensions: extensions, }, optional_signature: None, }; diff --git a/src/rust/src/x509/ocsp_resp.rs b/src/rust/src/x509/ocsp_resp.rs index 942822b48168f..728eb92cef381 100644 --- a/src/rust/src/x509/ocsp_resp.rs +++ b/src/rust/src/x509/ocsp_resp.rs @@ -316,20 +316,22 @@ impl OCSPResponse { #[getter] fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult { self.requires_successful_response()?; + + let response_data = &self + .raw + .borrow_value() + .response_bytes + .as_ref() + .unwrap() + .response + .get() + .tbs_response_data; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_extensions, - &self - .raw - .borrow_value() - .response_bytes - .as_ref() - .unwrap() - .response - .get() - .tbs_response_data - .response_extensions, + &response_data.raw_response_extensions, |oid, ext_data| { match oid { &oid::NONCE_OID => { @@ -362,11 +364,12 @@ impl OCSPResponse { .response .get(), )?; + let x509_module = py.import(pyo3::intern!(py, "cryptography.x509"))?; x509::parse_and_cache_extensions( py, &mut self.cached_single_extensions, - &single_resp.single_extensions, + &single_resp.raw_single_extensions, |oid, ext_data| match oid { &oid::SIGNED_CERTIFICATE_TIMESTAMPS_OID => { let contents = asn1::parse_single::<&[u8]>(ext_data)?; @@ -628,7 +631,7 @@ fn create_ocsp_response( cert_status, next_update, this_update, - single_extensions: None, + raw_single_extensions: None, }]; borrowed_cert = responder_cert.borrow(); @@ -669,7 +672,7 @@ fn create_ocsp_response( responses: common::Asn1ReadableOrWritable::new_write(asn1::SequenceOfWriter::new( responses, )), - response_extensions: x509::common::encode_extensions( + raw_response_extensions: x509::common::encode_extensions( py, builder.getattr(pyo3::intern!(py, "_extensions"))?, extensions::encode_extension, diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py index a32dfca930cf7..b33e09ce518fb 100644 --- a/tests/x509/test_x509.py +++ b/tests/x509/test_x509.py @@ -1000,6 +1000,20 @@ def test_tbs_certificate_bytes(self, backend): cert.signature_hash_algorithm, ) + def test_tbs_precertificate_bytes_duplicate_extensions_raises( + self, backend + ): + cert = _load_cert( + os.path.join("x509", "custom", "two_basic_constraints.pem"), + x509.load_pem_x509_certificate, + ) + + with pytest.raises( + x509.DuplicateExtension, + match="Duplicate 2.5.29.19 extension found", + ): + cert.tbs_precertificate_bytes + def test_tbs_precertificate_bytes_no_extensions_raises(self, backend): cert = _load_cert( os.path.join("x509", "v1_cert.pem"),