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

FIPS 186-4 / SP800-56B - RSA Key Generation & Validation (FIPS) #6652

Closed
wants to merge 2 commits into
base: master
from

Conversation

@slontis
Copy link
Contributor

slontis commented Jul 5, 2018

Additional changes for SP800-56B r2 have been added i.e

  • keygen: keysizes are no longer just 2048 and 3072)
  • validation: the prime factors p,q can be recovered from n,e,d.

Checklist

  • tests are added or updated

@mattcaswell mattcaswell added this to the Post 1.1.1 milestone Jul 5, 2018

@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Jul 5, 2018

Travis failures look relevant.

Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
Show resolved Hide resolved crypto/bn/bn_prime.c Outdated

@richsalz richsalz changed the title FIPS 186-4 / SP800-56B - RSA Key Generation & Validation (FIPS) WIP: FIPS 186-4 / SP800-56B - RSA Key Generation & Validation (FIPS) Jul 5, 2018

@richsalz richsalz added hold FIPS labels Jul 5, 2018

Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
case 3072:
min_bits = 171;
break;
default:

This comment has been minimized.

@t-j-h

t-j-h Aug 30, 2018

Member

Needs to add support for > 3072 ... 4096 should be possible.

This comment has been minimized.

@slontis

slontis Aug 31, 2018

Author Contributor

I was following the SP800-56Br1 doc. It has changed in the 56Br2 draft docs.
Will update

This comment has been minimized.

@slontis

slontis Oct 5, 2018

Author Contributor

Code has now been updated.

This comment has been minimized.

@mattcaswell

mattcaswell Mar 8, 2019

Member

Code has now been updated.

Has it? I still don't see support for >3072

This comment has been minimized.

@slontis

slontis Mar 9, 2019

Author Contributor

Err No. Not sure how I managed to lose those changes. Redoing.

@kroeckx kroeckx self-requested a review Sep 28, 2018

@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Oct 2, 2018

I think the "hold" label for this can be removed now?

@mattcaswell mattcaswell removed the hold label Oct 2, 2018

@slontis slontis force-pushed the slontis:FIPS-rsa-keygen branch 4 times, most recently from c4a7f0e to b236be6 Feb 18, 2019

@paulidale paulidale added this to In progress in 3.0 New Core + FIPS via automation Feb 27, 2019

@paulidale paulidale modified the milestones: Post 1.1.1, 3.0.0 Feb 27, 2019

@slontis slontis changed the title WIP: FIPS 186-4 / SP800-56B - RSA Key Generation & Validation (FIPS) FIPS 186-4 / SP800-56B - RSA Key Generation & Validation (FIPS) Feb 28, 2019

@slontis

This comment has been minimized.

Copy link
Contributor Author

slontis commented Mar 4, 2019

ping

@paulidale
Copy link
Contributor

paulidale left a comment

Small points mostly.

Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c Outdated
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated

3.0 New Core + FIPS automation moved this from In progress to Needs review Mar 5, 2019

@paulidale paulidale added the master label Mar 5, 2019

@slontis slontis force-pushed the slontis:FIPS-rsa-keygen branch from b236be6 to 8ad800b Mar 5, 2019

3.0 New Core + FIPS automation moved this from Needs review to Reviewer approved Mar 5, 2019

@paulidale

This comment has been minimized.

Copy link
Contributor

paulidale commented Mar 7, 2019

Ping?

@slontis

This comment has been minimized.

Copy link
Contributor Author

slontis commented Mar 7, 2019

ping

@mattcaswell
Copy link
Member

mattcaswell left a comment

Still going through this. Review comments so far

Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
Show resolved Hide resolved crypto/bn/bn_prime.c
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c Outdated
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c Outdated
Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c
Show resolved Hide resolved crypto/rsa/rsa_chk.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_gen.c
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c Outdated

3.0 New Core + FIPS automation moved this from Reviewer approved to Needs review Mar 7, 2019

Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated

@slontis slontis force-pushed the slontis:FIPS-rsa-keygen branch 2 times, most recently from 105ecc1 to 77a5833 Mar 8, 2019

case 3072:
min_bits = 171;
break;
default:

This comment has been minimized.

@mattcaswell

mattcaswell Mar 8, 2019

Member

Code has now been updated.

Has it? I still don't see support for >3072

Show resolved Hide resolved crypto/bn/bn_rsa_fips186_4.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c Outdated

@slontis slontis force-pushed the slontis:FIPS-rsa-keygen branch from 77a5833 to 7aeacfc Mar 9, 2019

3.0 New Core + FIPS automation moved this from Needs review to Reviewer approved Mar 11, 2019

@mattcaswell
Copy link
Member

mattcaswell left a comment

Approved - @paulidale to reconfirm?

@paulidale
Copy link
Contributor

paulidale left a comment

Reapproved with some trivial nits.
@mattcaswell could you please merge this.

Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
Show resolved Hide resolved crypto/bn/bn_prime.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_check.c Outdated
Show resolved Hide resolved crypto/rsa/rsa_sp800_56b_gen.c Outdated

@slontis slontis force-pushed the slontis:FIPS-rsa-keygen branch from 7aeacfc to 0782780 Mar 12, 2019

@slontis

This comment has been minimized.

Copy link
Contributor Author

slontis commented Mar 12, 2019

rebased with small nit changes from Paul

@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Mar 12, 2019

Pushed. Thanks.

3.0 New Core + FIPS automation moved this from Reviewer approved to Done Mar 12, 2019

levitte pushed a commit that referenced this pull request Mar 12, 2019

FIPS 186-4 RSA Generation & Validation
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6652)

levitte pushed a commit that referenced this pull request Mar 12, 2019

added generated files
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6652)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.