Closed
Bug 998804
Opened 11 years ago
Closed 11 years ago
Add support for SHA-1 and SHA-2 digests to WebCrypto API
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: rbarnes, Assigned: rbarnes)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•11 years ago
|
Blocks: web-crypto
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → rlb
Assignee | ||
Comment 1•11 years ago
|
||
Note: Assumes Bug 995385
Attachment #8409519 -
Flags: review?(dkeeler)
Attachment #8409519 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #8409965 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #8409519 -
Attachment is obsolete: true
Attachment #8409965 -
Attachment is obsolete: true
Attachment #8409519 -
Flags: review?(dkeeler)
Attachment #8409519 -
Flags: review?(bzbarsky)
Attachment #8409965 -
Flags: review?(bzbarsky)
Attachment #8409967 -
Flags: review?(cviecco)
Attachment #8409967 -
Flags: review?(bzbarsky)
Comment 4•11 years ago
|
||
Comment on attachment 8409967 [details] [diff] [review]
webcrypto-998804.patch
Review of attachment 8409967 [details] [diff] [review]:
-----------------------------------------------------------------
Again only PSM layer
Attachment #8409967 -
Flags: review?(cviecco) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Minor update to match latest patch to Bug 998385.
Attachment #8409967 -
Attachment is obsolete: true
Attachment #8409967 -
Flags: review?(bzbarsky)
Attachment #8416525 -
Flags: review?(dkeeler)
Attachment #8416525 -
Flags: review?(bzbarsky)
Comment 6•11 years ago
|
||
Comment on attachment 8416525 [details] [diff] [review]
webcrypto-998804.1.patch
>+++ b/dom/crypto/WebCryptoTask.cpp
>+class SimpleDigestTask : public ReturnArrayBufferViewTask
>+ SimpleDigestTask(JSContext* aCx,
>+ const ObjectOrString& aAlgorithm,
>+ const CryptoOperationData& aData)
Please fix the weird indent.
>+ : mData(aData)
If this OOMs, shouldn't we fail the task instead of digesting empty data?
>+ virtual nsresult DoCrypto() MOZ_OVERRIDE
>+ mResult.SetLength(hashLen);
If this fails, we need to fail out, instead of overrunning the buffer.
r=me with those fixed.
Attachment #8416525 -
Flags: review?(bzbarsky) → review+
Comment on attachment 8416525 [details] [diff] [review]
webcrypto-998804.1.patch
Review of attachment 8416525 [details] [diff] [review]:
-----------------------------------------------------------------
NSS parts look good. r=me with nits addressed.
::: dom/crypto/WebCryptoTask.cpp
@@ +110,5 @@
> +{
> +public:
> + SimpleDigestTask(JSContext* aCx,
> + const ObjectOrString& aAlgorithm,
> + const CryptoOperationData& aData)
nit: indentation
::: dom/crypto/test/test-vectors.js
@@ +38,5 @@
>
> // Truncated form of the PKCS#8 stucture above
> negative_spki: util.hex2abv("30819F300D06092A864886F70D010101050003"),
> +
> + // From the NESSIE project
nit: trailing space
::: dom/crypto/test/tests.js
@@ +313,5 @@
> +TestArray.addTest(
> + "SHA-256 digest",
> + function() {
> + var that = this;
> + crypto.subtle.digest("SHA-256", tv.sha256.data).then(
How about a sha-1 test as well? In fact, I think it would be best to have a test for each algorithm we support. Also, is there an "unknown algorithm" test?
Attachment #8416525 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #8416525 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 9•11 years ago
|
||
Clean try run covering:
* Bug 1005375
* Bug 995385
* Bug 998804
https://tbpl.mozilla.org/?tree=Try&rev=9e24926f0fe8
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 10•11 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Comment 11•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•