der-parser icon indicating copy to clipboard operation
der-parser copied to clipboard

Make Oid::from a const fn

Open str4d opened this issue 5 years ago • 10 comments

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 #17.

str4d avatar Dec 01 '19 18:12 str4d

This probably requires #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 #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 avatar Jan 05 '20 12:01 jannschu

A solution without #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 => { .. },
    }
}

jannschu avatar Jan 05 '20 12:01 jannschu

After merging #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?

chifflier avatar Jan 09 '20 12:01 chifflier

@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 avatar Jan 09 '20 14:01 jannschu

Maybe I should expand the Oid documentation.

jannschu avatar Jan 09 '20 14:01 jannschu

@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).

jannschu avatar Jan 09 '20 14:01 jannschu

Great, thanks @jannschu !

chifflier avatar Jan 09 '20 15:01 chifflier

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

jannschu avatar Jul 16 '20 15:07 jannschu

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

chifflier avatar Aug 13 '20 11:08 chifflier

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.

jannschu avatar Sep 22 '20 09:09 jannschu