Simple ASN1 utils for DSA based on PACKET#9111
Simple ASN1 utils for DSA based on PACKET#9111mattcaswell wants to merge 5 commits intoopenssl:masterfrom
Conversation
|
Note: the interesting changes are in the asn1_dsa.c file in the decode* functions. |
crypto/asn1_dsa.c
Outdated
There was a problem hiding this comment.
that last clause looks strange. perhaps a comment?
There was a problem hiding this comment.
Oh, it looked strange because it wasn't quite right. Fixed...and I actually ended up removing this whole function anyway.
mattcaswell
force-pushed
the
simple-asn1-utils
branch
from
0382e76 to
8051bd8
Compare
|
I'd like to see the encode side, this is where WPACKET doesn't match up with ASN.1 so nicely. Would a fuller ropes implementation be worthwhile? With the variable length representation of ASN.1, it might be reasonable. In my ideal world, ASN.1 decoding would be done using a grammar and a generated parser. |
|
I agree the encode side is the harder part, particularly for nested structures, as opposed to a simple sequence. Do we know what ASN1 we will have to have in the FIPS module? I am against a fuller ropes implementation until we know it's needed. You can always allocate the max space, keep a stack, and backpatch and squeeze up for example. |
... although, if DSA sigs are the only use case, maybe there's not much to worry about... |
|
For an example of the patch/backtrack method, see http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=blob;f=libraries/liblber/encode.c;h=8f6d4f6ea0c62a88ed54f1226b198217cacc1624;hb=HEAD#l360 I think the Brisbane team can easily post a comprehensive list of what we need; decode RSA-PSS parameters? |
|
I updated this to use the WPACKET API as well. As part of that I gave WPACKET the ability to write to a NULL buffer so the number of bytes required can be calculated before later initialising the buffer. |
|
Update pushed addressing the comment rewording suggestion. |
mattcaswell
changed the title
|
Taken this out of "WIP" |
crypto/asn1_dsa.c
Outdated
There was a problem hiding this comment.
"Results in... " rather than "Returns"
Adds simple utility functions to allow both the default and fips providers to encode and decode DSA-Sig-Value and ECDSA-Sig-Value (DSA_SIG and ECDSA_SIG structures) to/from ASN.1 DER without requiring those providers to have a dependency on the asn1 module.
This means the WPACKET API can be used for calculating the number of bytes that would have been written if a non-NULL buffer had been used. This enables us to calculate the number of length bytes required when encoding ASN.1
mattcaswell
force-pushed
the
simple-asn1-utils
branch
from
9943038 to
bc3c377
Compare
|
Rebased to resolve a conflict with master. Ping? |
|
I'm okay to approve both this and #9067. Has a decision been made as to which is preferred? |
Well no decision has been made as such. My own view is I strongly prefer the PACKET/WPACKET approach. Outside of that there hasn't been much discussion. My -1 on #9067 still stands. No one so far has seen fit to take that one to a vote. If an OMC member wants to take that to a vote then I'd be happy to take the debate to the OMC. Otherwise, I'd be happy to take an approval of this forward without a vote. |
I think it's wrong to block a PR without being willing to call a vote. |
|
Merged to master after removing end of line trailing space. |
Adds simple utility functions to allow both the default and fips providers to encode and decode DSA-Sig-Value and ECDSA-Sig-Value (DSA_SIG and ECDSA_SIG structures) to/from ASN.1 DER without requiring those providers to have a dependency on the asn1 module. Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #9111)
Reviewed-by: Paul Dale <[email protected]> (Merged from #9111)
Reviewed-by: Paul Dale <[email protected]> (Merged from #9111)
This means the WPACKET API can be used for calculating the number of bytes that would have been written if a non-NULL buffer had been used. This enables us to calculate the number of length bytes required when encoding ASN.1 Reviewed-by: Paul Dale <[email protected]> (Merged from #9111)
Reviewed-by: Paul Dale <[email protected]> (Merged from #9111)
Reviewed-by: Matt Caswell <[email protected]> (Merged from #9111)
5 participants
This is based on the PR by @davidmakepeace in #9067. It uses all the same commits as there, but then adds some additional ones to convert the code to use the PACKET API instead.
This is WIP because I have not yet tackled WPACKET. I ran out of time today. Hopefully it is enough for now to get a feel for what it would look like. I'll work on WPACKET next week.