From a4ad7d0a40bf98655bec0069119d473b9aad760b Mon Sep 17 00:00:00 2001 From: Arthur Gautier Date: Tue, 9 Jul 2024 20:33:05 +0000 Subject: [PATCH] der-derive: v0.7.3 (#1443) * der-derive: avoid type inference when using default (#1442) * Revert "x509-cert: specify concrete types to help the compiler (#1441)" This reverts commit 7a2d38a1c72b19d843face98db31e8083a46ac79. * der-derive: avoid type inference when using default This is another take on #1441. This is intended to make sure we can still continue to use `Default::default` and not break future releases by mistake. * der-derive: v0.7.3 Changed (2024-07-09) - avoid type inference when using default (#1443) * rust 1.78 fixups This fixes all `unnecessary qualification` warnings * x509-cert: ignore zlint error, fixed in main * x509-cert: fuzz: ignore unused_qualification errors --- .github/workflows/x509-cert.yml | 3 ++ Cargo.lock | 2 +- base16ct/src/error.rs | 6 ++-- base16ct/src/upper.rs | 2 +- der/derive/CHANGELOG.md | 6 ++++ der/derive/Cargo.toml | 2 +- der/derive/src/sequence/field.rs | 15 +++++---- der/tests/derive.rs | 56 ++++++++++++++++++++++++++++++++ pkcs1/src/error.rs | 12 +++---- pkcs1/src/traits.rs | 2 +- pkcs5/src/pbes1.rs | 6 ++-- pkcs5/src/pbes2.rs | 16 +++------ sec1/src/private_key.rs | 2 +- x509-cert/src/request.rs | 2 +- x509-cert/tests/builder.rs | 4 +++ 15 files changed, 101 insertions(+), 35 deletions(-) diff --git a/.github/workflows/x509-cert.yml b/.github/workflows/x509-cert.yml index 0c52777c7..62e74eb6f 100644 --- a/.github/workflows/x509-cert.yml +++ b/.github/workflows/x509-cert.yml @@ -66,6 +66,9 @@ jobs: fuzz: runs-on: ubuntu-latest + env: + # ignore unused_qualification errors + RUSTFLAGS: "" steps: - uses: actions/checkout@v3 - uses: dtolnay/rust-toolchain@nightly diff --git a/Cargo.lock b/Cargo.lock index 489298c44..973e5b5de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -439,7 +439,7 @@ dependencies = [ [[package]] name = "der_derive" -version = "0.7.2" +version = "0.7.3" dependencies = [ "proc-macro2", "quote", diff --git a/base16ct/src/error.rs b/base16ct/src/error.rs index a5f99c2ea..29cbabf5e 100644 --- a/base16ct/src/error.rs +++ b/base16ct/src/error.rs @@ -25,8 +25,8 @@ impl fmt::Display for Error { #[cfg(feature = "std")] impl std::error::Error for Error {} -impl From for core::fmt::Error { - fn from(_: Error) -> core::fmt::Error { - core::fmt::Error::default() +impl From for fmt::Error { + fn from(_: Error) -> fmt::Error { + fmt::Error::default() } } diff --git a/base16ct/src/upper.rs b/base16ct/src/upper.rs index a747f297c..2b82ce68a 100644 --- a/base16ct/src/upper.rs +++ b/base16ct/src/upper.rs @@ -46,7 +46,7 @@ pub fn encode_string(input: &[u8]) -> String { let res = encode(input, &mut dst).expect("dst length is correct"); debug_assert_eq!(elen, res.len()); - unsafe { crate::String::from_utf8_unchecked(dst) } + unsafe { String::from_utf8_unchecked(dst) } } /// Decode a single nibble of upper hex diff --git a/der/derive/CHANGELOG.md b/der/derive/CHANGELOG.md index 194f7af6c..a9dc9cd0e 100644 --- a/der/derive/CHANGELOG.md +++ b/der/derive/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 0.7.3 (2024-07-09) +### Changed +- avoid type inference when using default ([#1443]) + +[#1443]: https://github.com/RustCrypto/formats/pull/1443 + ## 0.7.2 (2023-08-07) ### Changed - fix doc typo and use a valid tag number ([#1184]) diff --git a/der/derive/Cargo.toml b/der/derive/Cargo.toml index cced55dcb..7b1b68114 100644 --- a/der/derive/Cargo.toml +++ b/der/derive/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "der_derive" -version = "0.7.2" +version = "0.7.3" description = "Custom derive support for the `der` crate's `Choice` and `Sequence` traits" authors = ["RustCrypto Developers"] license = "Apache-2.0 OR MIT" diff --git a/der/derive/src/sequence/field.rs b/der/derive/src/sequence/field.rs index 3fb183208..d91599c0a 100644 --- a/der/derive/src/sequence/field.rs +++ b/der/derive/src/sequence/field.rs @@ -97,7 +97,7 @@ impl SequenceField { !attrs.optional, "`default`, and `optional` are mutually exclusive" ); - lowerer.apply_default(&self.ident, default); + lowerer.apply_default(&self.ident, default, &self.field_type); } lowerer.into_tokens() @@ -189,14 +189,17 @@ impl LowerFieldEncoder { } /// Handle default value for a type. - fn apply_default(&mut self, ident: &Ident, default: &Path) { + fn apply_default(&mut self, ident: &Ident, default: &Path, field_type: &Type) { let encoder = &self.encoder; self.encoder = quote! { - if &self.#ident == &#default() { - None - } else { - Some(#encoder) + { + let default_value: #field_type = #default(); + if &self.#ident == &default_value { + None + } else { + Some(#encoder) + } } }; } diff --git a/der/tests/derive.rs b/der/tests/derive.rs index a8c77febc..35d38e381 100644 --- a/der/tests/derive.rs +++ b/der/tests/derive.rs @@ -459,3 +459,59 @@ mod sequence { ); } } + +mod infer_default { + //! When another crate might define a PartialEq for another type, the use of + //! `default="Default::default"` in the der derivation will not provide enough + //! information for `der_derive` crate to figure out. + //! + //! This provides a reproduction for that case. This is intended to fail when we + //! compile tests. + //! ``` + //! error[E0282]: type annotations needed + //! --> der/tests/derive.rs:480:26 + //! | + //!480 | #[asn1(default = "Default::default")] + //! | ^^^^^^^^^^^^^^^^^^ cannot infer type + //! + //!error[E0283]: type annotations needed + //! --> der/tests/derive.rs:478:14 + //! | + //!478 | #[derive(Sequence)] + //! | ^^^^^^^^ cannot infer type + //! | + //!note: multiple `impl`s satisfying `bool: PartialEq<_>` found + //! --> der/tests/derive.rs:472:5 + //! | + //!472 | impl PartialEq for bool { + //! | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + //! = note: and another `impl` found in the `core` crate: + //! - impl PartialEq for bool + //! where the constant `host` has type `bool`; + //! = note: required for `&bool` to implement `PartialEq<&_>` + //! = note: this error originates in the derive macro `Sequence` (in Nightly builds, run with -Z macro-backtrace for more info) + //! ``` + + use der::Sequence; + + struct BooleanIsh; + + impl PartialEq for bool { + fn eq(&self, _other: &BooleanIsh) -> bool { + unimplemented!("This is only here to mess up the compiler's type inference") + } + } + + #[derive(Sequence)] + struct Foo { + #[asn1(default = "Default::default")] + pub use_default_default: bool, + + #[asn1(default = "something_true")] + pub use_custom: bool, + } + + fn something_true() -> bool { + todo!() + } +} diff --git a/pkcs1/src/error.rs b/pkcs1/src/error.rs index 135bcd7d9..ac8f4feaa 100644 --- a/pkcs1/src/error.rs +++ b/pkcs1/src/error.rs @@ -75,18 +75,18 @@ impl From for Error { } #[cfg(feature = "pkcs8")] -impl From for pkcs8::spki::Error { - fn from(err: Error) -> pkcs8::spki::Error { +impl From for spki::Error { + fn from(err: Error) -> spki::Error { match err { - Error::Asn1(e) => pkcs8::spki::Error::Asn1(e), - _ => pkcs8::spki::Error::KeyMalformed, + Error::Asn1(e) => spki::Error::Asn1(e), + _ => spki::Error::KeyMalformed, } } } #[cfg(feature = "pkcs8")] -impl From for Error { - fn from(err: pkcs8::spki::Error) -> Error { +impl From for Error { + fn from(err: spki::Error) -> Error { Error::Pkcs8(pkcs8::Error::PublicKey(err)) } } diff --git a/pkcs1/src/traits.rs b/pkcs1/src/traits.rs index cd3d04e21..fe424cdce 100644 --- a/pkcs1/src/traits.rs +++ b/pkcs1/src/traits.rs @@ -171,7 +171,7 @@ where #[cfg(feature = "pkcs8")] impl DecodeRsaPublicKey for T where - T: for<'a> TryFrom, Error = pkcs8::spki::Error>, + T: for<'a> TryFrom, Error = spki::Error>, { fn from_pkcs1_der(public_key: &[u8]) -> Result { Ok(Self::try_from(pkcs8::SubjectPublicKeyInfoRef { diff --git a/pkcs5/src/pbes1.rs b/pkcs5/src/pbes1.rs index 217154795..946c3032a 100644 --- a/pkcs5/src/pbes1.rs +++ b/pkcs5/src/pbes1.rs @@ -91,8 +91,8 @@ impl<'a> TryFrom> for Algorithm { fn try_from(alg: AlgorithmIdentifierRef<'a>) -> der::Result { // Ensure that we have a supported PBES1 algorithm identifier - let encryption = EncryptionScheme::try_from(alg.oid) - .map_err(|_| der::Tag::ObjectIdentifier.value_error())?; + let encryption = + EncryptionScheme::try_from(alg.oid).map_err(|_| Tag::ObjectIdentifier.value_error())?; let parameters = alg .parameters @@ -153,7 +153,7 @@ impl TryFrom> for Parameters { salt: OctetStringRef::decode(reader)? .as_bytes() .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + .map_err(|_| Tag::OctetString.value_error())?, iteration_count: reader.decode()?, }) }) diff --git a/pkcs5/src/pbes2.rs b/pkcs5/src/pbes2.rs index 301105cc2..7b51ab09a 100644 --- a/pkcs5/src/pbes2.rs +++ b/pkcs5/src/pbes2.rs @@ -318,31 +318,25 @@ impl<'a> TryFrom> for EncryptionScheme<'a> { match alg.oid { AES_128_CBC_OID => Ok(Self::Aes128Cbc { - iv: iv - .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + iv: iv.try_into().map_err(|_| Tag::OctetString.value_error())?, }), AES_192_CBC_OID => Ok(Self::Aes192Cbc { - iv: iv - .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + iv: iv.try_into().map_err(|_| Tag::OctetString.value_error())?, }), AES_256_CBC_OID => Ok(Self::Aes256Cbc { - iv: iv - .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + iv: iv.try_into().map_err(|_| Tag::OctetString.value_error())?, }), #[cfg(feature = "des-insecure")] DES_CBC_OID => Ok(Self::DesCbc { iv: iv[0..DES_BLOCK_SIZE] .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + .map_err(|_| Tag::OctetString.value_error())?, }), #[cfg(feature = "3des")] DES_EDE3_CBC_OID => Ok(Self::DesEde3Cbc { iv: iv[0..DES_BLOCK_SIZE] .try_into() - .map_err(|_| der::Tag::OctetString.value_error())?, + .map_err(|_| Tag::OctetString.value_error())?, }), oid => Err(ErrorKind::OidUnknown { oid }.into()), } diff --git a/sec1/src/private_key.rs b/sec1/src/private_key.rs index 531579936..ec851e14e 100644 --- a/sec1/src/private_key.rs +++ b/sec1/src/private_key.rs @@ -98,7 +98,7 @@ impl<'a> DecodeValue<'a> for EcPrivateKey<'a> { fn decode_value>(reader: &mut R, header: Header) -> der::Result { reader.read_nested(header.length, |reader| { if u8::decode(reader)? != VERSION { - return Err(der::Tag::Integer.value_error()); + return Err(Tag::Integer.value_error()); } let private_key = OctetStringRef::decode(reader)?.as_bytes(); diff --git a/x509-cert/src/request.rs b/x509-cert/src/request.rs index 50478d8bc..4e15b0416 100644 --- a/x509-cert/src/request.rs +++ b/x509-cert/src/request.rs @@ -142,7 +142,7 @@ pub mod attributes { pub trait AsAttribute: AssociatedOid + Tagged + EncodeValue + Sized { /// Returns the Attribute with the content encoded. fn to_attribute(&self) -> Result { - let inner: Any = der::asn1::Any::encode_from(self)?; + let inner: Any = Any::encode_from(self)?; let values = SetOfVec::try_from(vec![inner])?; diff --git a/x509-cert/tests/builder.rs b/x509-cert/tests/builder.rs index 06aae51e7..3d45b6220 100644 --- a/x509-cert/tests/builder.rs +++ b/x509-cert/tests/builder.rs @@ -165,6 +165,8 @@ fn leaf_certificate() { "e_subject_common_name_not_exactly_from_san", // Extended key usage needs to be added by end-user and is use-case dependent "e_sub_cert_eku_missing", + // Zlint got updated, fixed in master + "w_subject_common_name_included", ]; zlint::check_certificate(pem.as_bytes(), &ignored); @@ -242,6 +244,8 @@ fn pss_certificate() { "e_sub_cert_eku_missing", // zlint warns on RSAPSS signature algorithms "e_signature_algorithm_not_supported", + // Zlint got updated, fixed in master + "w_subject_common_name_included", ]; zlint::check_certificate(pem.as_bytes(), ignored);