rusticata / asn1-rs

Parsers/Encoders for ASN.1 BER/DER data
Apache License 2.0
9 stars 14 forks source link

Make Oid::from a const fn #20

Open str4d opened 4 years ago

str4d commented 4 years ago

Currently, if I want to compare parsed OIDs against expected constants, the best I can do with the current API is the following:

const OID_RSA_ENCRYPTION: &str = "1.2.840.113549.1.1.1";
const OID_EC_PUBLIC_KEY: &str = "1.2.840.10045.2.1";

fn foo(oid: Oid) {
    match oid.to_string().as_str() {
        OID_RSA_ENCRYPTION => { .. },
        OID_EC_PUBLIC_KEY => { .. },
    }
}

If Oid::from was a const fn, then we could define the constants using something like:

const OID_RSA_ENCRYPTION: Oid = Oid::from(&[1, 2, 840, 113549, 1, 1, 1]);
const OID_EC_PUBLIC_KEY: Oid = Oid::from(&[1, 2, 840, 10045, 2, 1]);

fn foo(oid: Oid) {
    match oid {
        OID_RSA_ENCRYPTION => { .. },
        OID_EC_PUBLIC_KEY => { .. },
    }
}

This probably requires rusticata/der-parser#17.

jannschu commented 4 years ago

This probably requires rusticata/der-parser#17.

I think you are right. Since Oid::from as currently implemented in master allocates memory, I don't think is can be const.

While rusticata/der-parser#17 would allow a const fn, it needs the DER encoded format. So instead of Oud::from(&[1,2]) you have to call it as Oid::from(der_encoded). That is why the oid! macro is in that pull request (#18).

The oid! macro can not be used in pattern positions directly, but by defining a constant as you did it should work. The code would be similar to

const OID_RSA_ENCRYPTION: Oid = oid!(1.2.840.113549.1.1.1);
const OID_EC_PUBLIC_KEY: Oid = oid!(1.2.840.10045.2.1);

fn foo(oid: Oid) {
    match oid {
        OID_RSA_ENCRYPTION => { .. },
        OID_EC_PUBLIC_KEY => { .. },
    }
}

See https://github.com/dtolnay/proc-macro-hack/issues/20 and https://github.com/rust-lang/rust/issues/54727 for the issues related to using the macro in pattern position directly. On nightly a proc macro could be used in pattern position with the proc_macro_hygiene feature.

jannschu commented 4 years ago

A solution without rusticata/der-parser#17 would be to expose the field of the Oid struct. On master an iterator over the oid arcs is returned. If one adds, say

impl Oid {
    fn identifiers(&self) -> &[u64] { &self.0 }
}

you could match like this:

const OID_RSA_ENCRYPTION: &[u64] = &[1, 2, 840, 113549, 1, 1, 1];
const OID_EC_PUBLIC_KEY: &[u64] = &[1, 2, 840, 10045, 2, 1];

fn foo(oid: Oid) {
    match oid.identifiers() {
        OID_RSA_ENCRYPTION => { .. },
        OID_EC_PUBLIC_KEY => { .. },
    }
}
chifflier commented 4 years ago

After merging rusticata/der-parser#18, things have changed since Oid now has Eq, PartialEq, Clone. So, you can match an Oid:

assert_eq!(oid, oid!(1.2.840.113549.1.1.1));

However, as explained, this cannot be used yet with global items because the construction of an Oid allocates memory. This would be a nice feature, I'll try to think of a solution.

There is a workaround converting to string, but I find it not elegant:

const OID_RSA_ENCRYPTION: &str = "1.2.840.113549.1.1.1";

fn compare_oid(oid: &Oid) -> bool {
    match oid.to_id_string().as_ref() {
        OID_RSA_ENCRYPTION => true,
        _ => false,
    }
}

Finally, the third solution, that I used for x509-parser (to be committed soon), is to allocate a HashMap using lazy_static and use get:

lazy_static! {
    static ref OID_REGISTRY: HashMap<Oid<'static>, OidEntry> = {
        let mut m = HashMap::new();
        m.insert(Oid::from(&[0]).unwrap(), OidEntry{sn:"UNDEF", ln:"undefined", nid:Nid::Undef});
        // ...
        m
    };
}

pub fn oid2nid(oid: &Oid) -> Result<Nid, NidError> {
    OID_REGISTRY
        .get(oid)
        .map(|ref o| o.nid)
        .ok_or(NidError)
}

Again, having a static/const declaration would be nice. @jannschu , any thoughts?

jannschu commented 4 years ago

@str4d: I played a bit with this. I suggest the following code with the new api:

use der_parser::oid;
const OID_RSA_ENCRYPTION: &[u8] = &oid!(raw 1.2.840.113549.1.1.1);
const OID_EC_PUBLIC_KEY: &[u8] = &oid!(raw 1.2.840.10045.2.1);
fn foo(oid: &Oid) {
    match oid.bytes() {
        OID_RSA_ENCRYPTION => { ... },
        OID_EC_PUBLIC_KEY => { ... },
        ...
    }
}

This is as static as it gets, I guess. No strings, no allocation. At compile time the constants will become the DER encoded oid. The match then compares the encoded forms.

You might want to add a check for a relative oid (if oid.relative { ... }) since this is not checked when comparing the encoded form.

I also discovered that using constants in patterns is still restricted (see https://github.com/rust-lang/rust/issues/31434 for progress).

The ideal api would allow

match some_oid {
    oid!(...) => {...},
    ...
}

But that is currently not achievable I think (you can use if some_oid == oid!(...), though). The reasons are the current limitations for procedural macros or constants in patterns.

@chifflier: The code in x509-parser/src/objects.rs can probably stay more or less the same by using &oid!(raw ...) and a check for "is relative" in the functions. Btw, I wouldn't be suprised if the compiler generates the same code as a match in this case, since we use iterators on constants.

jannschu commented 4 years ago

Maybe I should expand the Oid documentation.

jannschu commented 4 years ago

@chifflier: The code in x509-parser/src/objects.rs can probably stay more or less the same by using &oid!(raw ...) and a check for "is relative" in the functions. Btw, I wouldn't be suprised if the compiler generates the same code as a match in this case, since we use iterators on constants.

Even cleaner solution: Make Oid::from and Oid::from_relative const and then use

struct OidEntry {
    sn: &'static str,
    ln: &'static str,
    nid: Nid,
    oid: Oid<'static>,
}

const OID_REGISTRY : &[OidEntry] = &[
    OidEntry{ sn:"UNDEF", ln:"undefined", nid:Nid::Undef, oid: oid!(0) },
    ...
];

fn nid2sn(nid: Nid) -> Result<&'static str, NidError> {
    OID_REGISTRY
        .iter()
        .find(|ref o| o.nid == nid)
        .map(|ref o| o.sn)
        .ok_or(NidError)
}

I will create a pull request (and also add more documentation for Oid and the macro).

chifflier commented 4 years ago

Great, thanks @jannschu !

jannschu commented 4 years ago

It seems Rust 1.45 now supports proc macros in expression and pattern position. This might allow the oid! macro in pattern position.

chifflier commented 4 years ago

That looks interesting. I made a quick test, but Rust 1.45 still complains

    match oid {
        oid!(1.2.840.113549.1.1.1) => true,
        _ => false,
    }

gives the following error:

18 |         oid!(1.2.840.113549.1.1.1) => true,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |         |
   |         expected pattern
   |         in this macro invocation
   |         this macro call doesn't expand to a pattern

(tried adding raw but no change). I have no time for this now (I'm busy trying to remove all macros, and provide - and use - functional versions), but will have a look later

jannschu commented 3 years ago

With https://github.com/rust-lang/rust/issues/76001 we could (probably?) get

match oid {
    oid!(1.2.840.113549.1.1.1) => ...
}

The proc macro would then generate the code const { Oid::new(...) }.

The raw option does work with 1.45 already. Assuming proc-macro-hack is removed:

match oid.bytes() {
    oid!(raw 1.2.840.113549.1.1.1) => ...
}

This increases the minimum Rust version supported to 1.45.

chifflier commented 1 year ago

Transferring issue to asn1-rs, OID handling has been moved to this crate.

chifflier commented 1 year ago

(March 2023) using const code is still not possible in match arms, so there is no solution at this point.

The problem is related to the fact building an Oid object is only possible though (const) functions, and fonctions cannot be called in match arms.

Alpha-3 commented 1 year ago

Would replacing the current OID implementation with const-oid help here? I've had good results with const_oid so far in my attempts at enabling use of asn1_rs for embedded applications with no support for std/alloc.

chifflier commented 1 year ago

I have a branch here, where I added a full const replacement for Oid. However, having a second look, it has a lot of similarities with const-oid, although my implementation is much more generic.

I'll make a new experiment, trying to create an abstraction over const-oid but using it. If this works, it would allow having all what we need, and relying on const-oid seems a good idea (the crate is maintained and seems quite good).