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

How to represent "ANY IDENTIFIED BY ..." #14

Open
NathanReb opened this issue Dec 4, 2015 · 7 comments
Open

How to represent "ANY IDENTIFIED BY ..." #14

NathanReb opened this issue Dec 4, 2015 · 7 comments

Comments

@NathanReb
Copy link

First of all I'd like to thank you for your work, asn1-combinators is a great library and
I'm using it every day.

At cryptosense we're open sourcing some the of the asn1 cryptographic key parsers we've wrote
so far and we're facing an issue.

I'm trying to express an ASN grammar for algorithmIdentifier which is defined in
RFC 5280 as:

AlgorithmIdentifier  ::=  SEQUENCE  {
    algorithm               OBJECT IDENTIFIER,
    parameters              ANY DEFINED BY algorithm OPTIONAL  }

When only considering RSA and DSA, it works well because RSA params are NULL and DSA ones are
a 3 integer sequence. The thing is when adding EC keys to the equation the grammar becomes
ambiguous because EC parameters can also be a sequence, even though it's completely
different from the DSA one.

Currently we have a different grammar for each key types and the generic decode function
is a very ugly doubly nested try ... with Asn.Parse_error _ ->.

I wrote a wildcard grammar with choice and fix allowing me to accept "almost" everything
but then I have to manipulate the recursive type and to write converters from and to it which
adds a lot of code and doesn't really look prettier in the end.

ASN.1 construction like:

Something ::= SEQUENCE {
    id                  OID,
    params              ANY IDENTIFIED BY id }

is pretty usual and I guess I'm not the only one that could run into this kind of problem.

I'd really like to know what you suggest to adress this issue.

Thank you :)

@hannesm
Copy link
Member

hannesm commented Dec 4, 2015

I'd use a choice and postprocessing, similar to how we do it in X.509: https://github.com/mirleft/ocaml-x509/blob/master/lib/asn_grammars.ml#L332-L418 (no EC support there yet). Maybe the parsing code (and esp. the ASN OID registry) should be a separate OPAM package, so that we do not have to duplicate this information in all the packages?

@pqwy
Copy link
Contributor

pqwy commented Dec 5, 2015

@NathanReb

I see what you mean. You cannot encode different params as CHOICE, because CHOICE components have to be internally disambiguated, whereas ANY IDENTIFIED BY doesn't.

I had a combinator in mind would would allow expressing this cleanly. I'll hack it in the next few days, and get back to you.

In the meantime, would you mind checking how does the other branch work for you? The parser changed and has different performance tradeoffs, but that's the branch I intend to continue working from.

@NathanReb
Copy link
Author

Sure ! I'll give it a try

@NathanReb
Copy link
Author

Ok so I installed the new asn1-combinators you gave me yesterday. I've tried it a bit on our open
source lib and it works fine.
We're currently still using some old parsers, waiting for the elliptic curve key part of
the key-parsers lib to be released so I ran the Cryptosense Analyzer test suite.
I found out that one of the test now fails. The good thing is it fails because of a
mistake within an EC private key grammar.
The question is why doesn't it fail with the current asn1-combinators version ?

The following is the base64 encoding of the DER-encoded EC private key we use as test
data:

MHQCAQEEIB7LbYFLdWg6VfF9/Xnc8AAOSoWJiB+5qtQvwtC2NCU/oAcGBSuBBAAKoUQDQgAE2RHVNu5/Opj8LzhZ+V4FoWM7JyzMMo3surH1iTOArw3GSVuCYaWYKhTBaUfiiy2IRD/HlHsCI1xBFtwUvrGCKw==

This is how the grammar was expressed:

sequence4
  (required ~label:"version" asn_version)
  (required ~label:"privateKey" octet_string)
  (optional ~label:"parameters" @@ explicit 0 oid)
  (optional ~label:"publicKey" @@ explicit 1 octet_string)

The test passes with the current version and fails with the one you gave me.

And this is how it should be:

sequence4
  (required ~label:"version" asn_version)
  (required ~label:"privateKey" octet_string)
  (optional ~label:"parameters" @@ explicit 0 oid)
  (optional ~label:"publicKey" @@ explicit 1 bit_string_cs)

This passes the test with both the current and the future version.

@NightBlues
Copy link

@NathanReb

I see what you mean. You cannot encode different params as CHOICE, because CHOICE components have to be internally disambiguated, whereas ANY IDENTIFIED BY doesn't.

I had a combinator in mind would would allow expressing this cleanly. I'll hack it in the next few days, and get back to you.

In the meantime, would you mind checking how does the other branch work for you? The parser changed and has different performance tradeoffs, but that's the branch I intend to continue working from.

Guys, I'm trying to implement PKCS12 (https://tools.ietf.org/html/rfc7292) and I ran into the same problem:

 SafeBag ::= SEQUENCE {
     bagId         BAG-TYPE.&id ({PKCS12BagSet}),
     bagValue      [0] EXPLICIT BAG-TYPE.&Type({PKCS12BagSet}{@bagId}),
     bagAttributes SET OF PKCS12Attribute OPTIONAL
 }
...
 keyBag BAG-TYPE ::=
     {KeyBag              IDENTIFIED BY {bagtypes 1}}
 certBag BAG-TYPE ::=
     {CertBag             IDENTIFIED BY {bagtypes 3}}
...
-- KeyBag
 KeyBag ::= PrivateKeyInfo
-- CertBag
 CertBag ::= SEQUENCE {
...

KeyBag and CertBag are both sequences and I tried to implement them through choice.
How do you hack it?

@hannesm
Copy link
Member

hannesm commented Dec 10, 2020

FWIW, I took a look on PKCS12 some time ago at mirleft/ocaml-x509#114 and avoided the ANY issue (though the asn1 looks pretty ugly)

@NightBlues
Copy link

Oh, it would be better for me, if I had found this PR before started :) (NightBlues/ocaml-x509#1)
Thank you, I'll steal some code from your PR:)

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

No branches or pull requests

4 participants