RustCrypto / formats

Cryptography-related format encoders/decoders: DER, PEM, PKCS, PKIX
228 stars 122 forks source link

Mising error in implementation #1384

Closed Firstyear closed 2 months ago

Firstyear commented 2 months ago

Attempting to use der from git main with derive throws this error:

error[E0046]: not all trait items implemented, missing: `Error`
   --> src/kdc.rs:9:39
    |
9   | #[derive(Clone, Debug, Eq, PartialEq, Sequence)]
    |                                       ^^^^^^^^
    |                                       |
    |                                       missing `Error` in implementation
    |                                       in this derive macro expansion
    |
   ::: /Users/william/.cargo/registry/src/index.crates.io-6f17d22bba15001f/der_derive-0.8.0-pre.0/src/lib.rs:274:1
    |
274 | pub fn derive_sequence(input: TokenStream) -> TokenStream {
    | --------------------------------------------------------- in this expansion of `#[derive(Sequence)]`
    |
    = help: implement the missing item: `type Error = /* Type */;`

I bisected this to:

commit c5018377fdd33bae78ba78166ec54f001ddc91bd
Author: turbocool3r <60693357+turbocool3r@users.noreply.github.com>
Date:   Sun Mar 3 00:28:07 2024 +0400

    Add custom error types support to the Decode and DecodeValue traits. (#1055)

Which of course makes sense that this was the cause. The problem is I'm not 100% sure how to actually fix this. Any ideas?

tarcieri commented 2 months ago

This does look like a regression related to that, although I'm curious why it isn't manifesting in the other uses of custom derive in this repo

cc @turbocool3r

turbocool3r commented 2 months ago

Interesting, I'll look into that tomorrow.

Firstyear commented 2 months ago

I'm happy to help in anyway too :)

turbocool3r commented 2 months ago

@Firstyear it would be great if you could provide the code or a small crate that reproduces the issue. Is it possible that your crate is built from multiple versions of der?

Firstyear commented 2 months ago

Single version of der, referencing git master.

use tracing::*;

use der::{asn1::{Any, Ia5String, OctetString, ContextSpecific, KerbString}, Encode, Decode, Sequence};

pub type KerbRealm = KerbString;

#[derive(Clone, Debug, Eq, PartialEq, Sequence)]
pub struct KdcProxyMessage {
    #[asn1(context_specific = "0")]
    pub kerb_message: OctetString,
    #[asn1(optional = "true", context_specific = "1")]
    pub target_domain: Option<KerbRealm>,
    #[asn1(optional = "true", context_specific = "2")]
    pub dclocator_hint: Option<i32>,
}

#[cfg(test)]
mod test {
    use super::*;
    use base64::{engine::general_purpose::URL_SAFE, Engine as _};

    #[test]
    fn basic_kkdcp_parse() {
        let _ = tracing_subscriber::fmt::try_init();
        let sample = URL_SAFE.decode(b"MIHMoIG8BIG5AAAAtWqBsjCBr6EDAgEFogMCAQqjGjAYMAqhBAICAJaiAgQAMAqhBAICAJWiAgQApIGGMIGDoAcDBQAAAAAQoRQwEqADAgEBoQswCRsHd2lsbGlhbaILGwlLS0RDUC5ERVajHjAcoAMCAQKhFTATGwZrcmJ0Z3QbCUtLRENQLkRFVqURGA8yMDI0MDQxNzA0MTU0OVqnBgIEf72nrqgaMBgCARICARECARQCARMCARACARcCARkCARqhCxsJS0tEQ1AuREVW")
            .expect("Unable to decode sample");

        tracing::warn!(?sample);

        let kdc_proxy_message = KdcProxyMessage::from_der(&sample)
            .expect("Failed to decode Kdc Proxy Message");

        tracing::warn!(?kdc_proxy_message);
    }
}
[package]
name = "kkdcp-experiment"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
axum = { version = "0.7", features = ["macros"] }
axum-server = { version = "0.6", features = ["tls-openssl"] }

clap = { version = "4.1", features = ["derive", "env"] }
tokio = { version = "1", features = ["macros", "rt"] }

tracing = { version = "0.1" }
tracing-subscriber = { version = "0.3" }
base64 = "0.22.0"

# der = { version = "0.7.9", features = ["alloc", "derive"] }
der = { path = "./formats/der", features = ["alloc", "derive"] }
turbocool3r commented 2 months ago

Hmm, and what is the version of der_derive?

turbocool3r commented 2 months ago

Seems like der_derive is being pulled from the crates.io registry ignoring the patch.crates-io section in Cargo.toml in the workspace. I tried duplicating the patch.crates-io section in Cargo.toml of der and the crate itself and only the latter works. Perhaps proc macros are treated differently than regular dependencies. I think the best way would be to make a prerelease of the derive crate. cc @tarcieri

tarcieri commented 2 months ago

Yeah, perhaps @Firstyear needs to add a similar [patch.crates-io] section to Cargo.toml above which overrides der_derive to be sourced from path = "./formats/der/derive

Firstyear commented 2 months ago

Damn you're probably right, I think this was my fault. Thank you both so much I feel very silly for wasting your time.