Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding AES-256 and AES-256-CTR encryption methods #6018

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

jvary
Copy link

@jvary jvary commented Dec 6, 2023

Description

Adding AES-256 and AES-256-CTR encryption methods (full segment, as AES-128)

Context

Need stronger encryption scheme than AES-128 for CCTV scenario, in the short-mid term.

See more context & informal addendum to rfc8216 at :
https://mailarchive.ietf.org/arch/msg/hls-interest/RZBgatOvI4W0M0J5cShuXWzdigg/

Are there any points in the code the reviewer needs to double check?

  • I have very little experience in JavaScript/TypeScript, please assume that everything is wrong language wise :-)
  • Error handling
  • Everything crypto related

Limitations

  • Does not enforce the presence of explicit IV in AES-256 & AES-256-CTR (when the rfc informal proposal stated they SHALL be present, and different for each segment).
  • AES-256 & AES-256-CTR will only work with WebCrypto, not the JavaScript implementation.
  • Range access ‘adjustment’ is not implemented for CTR mode (don’t re-compute nonce counter based on block offset)

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md
    - Not an official change yet?

src/crypt/decrypter.ts Outdated Show resolved Hide resolved
@jvary
Copy link
Author

jvary commented Dec 6, 2023

Test playlist can be found here:

https://jvaryhlstests.blob.core.windows.net/hlstestdata/playlist_encrypted.m3u8
(can't commit it to integration test, because default still software decryption, which will not work)

@avelad
Copy link

avelad commented Jan 15, 2024

Note: This functionality has already been added to ShakaPlayer and will appear in version 5.0

robwalch
robwalch previously approved these changes Jan 15, 2024
@robwalch robwalch added this to the 1.6.0 milestone Jan 15, 2024
@robwalch
Copy link
Collaborator

(can't commit it to integration test, because default still software decryption, which will not work)

Please rebase once #6015 is merged.

@robwalch
Copy link
Collaborator

@jvary, #6015 has been merged. Thanks for waiting.

@jvary
Copy link
Author

jvary commented Jan 23, 2024

@jvary, #6015 has been merged. Thanks for waiting.

@robwalch : rebased and added playlist in integration.

image

@jvary jvary requested a review from robwalch January 23, 2024 19:23
@robwalch robwalch merged commit 55b4aac into video-dev:master Jan 24, 2024
12 checks passed
@jvary jvary deleted the Aes256 branch January 24, 2024 00:10
@jvary jvary restored the Aes256 branch January 24, 2024 00:11
@@ -262,4 +262,9 @@ module.exports = {
NAL units are not starting right at the beginning of the PES packet when using hardware accelerated decoding.`,
abr: false,
},
aes256: {
url: 'https://jvaryhlstests.blob.core.windows.net/hlstestdata/playlist_encrypted.m3u8',
Copy link

Choose a reason for hiding this comment

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

@robwalch thanks for adding this! Did you encode this video? I would love to know what program you used. The ffmpeg still does not support AES-256.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That was @jvary, the PR author.

Copy link
Author

Choose a reason for hiding this comment

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

@jcalfee : Hello! This is full segment encryption (not Common Encryption), so I just encrypted the whole file with OpenSSL.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants