[FFmpeg-devel] [PATCH] avformat/hlsenc: added HLS encryption

Michael Niedermayer michaelni at gmx.at
Thu Nov 27 04:50:56 CET 2014


On Wed, Nov 26, 2014 at 04:37:51PM -0600, Christian Suloway wrote:
> Signed-off-by: Christian Suloway <csuloway at globaleagleent.com>
> ---

>  libavformat/crypto.c | 233 ++++++++++++++++++++++++++++---
>  libavformat/hlsenc.c | 387 ++++++++++++++++++++++++++++++++++++++++++++++-----

these should be split in 2 or more patches
also each should contain a verbose commit messsage and each should be
atomic and self contained not mix unrelated changes


>  2 files changed, 567 insertions(+), 53 deletions(-)
> 
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index a9b6e47..dc3dc88 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -38,17 +38,35 @@ typedef struct {
>      int indata, indata_used, outdata;
>      int eof;
>      uint8_t *key;
> -    int keylen;
> +    int key_len;
>      uint8_t *iv;
> -    int ivlen;
> +    int iv_len;

cosmetic changes should be seperate from functional ones
that is, in a seperate patch


> -    struct AVAES *aes;
> +    uint8_t *decrypt_key;
> +    int decrypt_key_len;
> +    uint8_t *decrypt_iv;
> +    int decrypt_iv_len;
> +    uint8_t *encrypt_key;
> +    int encrypt_key_len;
> +    uint8_t *encrypt_iv;
> +    int encrypt_iv_len;
> +    struct AVAES *aes_decrypt;
> +    struct AVAES *aes_encrypt;
> +
> +    uint8_t pad[BLOCKSIZE];
> +    int pad_len;
> +
>  } CryptoContext;
>  
>  #define OFFSET(x) offsetof(CryptoContext, x)
>  #define D AV_OPT_FLAG_DECODING_PARAM
> +#define E AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> -    {"key", "AES decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, .flags = D },
> -    {"iv",  "AES decryption initialization vector", OFFSET(iv), AV_OPT_TYPE_BINARY, .flags = D },
> +    {"key", "AES encryption/decryption key",                   OFFSET(key),         AV_OPT_TYPE_BINARY, .flags = D|E },
> +    {"iv",  "AES encryption/decryption initialization vector", OFFSET(iv),          AV_OPT_TYPE_BINARY, .flags = D|E },
> +    {"decryption_key", "AES decryption key",                   OFFSET(decrypt_key), AV_OPT_TYPE_BINARY, .flags = D },
> +    {"decryption_iv",  "AES decryption initialization vector", OFFSET(decrypt_iv),  AV_OPT_TYPE_BINARY, .flags = D },
> +    {"encryption_key", "AES encryption key",                   OFFSET(encrypt_key), AV_OPT_TYPE_BINARY, .flags = E },
> +    {"encryption_iv",  "AES encryption initialization vector", OFFSET(encrypt_iv),  AV_OPT_TYPE_BINARY, .flags = E },
>      { NULL }
>  };
>  
> @@ -72,28 +90,145 @@ static int crypto_open2(URLContext *h, const char *uri, int flags, AVDictionary
>          goto err;
>      }
>  
> -    if (c->keylen < BLOCKSIZE || c->ivlen < BLOCKSIZE) {
> -        av_log(h, AV_LOG_ERROR, "Key or IV not set\n");
> -        ret = AVERROR(EINVAL);
> -        goto err;
> +    if (flags & AVIO_FLAG_READ) {

> +        if (!c->decrypt_key_len) {
> +            if (!c->key_len) {
> +                av_log(h, AV_LOG_ERROR, "decryption key not set\n");
> +                ret = AVERROR(EINVAL);
> +                goto err;
> +            } else if (c->key_len != BLOCKSIZE) {
> +                av_log(h, AV_LOG_ERROR, "invalid key size "
> +                                        "(%d bytes, block size is %d)\n",
> +                                        c->key_len, BLOCKSIZE);
> +                ret = AVERROR(EINVAL);
> +                goto err;
> +            }
> +            c->decrypt_key = av_malloc(c->key_len);
> +            if (!c->decrypt_key) {
> +                ret = AVERROR(ENOMEM);
> +                goto err;
> +            }
> +            memcpy(c->decrypt_key, c->key, c->key_len);
> +            c->decrypt_key_len = c->key_len;
> +        } else if (c->decrypt_key_len != BLOCKSIZE) {
> +            av_log(h, AV_LOG_ERROR, "invalid decryption key size "
> +                                    "(%d bytes, block size is %d)\n",
> +                                    c->decrypt_key_len, BLOCKSIZE);
> +            ret = AVERROR(EINVAL);
> +            goto err;
> +        }
> +        if (!c->decrypt_iv_len) {
> +            if (!c->iv_len) {
> +                av_log(h, AV_LOG_ERROR, "decryption IV not set\n");
> +                ret = AVERROR(EINVAL);
> +                goto err;
> +            } else if (c->iv_len != BLOCKSIZE) {
> +                av_log(h, AV_LOG_ERROR, "invalid IV size "
> +                                        "(%d bytes, block size is %d)\n",
> +                                        c->iv_len, BLOCKSIZE);
> +                ret = AVERROR(EINVAL);
> +                goto err;
> +            }
> +            c->decrypt_iv = av_malloc(c->iv_len);
> +            if (!c->decrypt_iv) {
> +                ret = AVERROR(ENOMEM);
> +                goto err;
> +            }
> +            memcpy(c->decrypt_iv, c->iv, c->iv_len);
> +            c->decrypt_iv_len = c->iv_len;
> +        } else if (c->decrypt_iv_len != BLOCKSIZE) {
> +            av_log(h, AV_LOG_ERROR, "invalid decryption IV size "
> +                                    "(%d bytes, block size is %d)\n",
> +                                    c->decrypt_iv_len, BLOCKSIZE);
> +            ret = AVERROR(EINVAL);
> +            goto err;
> +        }

that looks like the same code twice with different variables
this should be factored into a seperate function
also there are more occurances of similar looking code


[...]
> +static int hex_string(char **string, uint8_t *buf, int size)
> +{
> +    int i;
> +
> +    *string = av_mallocz(size*2 + 1);
> +    if (!*string)
> +        return AVERROR(ENOMEM);

> +    for (i = 0; i < size; i++)
> +        snprintf(&(*string)[i*2], 3, "%02X", buf[i]);

the len parameter of snprintf() is intended to be the remaining
length of the array. setting that to a constant like that can
be dangerous

also for large enough size, size*2+1 will overflow

also theres ff_data_to_hex()


> +
> +    return 0;
> +}
> +
> +

> +static void hls_free_segment(HLSSegment *en)
> +{
> +    av_free(en->filename);
> +    av_free(en->key_uri);
> +    av_free(en->iv_string);
> +    av_free(en);

these should be av_freep()

[...]
> +    if ((ret = hls_encryption_init(s) < 0))

the () are wrong


[...]
>  #define OFFSET(x) offsetof(HLSContext, x)
>  #define E AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> -    {"start_number",  "set first number in the sequence",        OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0},     0, INT64_MAX, E},
> -    {"hls_time",      "set segment length in seconds",           OFFSET(time),    AV_OPT_TYPE_FLOAT,  {.dbl = 2},     0, FLT_MAX, E},
> -    {"hls_list_size", "set maximum number of playlist entries",  OFFSET(max_nb_segments),    AV_OPT_TYPE_INT,    {.i64 = 5},     0, INT_MAX, E},
> -    {"hls_ts_options","set hls mpegts list of options for the container format used for hls", OFFSET(format_options_str), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
> -    {"hls_wrap",      "set number after which the index wraps",  OFFSET(wrap),    AV_OPT_TYPE_INT,    {.i64 = 0},     0, INT_MAX, E},
> -    {"hls_allow_cache", "explicitly set whether the client MAY (1) or MUST NOT (0) cache media segments", OFFSET(allowcache), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, E},
> -    {"hls_base_url",  "url to prepend to each playlist entry",   OFFSET(baseurl), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,       E},
> -    {"hls_flags",     "set flags affecting HLS playlist and media file generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E, "flags"},
> -    {"single_file",   "generate a single media file indexed with byte ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E, "flags"},
> -
> +    {"start_number",         "set first number in the sequence",                                               OFFSET(start_sequence),     AV_OPT_TYPE_INT64,  {.i64 = 0},               0,       INT64_MAX, E},
> +    {"hls_time",             "set segment length in seconds",                                                  OFFSET(time),               AV_OPT_TYPE_FLOAT,  {.dbl = 2},               0,       FLT_MAX,   E},
> +    {"hls_list_size",        "set maximum number of playlist entries",                                         OFFSET(max_nb_segments),    AV_OPT_TYPE_INT,    {.i64 = 5},               0,       INT_MAX,   E},
> +    {"hls_ts_options",       "set hls mpegts list of options for the container format used for hls",           OFFSET(format_options_str), AV_OPT_TYPE_STRING, {.str = NULL},            0,       0,         E},
> +    {"hls_wrap",             "set number after which the index wraps",                                         OFFSET(wrap),               AV_OPT_TYPE_INT,    {.i64 = 0},               0,       INT_MAX,   E},
> +    {"hls_delete",           "delete segment files that are no longer part of the playlist",                   OFFSET(delete_segments),    AV_OPT_TYPE_INT,    {.i64 = 0},               0,       1,         E},
> +    {"hls_allow_cache",      "explicitly set whether the client MAY (1) or MUST NOT (0) cache media segments", OFFSET(allowcache),         AV_OPT_TYPE_INT,    {.i64 = -1},              INT_MIN, INT_MAX,   E},
> +    {"hls_base_url",         "url to prepend to each playlist entry",                                          OFFSET(baseurl),            AV_OPT_TYPE_STRING, {.str = NULL},            0,       0,         E},
> +    {"hls_flags",            "set flags affecting HLS playlist and media file generation",                     OFFSET(flags),              AV_OPT_TYPE_FLAGS,  {.i64 = 0},               0,       UINT_MAX,  E, "flags"},
> +    {"single_file",          "generate a single media file indexed with byte ranges",                          0,                          AV_OPT_TYPE_CONST,  {.i64 = HLS_SINGLE_FILE}, 0,       UINT_MAX,  E, "flags"},
> +    {"hls_segment_filename", "filename template for segment files",                                            OFFSET(segment_filename),   AV_OPT_TYPE_STRING, {.str = NULL},            0,       0,         E},
> +    {"hls_key_info_file",    "file with key URL and key file path",                                            OFFSET(key_info_file),      AV_OPT_TYPE_STRING, {.str = NULL},            0,       0,         E},
> +    {"hls_random_iv",        "randomize initialization vector",                                                OFFSET(random_iv),          AV_OPT_TYPE_INT,    {.i64 = 0},               0,       1,         E},
>      { NULL },
>  };

this mixes functional and cosmetic changes


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141127/38359561/attachment.asc>


More information about the ffmpeg-devel mailing list