Conversation
levitte
added
branch: master
|
I fully intend to use this DER writer in #11422 |
|
Looks kinda neat. Karma points for removing the "aid" files. Can we instead augment objects.txt somehow, or have a file that points into objects.txt so that we only have to write OID's in one place? I assume that while you sometimes you shorthands "{ pkcs1 12 }" and other times you write the full OID is because that's what the source documents do. |
mattcaswell
left a comment
mattcaswell
left a comment
There was a problem hiding this comment.
Ran out of time reviewing this - but here's my comments so far
|
Will look a tomorrow |
I'd rather not. The objects.txt format is a despicable pile of hacks on top of a pile of hacks, and I'm just as culpable for getting it there as anyone. Having to rewrite RFC input to that format is a waste of cycles when it wasn't too hard to put together something that can take OBJECT IDENTIFIER definitions directly. I'd say it's better to start over fresh. Besides, this is an internal thing for our providers, entirely separate from everything that has to do with objects.txt, and needed because we have banned OBJ and ASN1 routines from the FIPS module. |
|
I guess that given your affinity for perl, you're okay with more than one way to do it :) Bummer. |
If it were entirely up to me, I'd replace objects.txt and the Perl 4 flavored code that supports it. |
!!! please |
That's further in the future, mind... |
Yup. Straight copy, I only needed remove page footer and header when what i copied crossed from one RFC page to the next. |
slontis
left a comment
slontis
left a comment
There was a problem hiding this comment.
Good idea - It is probably less error prone doing it this way.
But maybe a bit more cryptic for someone to set up.
Any time new DER functions are created it is a bit of a worry that some edge case is going to cause issues.
paulidale
left a comment
paulidale
left a comment
There was a problem hiding this comment.
Seems like a good approach.
Travis looks relevant.
There's that risk whichever way we turn... It shouldn't be too hard to write a test, though... |
|
Travis is drunk and needs to catch up. Close/open kick happening now |
|
Travis failure is a timeout |
mattcaswell
left a comment
mattcaswell
left a comment
There was a problem hiding this comment.
Had more time to look at the rest of this. Further comments below.
providers/common/der/der_writer.c
Outdated
There was a problem hiding this comment.
Not sure the shifting is quite right here. For example, if n == BN_BYTES then we shift by 0 bits. Surely in that case we should shift by 8 * (BN_BYTES - 1) bits.
There was a problem hiding this comment.
No. n == BN_BYTES is the lowest byte in word [1], not the highest byte in word [0].
There was a problem hiding this comment.
Oh wait, now I see what you said...
|
So, did I correctly got that you've introduced provider-level set of OIDs and when we don't need corresponding NIDs for any purposes, most of, e.g. GOST OIDs can be removed from objects.txt and processed separately on provider-level? |
One of the objectives with the new core effort is that there should be no reason whatsoever for a provider author to have any specific information "registered" with OpenSSL. We're not entirely there yet, libssl needs some deep mods to stop depending on registered NIDs. |
|
I tried to look at the (internal) man pages, and for the 'cont' arguments, "context number of the element being handled" didn't really give me a clear picture. Is it supposed to be the ASN.1 tag value when explicit tags are used? |
Yes. I took the name because it's the context-specific tag, describe like this in X.690 (italics mine):
Feel free to propose a better text regarding this. As a matter of fact, I'd be grateful if you did. |
|
Woot! CIs are happy at least 😄 |
|
ctag or similar for context tag? or asn1tag? |
I put some words up at levitte#2 . Okay, and some code, too. |
Make the description more comprehensible to me.
Use 'tag' instead of 'cont' for the parameter name.
Assert that the given tag value is representable in a single octet.
Taken!
You're too modest, I didn't find any of it particularly invasive. It was all straight to the point. Thank you. |
We have an old OID database that's not as readable as would be desired, and we have spots with hand coded DER for well known OIDs. The perl modules added here give enough support that we can parse OBJECT IDENTIFIER definitions and encode them as DER. OpenSSL::OID is a general OID parsing and encoding of ASN.1 definitions, and supports enough of the X.680 syntax to understand what we find in RFCs and similar documents and produce the DER encoding for them. oids_to_c is a specialized module to convert the DER encoding from OpenSSL::OID to C code. This is primarily useful in file templates that are processed with util/dofile.pl. Reviewed-by: Matt Caswell <[email protected]> (Merged from #11450)
This library is meant to be small and quick. It's based on WPACKET,
which was extended to support DER writing. The way it's used is a
bit unusual, as it's used to write the structures backward into a
given buffer. A typical quick call looks like this:
/*
* Fill in this structure:
*
* something ::= SEQUENCE {
* id OBJECT IDENTIFIER,
* x [0] INTEGER OPTIONAL,
* y [1] BOOLEAN OPTIONAL,
* n INTEGER
* }
*/
unsigned char buf[nnnn], *p = NULL;
size_t encoded_len = 0;
WPACKET pkt;
int ok;
ok = WPACKET_init_der(&pkt, buf, sizeof(buf)
&& DER_w_start_sequence(&pkt, -1)
&& DER_w_bn(&pkt, -1, bn)
&& DER_w_boolean(&pkt, 1, bool)
&& DER_w_precompiled(&pkt, -1, OID, sizeof(OID))
&& DER_w_end_sequence(&pkt, -1)
&& WPACKET_finish(&pkt)
&& WPACKET_get_total_written(&pkt, &encoded_len)
&& (p = WPACKET_get_curr(&pkt)) != NULL;
Reviewed-by: Matt Caswell <[email protected]>
(Merged from #11450)
This replaces crypto/rsa/rsa_aid.c with new code and generated OIDs Reviewed-by: Matt Caswell <[email protected]> (Merged from #11450)
This replaces crypto/dsa/dsa_aid.c with new code and generated OIDs Reviewed-by: Matt Caswell <[email protected]> (Merged from #11450)
This replaces crypto/ec/ecdsa_aid.c with new code and generated OIDs Reviewed-by: Matt Caswell <[email protected]> (Merged from #11450)
|
I think this was a very nice example of collaboration. |
|
Yup, I thoroughly enjoyed that |
10 participants
When thinking about adding a lot more OID and AID generation, the thought of hand crafting more OID DER encoding made me cringe enough to bring in an OID reader/encoder that I'd already written and perl template support so we could get those encodings as C arrays.
The result is a ASN.1 reader that knows enough to find all OBJECT IDENTIFIER definitions in a given file and have that result in relevant C arrays. That way, we only need to copy OBJECT IDENTIFIER definitions from diverse standards and fire away.
Furthermore, this adds a small, quick and easily extensible DER writing library.
crypto/rsa/rsa_aid.c,crypto/dsa/dsa_aid.c,crypto/ec/ecdsa_aid.care replaced by this solution.