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

AES importKey never sets the key's [[type]] internal slot #376

Closed
BenWiederhake opened this issue Oct 25, 2024 · 4 comments · Fixed by #378
Closed

AES importKey never sets the key's [[type]] internal slot #376

BenWiederhake opened this issue Oct 25, 2024 · 4 comments · Fixed by #378

Comments

@BenWiederhake
Copy link
Collaborator

BenWiederhake commented Oct 25, 2024

I don't see where [[type]] is being set in the case of AES-CBC importKey, or in fact any of the AES variants. Maybe I overlooked it, but where?

  • AES-CBC importKey doesn't even mention [[type]]: https://w3c.github.io/webcrypto/#aes-cbc-operations
  • Most other algorithms (e.g. RSASSA-PKCS1-v1_5 importKey, rsa-pss importKey, ecdsa-operations importKey, RSA-OAEP importKey, HKDF importKey etc.) do it. This makes me think that AES-* importKey should also set its [[type]].
  • The SubtleCrypto.importKey interface simply reads from the CryptoKey.[[type]] slot, and never writes to it: https://w3c.github.io/webcrypto/#SubtleCrypto-method-importKey
  • It seems like most implementations set type to "secret", which seems reasonable.

I suggest to add a step like Set the [[type]] internal slot of key to "secret". to each AES algorithm, and also HMAC.

Or maybe I just missed a very important page? :-)

EDIT: Originally reported together with some false-positive issues: #373.

@twiss
Copy link
Member

twiss commented Oct 25, 2024

Yeah, it seems this text is missing :) Not just in the import key operations, but also in the generate key operations, btw.

Do you feel like making a PR for this as well? :) (If not I can also do it.)

@BenWiederhake
Copy link
Collaborator Author

I'd be happy to! Since this is definitely not just "editorial", I just applied for Passierschein A38 got a w3 account. Let's see if it really works that easily.

@twiss
Copy link
Member

twiss commented Oct 25, 2024

I think it is editorial since it's not intended to change implementations' existing behavior. You could also argue this behavior is described by the text saying that "Opaque keying material, including that used for symmetric algorithms, is represented by secret". (That's not to say that it shouldn't also be added to the individual operations, of course.)

But yes, getting a W3 account might make it easier to contribute in general :)

@BenWiederhake
Copy link
Collaborator Author

BenWiederhake commented Oct 25, 2024

I feel like this should add WPT tests, since this might be a new requirement, because technically the [[type]] wasn't defined before. So it is imaginable that a hypothetical browser might initialize the AES key as type "private" or "public", even though it's not supposed to be that way. (Hence the need for tests, to ensure interop.)

What do you think?

EDIT: Turns out, this is already tested: https://github.com/web-platform-tests/wpt/blob/master/WebCryptoAPI/generateKey/successes.js#L70

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

Successfully merging a pull request may close this issue.

2 participants