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 7a2d38a. * 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
RustCrypto · Jul 9, 2024 · a4ad7d0 · a4ad7d0
1 parent 9bf8809
commit a4ad7d0
Show file tree

Hide file tree

Showing 15 changed files with 101 additions and 35 deletions.
diff --git 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
diff --git 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<Error> for core::fmt::Error {
-    fn from(_: Error) -> core::fmt::Error {
-        core::fmt::Error::default()
+impl From<Error> for fmt::Error {
+    fn from(_: Error) -> fmt::Error {
+        fmt::Error::default()
     }
 }
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<BooleanIsh> for bool {
+    //!    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    //!    = note: and another `impl` found in the `core` crate:
+    //!            - impl<host> 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<BooleanIsh> 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
@@ -75,18 +75,18 @@ impl From<pkcs8::Error> for Error {
 }
 
 #[cfg(feature = "pkcs8")]
-impl From<Error> for pkcs8::spki::Error {
-    fn from(err: Error) -> pkcs8::spki::Error {
+impl From<Error> 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<pkcs8::spki::Error> for Error {
-    fn from(err: pkcs8::spki::Error) -> Error {
+impl From<spki::Error> 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
@@ -171,7 +171,7 @@ where
 #[cfg(feature = "pkcs8")]
 impl<T> DecodeRsaPublicKey for T
 where
-    T: for<'a> TryFrom<pkcs8::SubjectPublicKeyInfoRef<'a>, Error = pkcs8::spki::Error>,
+    T: for<'a> TryFrom<pkcs8::SubjectPublicKeyInfoRef<'a>, Error = spki::Error>,
 {
     fn from_pkcs1_der(public_key: &[u8]) -> Result<Self> {
         Ok(Self::try_from(pkcs8::SubjectPublicKeyInfoRef {

diff --git a/pkcs5/src/pbes1.rs b/pkcs5/src/pbes1.rs
@@ -91,8 +91,8 @@ impl<'a> TryFrom<AlgorithmIdentifierRef<'a>> for Algorithm {
 
     fn try_from(alg: AlgorithmIdentifierRef<'a>) -> der::Result<Self> {
         // 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<AnyRef<'_>> 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
@@ -318,31 +318,25 @@ impl<'a> TryFrom<AlgorithmIdentifierRef<'a>> 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
@@ -98,7 +98,7 @@ impl<'a> DecodeValue<'a> for EcPrivateKey<'a> {
     fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> der::Result<Self> {
         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
@@ -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<Attribute> {
-            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
@@ -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);