########################################################################################## HLS-21: Review Encrypt function ########################################################################################## Issue Type: Task ----------------------------------------------------------------------------------------- Issue Information ==================== Priority: Major Status: Closed Resolution: Done (2017-04-29 12:16:43) Project: HLS Stream Creator (HLS) Reported By: btasker Assigned To: btasker Components: - Encryption Targeted for fix in version: - 1.0 Time Estimate: 120 minutes Time Logged: 0 minutes ----------------------------------------------------------------------------------------- Issue Description ================== The ability to encrypt segments was introduced as a pull request in HLS-17 At the time I did a fairly cursory code review - scanned the patch, and ran against a video to check it worked. HLS-20 shows that this probably wasn't enough, so I want to take a close look at that function (including the HLS-20 changes) and make sure there aren't any other issues hiding. ----------------------------------------------------------------------------------------- Issue Relations ================ - relates to HLS-20: Reported issues with Encryption - relates to HLS-17: PR#7 Added AES-128 encryption to the generated files - Discovered HLS-22: IV will be wrong for some bitrates in ABR streams ----------------------------------------------------------------------------------------- Activity ========== ----------------------------------------------------------------------------------------- 2017-04-27 09:16:30 btasker ----------------------------------------------------------------------------------------- I've a feeling the HLS-20 changes still aren't quite up to scratch for adaptive bitrate streams. The for loop ( https://github.com/bentasker/HLS-Stream-Creator/blob/42de7b6cd9a344d6e57d2544f1b18376aaef7bc1/HLS-Stream-Creator.sh#L226 ) is written on the basis of there only being one bitrate, and so generates an IV based upon the segment number -- BEGIN SNIPPET -- count=0 for file in ${OUTPUT_DIRECTORY}/*.ts do ENC_FILENAME="$OUTPUT_DIRECTORY/${SEGMENT_PREFIX}_enc_${count}".ts INIT_VECTOR=$(printf '%032x' $count) -- END SNIPPET -- If we've got an ABR stream with 3 bitrates, containing 5 segments each, then when we move onto the second bitrate count is going to be at 6 (whilst we'll expect it to be 0). So the function almost certainly needs adapting to take bitrates into account rather than simply iterating over files. Apple's technote ( https://developer.apple.com/library/content/technotes/tn2288/_index.html ) specifies that -- BEGIN QUOTE -- The default Initialization Vector for media encryption (if none is specified) is the sequence number of the media file. You should specify an Initialization Vector value, and not rely on sequence numbers. The main reason for this is portability. For example, if you change where the segment appears in the playlist (e.g. inserting an ad), that changes its sequence number, requiring a re-encrypt. -- END QUOTE -- So this is almost certainly going to be an issue. Longer term, it'd be better to follow their advice and set a specific IV (which will mean rewriting the manifest), but in the short term need to make the sequence numbers work as a minimum Raised as HLS-22 ----------------------------------------------------------------------------------------- 2017-04-29 12:12:51 git ----------------------------------------------------------------------------------------- -- BEGIN QUOTE -- Repo: HLS-Stream-Creator Commit: 40dc4fca5c0364ea9887ebd3b70ac7be3b7aaf04 Author: B Tasker