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

clarify content of algorithm member of ScopedCredentialParameters #113

Closed
equalsJeffH opened this issue Jun 1, 2016 · 4 comments
Closed

Comments

@equalsJeffH
Copy link
Contributor

in https://w3c.github.io/webauthn/#credential-params, the statement..

  The algorithm member specifies the cryptographic algorithm with 
  which the newly generated credential will be used.

ought to be something more akin to..

  The algorithm member specifies the cryptographic key generation 
  algorithm parameters with which the credential key pair MUST be 
  generated.

..and further refinement beyond that may be needed:

I.e., a question is just how much of gnarly key gneration params ought a RP webapp be supplying, and how much does the underlying client platform supply?
See https://www.w3.org/TR/WebCryptoAPI/#examples-section for an example of the full unabridged key gen params used to generate an RSA key pair. Perhaps the RP webapp could pass in only as much as it cares/needs to specify..

  { "name": "RSASSA-PKCS1-v1_5" }

..on the makeCredential() method, and the underlying platform takes care of the other details (i.e., modulusLength, publicExponent, and hash), or it could pass in..

  { "name": "RSASSA-PKCS1-v1_5",
     "hash":"SHA-256" }

..or the complete algorithmIdentifier as shown in the webcrypto spec example..

var algorithmKeyGen = {
  name: "RSASSA-PKCS1-v1_5",
  // RsaHashedKeyGenParams
  modulusLength: 2048,
  publicExponent: new Uint8Array([0x01, 0x00, 0x01]),  // Equivalent to 65537
  hash: {
    name: "SHA-256"
  }
};
@apowers313
Copy link
Contributor

The generation of keys based on the requested crypto parameters is currently specified as a "best effort" activity. From Section 4.1.1 makeCredential:

The cryptoParameters parameter supplies information about the desired properties of the credential to be created. The sequence is ordered from most preferred to least preferred. The platform makes a best effort to create the most preferred credential that it can.

@equalsJeffH
Copy link
Contributor Author

I believe you are misunderstanding the meaning of the prose you quote above. that prose in {#makeCredential} section is regarding the sequence of cryptoParameters passed into the makeCredential() promise. It is regarding (a) how the RP denotes the priority of the cryptoparameters it desires, and (b) how the platform selects which entry of the cryptoParameters sequence it honors. This is orthogonal to the theme of the original issue above.

@apowers313
Copy link
Contributor

The words "desired" and "best effort" led me to believe that cryptoParameters was just a guide, and that if no match in cryptoParameters was found then any alternative credential would be acceptable. I would suggest clarifying that if no match is found in cryptoParameters an error is returned.

Returning to your question about how much detail an RP App should provide in specifying a credential, isn't this already addressed by the definition of AlgorithmIdentifier which may be either a string or an object? The object can contain all the details you want and if it's a string like "RSASSA-PKCS1-v1_5", then the normalizing algorithm will fill in the details?

PS - Hopefully it's obvious, but we may want to specify that the algorithm must be one that supports the sign and verify methods, as described in WebCrypto Section 19.

@equalsJeffH equalsJeffH changed the title clarify content of algorithm member of copedCredentialParameters clarify content of algorithm member of ScopedCredentialParameters Aug 22, 2016
@equalsJeffH
Copy link
Contributor Author

The words "desired" and "best effort" led me to believe that cryptoParameters was just a guide, and that if no match in cryptoParameters was found then any alternative credential would be acceptable.

I do not think that is the case given how the spec is currently written 2b72ddf, specifically in section {#makeCredential}

I would suggest clarifying that if no match is found in cryptoParameters an error is returned.

yes, it seems that one could complete step 5 in {#makeCredential} having a zero-length normalizedParameters sequence. E.g., if the caller specified incorrect, mangled, or malformed cryptoParameters.algorithm. Presently, it appears the overall {#makeCredential} process will pass a zero-length normalizedParameters sequence to all the authnrs on the platform (in step 8), and it is tacitly up to the authnrs (in step 9) to either time out or return an error status. This could be overall made explicit for the zero-length normalizedParameters sequence case, though seems to me to be a medium- or low-priority.

Returning to your question about how much detail an RP App should provide in specifying a credential, isn't this already addressed by the definition of AlgorithmIdentifier which may be either a string or an object? The object can contain all the details you want and if it's a string like "RSASSA-PKCS1-v1_5", then the normalizing algorithm will fill in the details?

yes, that is understood, though it is tough (at least it was for me) to parse out of the WebCrypto spec and we may wish to include some sort of example or guidance in the WebAuthn spec. In any case, I believe the "algorithm member" language cited at the beginning of the issue merits polishing.

PS - Hopefully it's obvious, but we may want to specify that the algorithm must be one that supports the sign and verify methods, as described in WebCrypto Section 19.

Agreed, I had noticed that also. Additionally they need to support key generateKey (in terms of public-private key pair gen) it would seem. fwiw, there appears to be only three algs supporting all three: RSASSA-PKCS1-v1_5, RSA-PSS, and ECDSA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants