wbamberg commented 5 years ago

Improve the Web Crypto API documentation

This is based on https://gist.github.com/wbamberg/66ae0a5de404df381b3fe58aa9e335f8.

Acceptance criteria

All items listed below are complete.

Tasks

- [x] Write Web Crypto API examples
- [x] Rewrite "supported algorithms" page
- [x] Write dictionary pages for algorithm objects
- [x] Update all method docs
- [x] Update BCD
- [x] Write missing pages
- [x] Update main interface page
- [ ] Write "guidance" page

wbamberg commented 5 years ago

I thought before I break I'd update you with where we are here.

I think all the pages linked below are now ready for review.

Function pages

I've substantially rewritten the following function pages:

For all functions that implement actual crypto functions (as opposed to key gen/key import) I've got a "supported algorithms" section that lists the supported algorithms for that function with a bit of introductory info on them and pointers to more details.

I've added lots of examples, taken from the PRs I've submitted to https://github.com/mdn/dom-examples. Not all the links to these examples will work yet, because not all the PRs are merged.

Dictionaries

The Web Crypto API uses lots of dictionary objects for algorithm-specific parameters. Following https://discourse.mozilla.org/t/subtlecrypto-and-documenting-dictionaries/33625 I've documented these as separate pages. Sometimes they are quite thin, sometimes they contain important guidelines for selecting good parameter values, as Web Crypto is a real footgun API.

I haven't yet filled in "Examples", "Browser Compatibility" or "See Also" sections, and tbh I'm inclined to omit these sections from these pages, as the pages that link to them (e.g. sign()) will have duplicate info.

Glossary articles

I've written a few glossary articles for bits of jargon:

https://developer.mozilla.org/en-US/docs/Glossary/Public-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Symmetric-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Block_cipher_mode_of_operation

We could perhaps use a few more, for things like salt and AES and dictionary attacks.

Examples

I've filed a few PRs for examples:

merged:
- sign/verify: https://github.com/mdn/dom-examples/tree/master/web-crypto/sign-verify
in review:
- encrypt/decrypt: https://github.com/mdn/dom-examples/pull/34
- deriveKey: https://github.com/mdn/dom-examples/pull/35
- importKey: https://github.com/mdn/dom-examples/pull/36

What's left?

Apart from reviews of all this stuff, I still need to:

add examples for exportKey, wrapKey, unwrapKey, and update the corresponding pages
write missing pages for CryptoKeyPair and deriveBits
update the main interface pages to have a bit of an overview of the API, warn people never to use it, and point to some useful deeper docs like https://www.crypto101.io/.

rugk commented 5 years ago

Great! I looked at some pages, and IMHO, you can further stress the need why authenticated encryption (AEADs) are better than CBC or so. Recent efail problem has highlighted this again and I totally recommend reading that blog by @hannob.

On the sign doc I'd argue that HMAC is presented a little bit too scary. I mean, I guess it's the APIs fault here, because HMAC is more like "Hashing on steriods", i.e. hashing with additional authentication, as it obviously uses a usual "hash algo" inside of it, i.e. MD5, SHA-1, etc. Note: You also do not mention what is used internally, i.e. HMAC-MD5, HMAC-SHA1 or what? (Note that due to the construction of a HMAC even HMAC-MD5 is still secure even as MD5 is not collision resistant.) This is very good to know, when you want to re-implement (a web application) in another programming language.

As such, HMAC is IMHO more fitting for the digest API, but well… I see why it is here. But what matters is: These are totally different use cases.

a signature: Alice wants to verify the message bob sent is authentic (+ integrity).
a HMAC: Alice gives a challenge (text) to Bob and Bob creates a HMAC of it to verify it has a secret key (for password verification without leaking the secret, i.e. the password e.g.)

You see these are used in totally different ways? As such, it is just a different use case.

Thus the HMAC example should have, IMHO, boxes labelled "hash" and "verify [hash]". I mean it should make it clear that the "verify" function also just re-calculates the HMAC and then compares it (in a time-safe way, I guess/hope) to the original one. It should also be clear that HMAC is just a simple thing, while all other signature functions are assymetric crypto. As Wikipedia says it involves "a cryptographic hash function and a secret cryptographic key.", but these are just hashes (MD5, SHA-1, SHA-256 etc.) and a key, there is no symmetric or asymmetric encryption.

So to summarize, IMHO:

[ ] highlight/recommend AEADs more for decryption/encryption or add warnings to the others
[ ] what does HMAC internally use? Is it HMAC-MD5, HMAC-SHA1 or what?
[ ] HMAC should be presented as an alternative to hashes, not really as an alternative to signatures.
[ ] Referring to the previous point, I'd e.g. say the buttons for the HMAC example here should be labelled "hash" and "verify [hash]". I.e. generally avoid the wording "sign" in regards to HMAC, it's more of just "hashing".

wbamberg commented 5 years ago

Thanks for your comments @rugk! I'm still out for the next few days, but tl;dr: I think your comments are reasonable but would also like to hear from the Mozilla security people before deciding on what to change exactly.

highlight/recommend AEADs more for decryption/encryption or add warnings to the others

I think it's a tricky question how far to go with recommendations versus just presenting facts. The concern I think is that it's very hard to use a low-level API like this securely unless you have a lot of knowledge, and presenting some normative guidelines might make give people, well, a false sense of security. Another way to put this is: the docs can't teach you everything about designing secure systems using low-level primitives, so if you need the docs to tell you when/why you should use authenticated encryption, you should not be using the API.

But that said, it's a judgement call about what to include and what not, and I'd be happy to make this change if the Mozilla security people agree.

what does HMAC internally use? Is it HMAC-MD5, HMAC-SHA1 or what?

It's set in HmacKeyGenParams, but I should probably link to that from https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#HMAC.

HMAC should be presented as an alternative to hashes, not really as an alternative to signatures. These are totally different use cases.

a signature: Alice wants to verify the message bob sent is authentic (+ integrity).

a HMAC: Alice gives a challenge (text) to Bob and Bob creates a HMAC of it to verify it has a secret key (for password verification without leaking the secret, i.e. the password e.g.)

As I understand it this is questionable. The use case you've given for HMAC is not the only one, for example https://timtaubert.de/talks/keeping-secrets-with-javascript/ presents a use case much more like a signature. I think of an HMAC as very different from a hash - an HMAC is a MAC, that just happens to be implemented using a hash. https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#Supported_algorithms does explain a bit that HMAC is quite different from the others, but perhaps this could be made stronger.

Again though I'd be happy to hear what the security folk have to say about this.

rugk commented 5 years ago

Certainly HMAC is different from a hash, but it is also very different from a (RSA/ECDSA). That's my whole point. And yes, there likely exist other use cases, my one was just an common example to demonstrate that there are really different (and valid) use-cases. Looking through the Iinked talk I also just see HMAC used for password verification and thus then deciding whether to load/show a note or discard it. But possibly you need to link to the exact slide, so I can see what you mean.

wbamberg commented 5 years ago

But possibly you need to link to the exact slide, so I can see what you mean.

Sorry, I could have been clearer. I don't know how to link to a slide, but in the video he starts talking about using HMAC for message authentication at 7.44. But I think we might be talking past each other at this point. I do think it would be quite misleading to talk about HMAC as if it is "like a hash" so I've left this for now. Honestly though: people who know what an HMAC is will know, and people who don't will have to learn.

I have made the stuff about AEAD in https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/encrypt#Supported_algorithms a bit stronger, and added a pointer in https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#HMAC on where to set the hash function.

wbamberg commented 5 years ago

Update: I've now finished updating all the method pages as well as the top-level SubtleCrypto page. In all the following pages have been written/updated:

Top-level page

https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto

Method pages

Dictionaries

Glossary entries

https://developer.mozilla.org/en-US/docs/Glossary/Public-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Symmetric-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Block_cipher_mode_of_operation

rugk commented 5 years ago

I have made the stuff about AEAD in https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/encrypt#Supported_algorithms a bit stronger

Yeah, great! For formatting and highlighting the warning a little more, maybe one can use the "note" or even "warning" block though?

chrisdavidmills commented 5 years ago

@wbamberg I have reviewed the following pages:

I think it is OK to have the "big scary warning" from the SubtleCrypto page just on the main landing page and interface page, rather than everymethod page.

I have mainly concentrated on making all the pages more consistent, and making sure the text all makes sense as much as it can (this is a complex subject, but there were a few bits I managed to improve the explanation of). If you notice any general changes that you disagree with, let me know. I also updated promise desciptions to use the word "fulfilled", which is the technical correct term for resolving successfully.

I had a couple of queries for you:

Some of the syntax blocks use var, and some use const. Is there any reason not to just make them all use const for consistency?
The wrapKey page doesn't have an "Examples" section.

I also identified a few terms that feel like they could do with glossary entries (the crypto stuff does have quite a bit of jargon):

signature
pseudo-random bits
extractability
asymmetric/symmetric keys/algorithms
collision resistance

Up to you; I'm not that religious about having them added. More like suggestions really.

OK, I'll do more next week!

wbamberg commented 5 years ago

Thanks @chrisdavidmills !

Some of the syntax blocks use var, and some use const. Is there any reason not to just make them all use const for consistency?

Done!

The wrapKey page doesn't have an "Examples" section.

Doh! I forgot to copy these out of the dom-examples repo. Should be good now.

signature pseudo-random bits extractability asymmetric/symmetric keys/algorithms collision resistance

Some of these already have entries, although I'm not particularly happy with all of them:

https://developer.mozilla.org/en-US/docs/Glossary/Signature/Security https://developer.mozilla.org/en-US/docs/Glossary/Symmetric-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Public-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/RNG

I think "extractability" is hard to conceive of as real jargon, but collision-resistance, as well as things like salt and dictionary attacks, are definitely valid examples. I'll have a think about those.

We should probably link to all these jargon terms somewhere?

chrisdavidmills commented 5 years ago

I've now reviewed the dictionary pages:

Mostly fine; just a few points:

The CryptoKey page has separate subpages for the properties, which is a bit odd. the others don't seem to.
The CryptoKey and CryptoKeyPair pages didn't have an Examples section. I've filled one in, but it'd be nice to have a link to when examples are shown, like with the other pages.
https://developer.mozilla.org/en-US/docs/Web/API/EcdsaParams: "but the SHA-1 algorithm should no longer be used." It might be nice to say why.
Most of these don't have BCD yet — I'm assuming this is just waiting to be put into production?
https://developer.mozilla.org/en-US/docs/Web/API/Pbkdf2Params — a couple of points here:
- The Specifications and Browser compatibility sections seem to reference the wrong feature of the API?
- It says that the hash property can be set to SHA-1 — but other pages say that using this is not recommended. Should this say the same?
https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedImportParams — does the Browser compatibility section refer to the wrong feature here?
https://developer.mozilla.org/en-US/docs/Web/API/EcKeyImportParams - again, does the {{compat}} call refer to the wrong feature?
Same comment for this page too.
I just thought — the sidebar is not updated the the latest pages, etc. Have you worked towards this?

chrisdavidmills commented 5 years ago

I reviewed the glossary entries too:

https://developer.mozilla.org/en-US/docs/Glossary/Public-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Symmetric-key_cryptography https://developer.mozilla.org/en-US/docs/Glossary/Block_cipher_mode_of_operation

A couple of comments:

In the Public-key cryptography page you also mention asymmetric cryptography as a synonym; have you created a page for that to, to point to this page?
Do we have glossary entries for private key and public key?
We could also do with entries for RSA, DSA, and Diffie-Hellman

wbamberg commented 5 years ago

Thanks @chrisdavidmills !

The CryptoKey page has separate subpages for the properties, which is a bit odd. the others don't seem to.

I slurped the subpages into the main page.

The CryptoKey and CryptoKeyPair pages didn't have an Examples section. I've filled one in, but it'd be nice to have a link to when examples are shown, like with the other pages.

Done, although just as links to the method pages.

https://developer.mozilla.org/en-US/docs/Web/API/EcdsaParams: "but the SHA-1 algorithm should no longer be used." It might be nice to say why.

I have added a little on this but didn't go into much detail.

Most of these don't have BCD yet — I'm assuming this is just waiting to be put into production?

I haven't yet done the BCD updates. I need to think about how best to represent this. For instance it's not obvious to me that having separate BCD for all the parameter objects is useful to people.

https://developer.mozilla.org/en-US/docs/Web/API/Pbkdf2Params — a couple of points here:

The Specifications and Browser compatibility sections seem to reference the wrong feature of the API?

Fixed, but see above for BCD.

It says that the hash property can be set to SHA-1 — but other pages say that using this is not recommended. Should this say the same?

AIUI this particular usage of SHA-1 is OK, but people should just stop using it. I have tried to clarify this.

https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedImportParams — does the Browser compatibility section refer to the wrong feature here?

https://developer.mozilla.org/en-US/docs/Web/API/EcKeyImportParams - again, does the {{compat}} call refer to the wrong feature?

Same comment for this page too.

See above for BCD.

I just thought — the sidebar is not updated the the latest pages, etc. Have you worked towards this?

Ugh. Can you help me understand what I have to do for this? I don't understand the whole APIRef thing.

About glossary entries: since this is a new task, I'd suggest filing a new issue for it so we can scope it properly, rather than bolting it onto this as some amorphous mass.

chrisdavidmills commented 5 years ago

Most of this sounds fine. I had a few further comments to clear up remaining bits in question.

I haven't yet done the BCD updates. I need to think about how best to represent this. For instance it's not obvious to me that having separate BCD for all the parameter objects is useful to people.

This is true — you are not going to have a method implemented that requires/returns a dictionary object, but then not support the dictionary object. And even if you did, you'd be more likely to mention this on the method page. I'd just point to the support data of the relevant methods that use these dictionaries?

Ugh. Can you help me understand what I have to do for this? I don't understand the whole APIRef thing.

As far as I understand it, the modern api to use is {{defaultapisidebar}}. You have to call it with the API name passed as an argument, i.e. {{defaultapisidebar("Web Crypto API")}}, and the links to generate are taken from the GroupData entry for that API:

https://github.com/mdn/kumascript/blob/master/macros/GroupData.json#L1933

And because of the work sheppy did, you can now put dictionaries, etc., in their own separate lists so they are not lumped together and all listed as interfaces. See the WebRTC entry for an example:

https://github.com/mdn/kumascript/blob/master/macros/GroupData.json#L2027

One warning — if guides are included as subpages below the landing page and given the tag "Guide" (I think it's "Guide"), they will appear automatically in the sidebar under the Guides list. If you put an entry for them in the GroupData, they will appear twice.

ExE-Boss commented 5 years ago

{{DefaultAPISidebar(…)}} is for overview, guide and miscellaneous pages, whereas {{APIRef(…)}} is only for interface pages and subpages.

wbamberg commented 5 years ago

if guides are included as subpages below the landing page and given the tag "Guide" (I think it's "Guide"), they will appear automatically in the sidebar under the Guides list. If you put an entry for them in the GroupData, they will appear twice.

It appears as if they don't need the tag: https://github.com/mdn/kumascript/blob/master/macros/DefaultAPISidebar.ejs#L41-L45

chrisdavidmills commented 5 years ago

Ah, ok ;-)

wbamberg commented 5 years ago

I'm closing this: the only item not addressed is the separate "guidance" page. As explained in https://github.com/mdn/sprints/issues/672, I think this needs the active collaboration of security experts to determine even the general scope of this guide, so it's not a thing I can move forward on my own.

Note that there is now some guidance on basic issues (e..g. not using SHA-1, not reusing IVs, key length recommendations) inline in the docs.

mdn / sprints

Improve Web Crypto docs #664

Improve the Web Crypto API documentation

Acceptance criteria

Tasks

Function pages

Dictionaries

Glossary articles

Examples

What's left?

Top-level page

Method pages

Dictionaries

Glossary entries