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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: rbarnes, Assigned: rbarnes)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

No description provided.
Blocks: web-crypto
Assignee: nobody → rlb
Attached patch webcrypto-998804.patch (obsolete) (deleted) — Splinter Review
Note: Assumes Bug 995385
Attachment #8409519 - Flags: review?(dkeeler)
Attachment #8409519 - Flags: review?(bzbarsky)
Attached patch webcrypto-998804.patch (obsolete) (deleted) — Splinter Review
Attachment #8409965 - Flags: review?(bzbarsky)
Attached patch webcrypto-998804.patch (obsolete) (deleted) — Splinter Review
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 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+
Attached patch webcrypto-998804.1.patch (obsolete) (deleted) — Splinter Review
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 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+
* Addressed nits in Comment 6 and Comment 7 * Added a test with an unknown algorithm name
Attachment #8416525 - Attachment is obsolete: true
Depends on: 995385
Keywords: checkin-needed
Keywords: checkin-needed
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.

Attachment

General

Created:
Updated:
Size: