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

Only generate parameters approved by current crypto-policy #19

Closed
cipherboy opened this issue Sep 1, 2020 · 14 comments · Fixed by #24
Closed

Only generate parameters approved by current crypto-policy #19

cipherboy opened this issue Sep 1, 2020 · 14 comments · Fixed by #24
Milestone

Comments

@cipherboy
Copy link

See also: https://bugzilla.redhat.com/show_bug.cgi?id=1871988

Essentially, dhparam generation in FIPS mode should only allow output of known parameter sets, rather than actually invoking the underlying OpenSSL calls. Various linked BZs describe the actual set of allowed parameters. Perhaps a "named parameter" generator could be provided?

Thanks!

@sgallagher
Copy link
Owner

I'm not entirely sure what you are asking for here. SSCG already provides:

      --dhparams-prime-len=INT                          The length of the prime number to generate for dhparams, in bits. (default: 2048)
      --dhparams-generator={2,3,5}                      The generator value for dhparams. (default: 2)

Are you asking for the defaults to differ depending on SSL_CTX_get_security_level()? If so, what should they be?

@sgallagher
Copy link
Owner

Maybe you could provide a patch to https://github.com/sgallagher/sscg/blob/master/src/sscg.c#L59 ?

@cipherboy
Copy link
Author

cipherboy commented Sep 2, 2020

The current output of this function is a random Diffie-Hellman group parameters. prime-len describes the length (in bits) of the prime field; this field is random (i.e., prime is a random prime fitting certain criteria). The generator (2, 3 -- which is kinda out of favor, and 5) describes the generator on this group (g^n mod prime -- where g is the generator specified). It doesn't impact security much AFAICT, unless it happens to fall in a small sub-group. OpenSSL is mostly supposed to protect us from that though.

Instead, per FIPS, it really shouldn't be generating any parameters at all, but spitting out one of the pre-approved values instead. This is also more in line with what, e.g., TLSv1.3 is expecting also.

You can see some more context on some of these other BZs (some are internal only):

So I'd add a parameter:

--dhparams-named=NAME    The name of a well-known dhparam to generate (such as rfc3526-group-18-8192)

And have it output some well-known dhparam values from RFCs such as:

&c.

@sgallagher
Copy link
Owner

Sorry it's taken so long to get back to this, but I'm working on it now. Can you tell me if there's a standard I can use for the well-known names? You gave rfc3526-group-18-8192 is somewhat redundant, since group 18 is the 8192-bit example. I just want to make sure that whatever I implement here will match what people expect to use.

@cipherboy
Copy link
Author

cipherboy commented Jul 15, 2021

Hi @sgallagher,

Two things here...

  1. No, my names were made up, and yes, that is redundant :-D
  2. You might want to sync with the (Red Hat) crypto team and ask them to talk to/put you in touch with Stephan Muller at AtSec -- he mentioned there'd be a really easy patch to OpenSSL to get it to output these curves directly. If you or him or someone on the crypto team want to upstream that patch (or write it yourselves), you can avoid needing to carry these curves in sscg and simply pass the right flags to an API call.

My 2c., hth.

@sgallagher
Copy link
Owner

  1. You might want to sync with the (Red Hat) crypto team and ask them to talk to/put you in touch with Stephan Muller at AtSec -- he mentioned there'd be a really easy patch to OpenSSL to get it to output these curves directly. If you or him or someone on the crypto team want to upstream that patch (or write it yourselves), you can avoid needing to carry these curves in sscg and simply pass the right flags to an API call.

They're actually already available in OpenSSL 3.0: https://www.openssl.org/docs/man1.1.0/man3/BN_get_rfc3526_prime_8192.html

I'm currently working on OpenSSL 3.0 compatibility, after which I'll look at backporting to OpenSSL 1.1.1.

@cipherboy
Copy link
Author

cipherboy commented Jul 15, 2021

Aha :-) Well that's definitely useful! Perhaps he was talking about backporting this patch to the 1.1.1 series then.


Edit: I just realized that the link you posted is in the 1.1.1 maintenance branch as well, so all modern OpenSSL versions should have that function since 2016: openssl/openssl@9021a5dfb37

@sgallagher sgallagher added this to the 3.0.0 milestone Jul 20, 2021
@sgallagher
Copy link
Owner

@cipherboy Would you be so kind as to build and test the contents of the openssl3 branch? (Ignore the name; originally it was meant for OpenSSL 3.0 changes, but it evolved to include this change as well). I want to make sure this matches what you're looking for.

sgallagher added a commit that referenced this issue Jul 21, 2021
Fixes #19

Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
@sgallagher sgallagher linked a pull request Jul 21, 2021 that will close this issue
@sgallagher
Copy link
Owner

@cipherboy You can install testing packages from https://copr.fedorainfracloud.org/coprs/packit/sgallagher-sscg-24/

@cipherboy
Copy link
Author

cipherboy commented Jul 21, 2021

@sgallagher This looks fairly good, but from a UX perspective the CLI doesn't give a way of indicating what the supported group names are. Maybe if a bad option is specified it should output an error like The allowed group names are... ?

Otherwise, I used ffdhe8192 (from looking at the code) and got a rfc7919 known DH parameter so I think it looks good from my PoV.

I didn't do a code review :-)

Thanks!

@sgallagher
Copy link
Owner

@sgallagher This looks fairly good, but from a UX perspective the CLI doesn't give a way of indicating what the supported group names are. Maybe if a bad option is specified it should output an error like The allowed group names are... ?

I'll see what I can do to improve this here, but the --help text gives:

      --dhparams-named-group=STRING                     Output well-known DH parameters. See
                                                        https://www.openssl.org/docs/manmaster/man7/EVP_KEYMGMT-DH.html for details on the
                                                        available groups. (Default: "ffdhe4096")

I agree, a more helpful error message might be useful.

@cipherboy
Copy link
Author

Ah ok, I see now. That's probably fine if I actually had taken the time to read it. ;-)

@sgallagher
Copy link
Owner

The help text is more clear now:

      --dhparams-named-group=STRING                     Output well-known DH parameters. The available named groups are: ffdhe2048,
                                                        ffdhe3072, ffdhe4096, ffdhe6144, ffdhe8192. (Default: "ffdhe4096")

(Note that's for OpenSSL 1.1; the list is longer on OpenSSL 3.0).

It will also provide this list as an error message if an unknown one was specified.

@sgallagher
Copy link
Owner

The help text is more clear now:

      --dhparams-named-group=STRING                     Output well-known DH parameters. The available named groups are: ffdhe2048,
                                                        ffdhe3072, ffdhe4096, ffdhe6144, ffdhe8192. (Default: "ffdhe4096")

(Note that's for OpenSSL 1.1; the list is longer on OpenSSL 3.0).

It will also provide this list as an error message if an unknown one was specified.

Thanks for the feedback!

sgallagher added a commit that referenced this issue Jul 21, 2021
Fixes #19

Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
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

Successfully merging a pull request may close this issue.

2 participants