Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Oid::from a const fn #20

Open
str4d opened this issue Dec 1, 2019 · 14 comments
Open

Make Oid::from a const fn #20

str4d opened this issue Dec 1, 2019 · 14 comments

Comments

@str4d
Copy link

str4d commented Dec 1, 2019

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
Copy link

jannschu commented Jan 5, 2020

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 dtolnay/proc-macro-hack#20 and rust-lang/rust#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
Copy link

jannschu commented Jan 5, 2020

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
Copy link
Member

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
Copy link

jannschu commented Jan 9, 2020

@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 rust-lang/rust#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
Copy link

jannschu commented Jan 9, 2020

Maybe I should expand the Oid documentation.

@jannschu
Copy link

jannschu commented Jan 9, 2020

@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
Copy link
Member

Great, thanks @jannschu !

@jannschu
Copy link

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

@chifflier
Copy link
Member

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
Copy link

With rust-lang/rust#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 chifflier transferred this issue from rusticata/der-parser Mar 7, 2023
@chifflier
Copy link
Member

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

@chifflier
Copy link
Member

(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
Copy link

Alpha-3 commented Jun 13, 2023

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants