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.

Sign up for GitHub

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

Use WebCrypto by default for AES decryption #6015

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Use WebCrypto by default for AES decryption #6015

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/crypt/decrypter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ export default class Decrypter {
private currentIV: ArrayBuffer | null = null;
private currentResult: ArrayBuffer | null = null;
private useSoftware: boolean;
private enableSoftwareAES: boolean;

constructor(config: HlsConfig, { removePKCS7Padding = true } = {}) {
this.useSoftware = config.enableSoftwareAES;
this.enableSoftwareAES = config.enableSoftwareAES;
this.removePKCS7Padding = removePKCS7Padding;
// built in decryptor expects PKCS7 padding
if (removePKCS7Padding) {
Copy link

@jvary jvary Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In src/demux/sample-aes.ts, 'removePKCS7Padding' is set to false in SampleAesDecrypter constructor..
If I follow the code correctly (in your PR and previously), that will always ends-up in software because 'this.subtle' is never set in Decrypter constructor with removePKCS7Padding===false, and then useSoftware=true
Do I follow the code correctly, and if yes, is that the intended behavior ?

Copy link
Collaborator Author

@robwalch robwalch Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's correct. HLS.js uses software decryption in order to skip removal of PKCS7 padding for clear SAMPLE-AES MPEG-TS.

Support for SAMPLE-AES in MPEG-TS "identity" format segments is maintained with no plans to expand on it in JavaScript. It would probably be better handled by EME with ClearKey support. The PR for that fell by the way side, but it would be great if someone wanted to pick it up:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With this PR, WebCrypto will be used by default for AES-128, but not for SAMPLE-AES in MPEG-TS (with no-CDM).

Expand All @@ -36,9 +37,7 @@ export default class Decrypter {
/* no-op */
}
}
if (this.subtle === null) {
this.useSoftware = true;
}
this.useSoftware = this.subtle === null;
}

destroy() {
Expand Down Expand Up @@ -174,14 +173,21 @@ export default class Decrypter {
}

private onWebCryptoError(data, key, iv): ArrayBuffer | never {
this.useSoftware = true;
this.logEnabled = true;
this.softwareDecrypt(data, key, iv);
const decryptResult = this.flush();
if (decryptResult) {
return decryptResult.buffer;
const enableSoftwareAES = this.enableSoftwareAES;
if (enableSoftwareAES) {
this.useSoftware = true;
this.logEnabled = true;
this.softwareDecrypt(data, key, iv);
const decryptResult = this.flush();
if (decryptResult) {
return decryptResult.buffer;
}
}
throw new Error('WebCrypto and softwareDecrypt: failed to decrypt data');
throw new Error(
'WebCrypto' +
(enableSoftwareAES ? ' and softwareDecrypt' : '') +
': failed to decrypt data',
);
}

private getValidChunk(data: Uint8Array): Uint8Array {
Expand Down
Loading