Use webcrypto for aes-cbc segment decryption when supported #4
Conversation
| * @see http://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Block_Chaining_.28CBC.29 | ||
| * @see https://tools.ietf.org/html/rfc2315 | ||
| */ | ||
| export const decrypt = function(encrypted, key, iv, done) { |
There was a problem hiding this comment.
this is already being exported below.
| return decryptionMethod(encrypted, key, iv, done); | ||
| }; | ||
|
|
||
| export default decrypt; |
There was a problem hiding this comment.
This makes the API backwards incompatible, no? Though, I guess the key change above is also backwards incompatible anyway.
There was a problem hiding this comment.
Right, we need to update -contrib-hls and hls-fetcher anyway. I can revert to the old export default of an object with Decrypter and decrypt but this seemed cleaner.
There was a problem hiding this comment.
The Decrypter was an object because there is state that has to be regenerated per key-iv combo. Our usage of this project is very performance sensitive so I think the old API is justified.
There was a problem hiding this comment.
The original non-native Decrypter remains an object and preserves the state management for each key+iv tuple. Each segment gets its key and computes its iv for each construction/invocation of Decrypter/decrypt():
So the Decrypter object being constructed for each segment is consistent with the previous implementation, and it continues to maintain its state for block chaining, etc. But the operation is still decrypt(), and the inputs remain the same, and the output is a decrypted segment, which sounds very functional to me. The fact that the non-native impl. required state maintenance is an implementation detail that leaked into the API.
|
Note: since IE11's implementation of webcrypto is based on an old version of the spec that doesn't use Promises, and IE11 doesn't have native Promises either, this falls back to the original pure-JS decryption path in the presence of window.msCrypto. Edge is fully up to spec and does the right thing like Chrome, Firefox, etc. |
|
|
||
| let done = assert.async(); |
There was a problem hiding this comment.
For simple timeout stuff, our standard is to use sinon's clock.tick() instead of async tests.
There was a problem hiding this comment.
clock.tick() didn't seem happy with Promises, but I'll try again.
|
I don't believe the tests are exercising the non-native code path anymore (unless you're running them on IE10), and that's why the key-byte-order incompatibility isn't causing tests to fail. We need to support IE10 in this project for awhile so we should find a way to ensure that path is hit. |
| 0x4f, 0xae, 0x01, 0x1c, | ||
| 0x82, 0xa8, 0xf0, 0x67]); | ||
| /* eslint-disable no-unused-vars */ | ||
| let decrypter = new Decrypter(encrypted, |
There was a problem hiding this comment.
Decrypter is all non-native no matter the browser, so it's being exercised here.
|
@dmlap I have annotated the unit tests to show the non-native (original) |
dconnolly
force-pushed
the
webcrypto
branch
2 times, most recently
from
23a914f
to
5c66733
Compare
If webcrypto is supported, the spec requires Promises to be available as well, so we use Promises in that branch. Otherwise it is the same SJCL-based Decrypter implementation as before, just slightly reorganized.
|
We will have to make some usage updates to contrib-hls to pull this in but LGTM. |
If webcrypto is supported, the spec requires Promises to be available as well, so we use Promises in that branch. Otherwise it is the same SJCL-based Decrypter implementation as before, just slightly reorganized.