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

Bug 27717 - Require RSA key import to validate the key parameters #72

Closed
mwatson2 opened this issue May 24, 2016 · 7 comments
Closed

Bug 27717 - Require RSA key import to validate the key parameters #72

mwatson2 opened this issue May 24, 2016 · 7 comments

Comments

@mwatson2
Copy link
Collaborator

Bug 27717:

The RSA key import does not appear to mandate any validity tests on the key data. (for instance require that n = pq).

I recommend adding a step that validates the key parameters, and throws a DataError if they are not legitimate.

This would match up with EC key import, which minimally requires the public key to be a point on the curve, and throws a DataError if not.

@mwatson2
Copy link
Collaborator Author

In the public key case, I don't think it is possible to determine whether it is valid, though there are some invalid cases that can be detected (e < 3 or e >= n ), so we will need some wording like "If the key is determined to be invalid ..."

@mwatson2
Copy link
Collaborator Author

I have a vague recollection, though, of some discussion that perhaps some libraries don't check validity until you try to use the key. Does anyone have any information about that ?

mwatson2 added a commit to mwatson2/webcrypto that referenced this issue May 25, 2016
@mwatson2
Copy link
Collaborator Author

Pull Request #104 if we decide to go ahead.

@jimsch
Copy link
Collaborator

jimsch commented May 26, 2016

These are not completely comparable situations. There are two different issues that are being discussed here.

  1. If you import a public ECDH key, then there is a requirement to check that the point is on the required curve.
  2. If you import a private key and there is a public key in the structure, then validate that the public and private keys are associated with each other.

In the first case there is a clear reason to perform the operation. However in the second case I cannot see any reason to perform the check. The public key is going to be completely ignored by any and all operations on the resulting CryptoKey structure as only private key operations can be performed. There is no way to import a PKCS8 structure and get back a public key so the fact that there is no check that the public and private keys match each other does not seem to be very important.

@mwatson2
Copy link
Collaborator Author

mwatson2 commented Jun 8, 2016

@jimsch I agree those things are different, but what's suggested here is not checking that the private and public key are associated but checking that the private key is in fact a valid RSA private key according to RFC3447.

I am still looking for feedback on whether the crypto libraries used by browsers perform such checks on RSA key import, or only when they keys are used for an operation.

@jimsch
Copy link
Collaborator

jimsch commented Jun 8, 2016

Both of the examples that you gave above where dealing with either the public key or the fact that the private key produces the public key.

If you are talking about checks along the lines of p and q being prime numbers, ranges and so forth. That would be a different story. I don't know how many libraries do the checks that you want on import or if they just assume that they have been done however. It would be an interesting question to do research on.

@hhalpin
Copy link

hhalpin commented Jun 20, 2016

Accepting the change and then putting in the test-cases since, reverting if not the case.

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

No branches or pull requests

3 participants