accept PKCS#3 DH parameters with privateValueLength included

[dkg]

library/dhm.c: accept (and ignore) optional privateValueLength for
PKCS#3 DH parameters.

PKCS#3 defines the ASN.1 encoding of a DH parameter set like this:

---

DHParameter ::= SEQUENCE {
prime INTEGER, -- p
base INTEGER, -- g
privateValueLength INTEGER OPTIONAL }

The fields of type DHParameter have the following meanings:

 o    prime is the prime p.

 o    base is the base g.

 o    privateValueLength is the optional private-value
      length l.

---

See: ftp://ftp.rsasecurity.com/pub/pkcs/ascii/pkcs-3.asc

This optional parameter was added in PKCS#3 version 1.4, released
November 1, 1993.

dhm.c currently doesn't cope well with PKCS#3 files that have this
optional final parameter included. i see errors like:

---

dhm_parse_dhmfile returned -0x33E6

Last error was: -0x33E6 - DHM - The ASN.1 data is not formatted correctly : ASN1 - Actual length differs from expected lengt

You can generate PKCS#3 files with this final parameter with recent
versions of certtool from GnuTLS:

certtool --generate-dh-params > dh.pem

[mpg]

Hi Daniel,

Thanks for your contribution! I quickly looked at the patch and it looks good. We'll need some test cases for that but we can add them ourselves based on the command you mentioned.

@pjbakker : Paul, can you take care of the legal details if any?

Of course the next step is to actually use this parameter, which I just added as a todo item.

[dkg]

On Tue 2015-04-07 05:03:17 -0400, Manuel Pégourié-Gonnard wrote:

Thanks for your contribution! I quickly looked at the patch and it looks good. We'll need some test cases for that but we can add them ourselves based on the command you mentioned.

great, i'm glad to hear it sounds sensible.

@pjbakker : Paul, can you take care of the legal details if any?

If you think that copyright is relevant in this case, i happily
contribute the code i've written under the GPL, version 2 or later.
Please let me know if you need any more details.

Of course the next step is to actually use this parameter, which I just added as a todo item.

I avoided doing that because it adding a new element to dhm_context
appears to cause an API/ABI change (since dhm_context is defined directly in
includes/dhm.h), and i didn't want backporting this fix to existing code
to require an API/ABI change.

If your structs were declared in an internal header and only referenced
by pointers externally, then an upgrade to full functionality without an
API change would be possible.

Fixing this would probably require changing dhm_init to return a
dhm_context* instead of taking one as an input, though, which is an even
bigger API/ABI change (one that wouldn't be fixed for dependent code by
a simple recompile against the new version). :/

--dkg

[mpg]

I agree with your comments regarding API/ABI changes. In the next major version we'll be moving some structure definitions to internal headers or even C files, but for most structures we prefer to allow people to use non-dynamic allocation, especially for the basic modules that may be used in environments where dynamic allocation is not welcome. (This obviously doesn't apply to the DHM module, the argument would probably be uniformity, and also that a few ABI changes are not such a big price to pay.)

Anyway, I did not mean to imply your contribution was incomplete, just mentioning the next step we're likely to take FYI. Thanks for taking the ABI compatibility into consideration btw.

[first message sent early due to misclicking, as well as closing/reopening, sorry for that]

[mpg]

[previous message sent early due to misclicking, as well as closing/reopening, sorry for that]

[pjbakker]

@dkg In order to be able to use a contribution a CLA needs to be signed. We need more rights than just GPL for that. Can you send me an e-mail (paul dot bakker at arm dot com) so I can send you the document?

[mpg]

Merged, thanks!