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

Add examples for deriveBits #39

Merged
merged 2 commits into from
Jan 9, 2019

Conversation

wbamberg
Copy link
Contributor

@wbamberg wbamberg commented Jan 8, 2019

Examples for the deriveBits() API. This is like deriveKey(), except that deriveKey() also imports the bits as a CryptoKey, enabling you to actually use the derived key to do things like encryption.

So these examples are based on the examples for deriveKey(), except they're simpler because you can't actually use the derived bits to encrypt.

I think this is the last example we need :).

@wbamberg wbamberg changed the title Add deriveBits examples Add examples for deriveBits Jan 8, 2019
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, from a comprehension/editorial perspective, so I'm approving.

The only comment that came to mind was maybe you should link the mention of "deriveBits()" to the appropriate reference page on MDN, when it is available, and do the same for the other example pages?

@chrisdavidmills
Copy link
Contributor

Once @april has given this a review and approved, I think we can merge it.

@wbamberg
Copy link
Contributor Author

wbamberg commented Jan 8, 2019

maybe you should link the mention of "deriveBits()" to the appropriate reference page on MDN, when it is available

It's a good suggestion and I don't know why I didn't do this (and BTW deriveBits is available now, I wrote the page yesterday: https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/deriveBits). But it would apply to all the examples (e.g. https://github.com/mdn/dom-examples/blob/master/web-crypto/derive-key/index.html#L14), so it would be much easier to do this as a single follow-up PR, rather than amend every open PR with this change, so can we do that?

@chrisdavidmills
Copy link
Contributor

it would be much easier to do this as a single follow-up PR, rather than amend every open PR with this change, so can we do that?

Yup, absolutely.

@april
Copy link

april commented Jan 8, 2019

Overall it looks fine to me, other than to say that if we're deriving a 256-bit key in deriveKey() then we should probably do the same in deriveBits(). Currently ecdh.js is deriving a 128-bit key.

@wbamberg
Copy link
Contributor Author

wbamberg commented Jan 8, 2019

Thanks again for the reviews!

Currently ecdh.js is deriving a 128-bit key.

Again, I don't know why I did this :). I've updated the PR.

it would be much easier to do this as a single follow-up PR, rather than amend every open PR with this change, so can we do that?

Yup, absolutely.

I've filed #40 so we don't forget.

@chrisdavidmills
Copy link
Contributor

Cool, all looks good then. Merging.

@chrisdavidmills chrisdavidmills merged commit 60b72ef into mdn:master Jan 9, 2019
@@ -8,7 +8,7 @@
public: publicKey
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mdn:master

@@ -48,4 +48,5 @@
deriveBitsButton.addEventListener("click", () => {
getDerivedBits();
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

51

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

Successfully merging this pull request may close these issues.

None yet

4 participants