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.

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

AES encryption keys are requested from the server for every segment #140

Closed
ghost opened this issue Jul 2, 2018 · 5 comments
Closed

AES encryption keys are requested from the server for every segment #140

ghost opened this issue Jul 2, 2018 · 5 comments
Labels

Comments

@ghost
Copy link

ghost commented Jul 2, 2018

benwilber commented on Mar 31, 2018, 10:57 PM UTC:

Description

I expect the AES encryption key to be fetched from the server (at most) once, but instead it is fetched for every single segment in the playlist.

Given the following playlist:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-TARGETDURATION:7
#EXT-X-KEY:METHOD=AES-128,URI="/keys/foostream/1522536482595.key",IV=0x0000000000000000000001627E3F0723
#EXTINF:6.848,
1522536482595.ts
#EXTINF:6.000,
1522536489415.ts
#EXTINF:6.600,
1522536495457.ts
#EXTINF:6.167,
1522536502014.ts
#EXTINF:6.000,
1522536508181.ts
#EXTINF:6.000,
1522536514179.ts
#EXTINF:6.000,
1522536520184.ts
#EXTINF:6.000,
1522536526177.ts
#EXTINF:6.000,
1522536532179.ts
#EXTINF:6.500,
1522536538221.ts

The HLS specification indicates that the same key should be used for every segment following an EXT-X-KEY tag, until the next EXT-X-KEY. So then it is not needed to re-fetch the same key over and over again for every segment. Especially since acquiring the AES encryption key might be an expensive operation that involves verifying viewer access with database lookups, etc.

EDIT:

I see there is a patch for this here: #776

I think deferring to HTTP caching is incorrect in this case and against the HLS specification:

Media segments MAY be encrypted.  The EXT-X-KEY tag specifies how to
   decrypt them.  It applies to every media segment that appears between
   it and the next EXT-X-KEY tag in the Playlist file with the same
   KEYFORMAT attribute

The URL in the URI value of an EXT-X-KEY tag may or may not be cache-able upstream -- which is the whole point of defining the interval of segments encrypted per key. If the player just fetches the key for every single segment then it defeats the whole point of encrypting multiple segments with the same key.

EDIT EDIT:

I can't find a single other HLS media player that re-fetches the encryption keys for every segment. I've tested Clappr player, Flow player, JW Player, ffplay, QuickTime, and VLC. It appears this is behavior unique to this Videojs HLS plugin.

This issue was moved by forbesjo from videojs/videojs-contrib-hls#1395.

@ghost
Copy link
Author

ghost commented Jul 2, 2018

benwilber commented on Apr 4, 2018, 2:49 AM UTC:

I'm happy to submit a PR to correct this issue, I just want confirmation that it would be welcome since it looks like the last PR that addressed this issue was declined in favor of deferring to the browser cache (which is incorrect)

@ghost
Copy link
Author

ghost commented Jul 2, 2018

blacktrash commented on May 20, 2018, 9:06 AM UTC:

It's a showstopper for me. videojs-http-streaming unfortunately has the same issue.

@ghost
Copy link

ghost commented Oct 2, 2018

What does a closed bug issue mean? benwilber wants to submit a PR. We are developing a workaround also. Subscribing to this issue, ever hopeful.

@forbesjo
Copy link
Contributor

forbesjo commented Oct 2, 2018

We used the move bot to move the issue from videojs-contrib-hls to videojs-http-streaming. Ideally all bugfixes should go into videojs-http-streaming unless it is absolutely necessary to update videojs-contrib-hls

@gkatsev
Copy link
Member

gkatsev commented Apr 1, 2019

We have a fix for this in master. After spending some time with it, we do think that a proper cache-control solution is the preferred (something like cache-control: max-age={{duration*2}}; private, we realize that not everyone can set it, so, we added an option for it, which is disabled by default to maintain backwards compatibility.

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

No branches or pull requests

2 participants