diff mbox series

[FFmpeg-devel] avformat/hls: add macros for iv and key size

Message ID 1620736947-4319-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/hls: add macros for iv and key size | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

Lance Wang May 11, 2021, 12:42 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/hls.c      | 12 ++++++------
 libavformat/hlsenc.c   | 19 +++++++++----------
 libavformat/internal.h |  2 ++
 3 files changed, 17 insertions(+), 16 deletions(-)

Comments

Nicolas George May 11, 2021, 12:44 p.m. UTC | #1 
lance.lmwang@gmail.com (12021-05-11):
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/hls.c      | 12 ++++++------
>  libavformat/hlsenc.c   | 19 +++++++++----------
>  libavformat/internal.h |  2 ++
>  3 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8fc6924..a215c6a 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -72,7 +72,7 @@ struct segment {
>      char *url;
>      char *key;
>      enum KeyType key_type;
> -    uint8_t iv[16];
> +    uint8_t iv[KEYSIZE];
>      /* associated Media Initialization Section, treated as a segment */
>      struct segment *init_section;
>  };
> @@ -131,7 +131,7 @@ struct playlist {
>      unsigned int init_sec_buf_read_offset;
>  
>      char key_url[MAX_URL_SIZE];
> -    uint8_t key[16];
> +    uint8_t key[KEYSIZE];
>  
>      /* ID3 timestamp handling (elementary audio streams have ID3 timestamps
>       * (and possibly other ID3 tags) in the beginning of each segment) */
> @@ -374,7 +374,7 @@ static void handle_variant_args(struct variant_info *info, const char *key,
>  struct key_info {
>       char uri[MAX_URL_SIZE];
>       char method[11];
> -     char iv[35];
> +     char iv[KEY_STRING_SIZE + 2]; /* 2 -> 0x */
>  };
>  
>  static void handle_key_args(struct key_info *info, const char *key,
> @@ -714,7 +714,7 @@ static int parse_playlist(HLSContext *c, const char *url,
>      int ret = 0, is_segment = 0, is_variant = 0;
>      int64_t duration = 0;
>      enum KeyType key_type = KEY_NONE;
> -    uint8_t iv[16] = "";
> +    uint8_t iv[KEYSIZE] = "";
>      int has_iv = 0;
>      char key[MAX_URL_SIZE] = "";
>      char line[MAX_URL_SIZE];
> @@ -1252,7 +1252,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
>      if (seg->key_type == KEY_NONE) {
>          ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
>      } else if (seg->key_type == KEY_AES_128) {
> -        char iv[33], key[33], url[MAX_URL_SIZE];
> +        char iv[KEY_STRING_SIZE], key[KEY_STRING_SIZE], url[MAX_URL_SIZE];
>          if (strcmp(seg->key, pls->key_url)) {
>              AVIOContext *pb = NULL;
>              if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
> @@ -1270,7 +1270,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
>          }
>          ff_data_to_hex(iv, seg->iv, sizeof(seg->iv), 0);
>          ff_data_to_hex(key, pls->key, sizeof(pls->key), 0);
> -        iv[32] = key[32] = '\0';
> +        iv[KEY_STRING_SIZE - 1] = key[KEY_STRING_SIZE - 1] = '\0';
>          if (strstr(seg->url, "://"))
>              snprintf(url, sizeof(url), "crypto+%s", seg->url);
>          else
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index c43d02c..d72edcf 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -67,7 +67,6 @@ typedef enum {
>      CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN,
>  } CodecAttributeStatus;
>  
> -#define KEYSIZE 16
>  #define LINE_BUFFER_SIZE MAX_URL_SIZE
>  #define HLS_MICROSECOND_UNIT   1000000
>  #define BUFSIZE (16 * 1024)
> @@ -85,7 +84,7 @@ typedef struct HLSSegment {
>      unsigned var_stream_idx;
>  
>      char key_uri[LINE_BUFFER_SIZE + 1];
> -    char iv_string[KEYSIZE*2 + 1];
> +    char iv_string[KEY_STRING_SIZE];
>  
>      struct HLSSegment *next;
>      double discont_program_date_time;
> @@ -169,8 +168,8 @@ typedef struct VariantStream {
>  
>      char key_file[LINE_BUFFER_SIZE + 1];
>      char key_uri[LINE_BUFFER_SIZE + 1];
> -    char key_string[KEYSIZE*2 + 1];
> -    char iv_string[KEYSIZE*2 + 1];
> +    char key_string[KEY_STRING_SIZE];
> +    char iv_string[KEY_STRING_SIZE];
>  
>      AVStream **streams;
>      char codec_attr[128];
> @@ -228,8 +227,8 @@ typedef struct HLSContext {
>      char *key_info_file;
>      char key_file[LINE_BUFFER_SIZE + 1];
>      char key_uri[LINE_BUFFER_SIZE + 1];
> -    char key_string[KEYSIZE*2 + 1];
> -    char iv_string[KEYSIZE*2 + 1];
> +    char key_string[KEY_STRING_SIZE];
> +    char iv_string[KEY_STRING_SIZE];
>      AVDictionary *vtt_format_options;
>  
>      char *method;
> @@ -734,8 +733,8 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>      }
>  
>      if (!*hls->iv_string) {
> -        uint8_t iv[16] = { 0 };
> -        char buf[33];
> +        uint8_t iv[KEYSIZE] = { 0 };
> +        char buf[KEY_STRING_SIZE];
>  
>          if (!hls->iv) {
>              AV_WB64(iv + 8, vs->sequence);
> @@ -743,7 +742,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>              memcpy(iv, hls->iv, sizeof(iv));
>          }
>          ff_data_to_hex(buf, iv, sizeof(iv), 0);
> -        buf[32] = '\0';
> +        buf[KEY_STRING_SIZE - 1] = '\0';
>          memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
>      }
>  
> @@ -1674,7 +1673,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
>      AVDictionary *options = NULL;
>      const char *proto = NULL;
>      int use_temp_file = 0;
> -    char iv_string[KEYSIZE*2 + 1];
> +    char iv_string[KEY_STRING_SIZE];
>      int err = 0;
>  
>      if (c->flags & HLS_SINGLE_FILE) {
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index d57e63c..2aefac0 100644
> --- a/libavformat/internal.h

> +++ b/libavformat/internal.h
> @@ -28,6 +28,8 @@
>  #include "os_support.h"
>  
>  #define MAX_URL_SIZE 4096
> +#define KEYSIZE 16
> +#define KEY_STRING_SIZE (KEYSIZE * 2 + 1)

So, anything using internal.h can only deal with 128 bits keys? Seems
wrong.

>  
>  /** size of probe buffer, for guessing file type from file contents */
>  #define PROBE_BUF_MIN 2048

Regards,
Nicolas George May 11, 2021, 12:45 p.m. UTC | #2 
Nicolas George (12021-05-11):
> So, anything using internal.h can only deal with 128 bits keys? Seems
> wrong.

Are you actually trying to fix something?

Regards,
Lance Wang May 11, 2021, 1:21 p.m. UTC | #3 
On Tue, May 11, 2021 at 02:44:34PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12021-05-11):
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/hls.c      | 12 ++++++------
> >  libavformat/hlsenc.c   | 19 +++++++++----------
> >  libavformat/internal.h |  2 ++
> >  3 files changed, 17 insertions(+), 16 deletions(-)
> > 
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 8fc6924..a215c6a 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -72,7 +72,7 @@ struct segment {
> >      char *url;
> >      char *key;
> >      enum KeyType key_type;
> > -    uint8_t iv[16];
> > +    uint8_t iv[KEYSIZE];
> >      /* associated Media Initialization Section, treated as a segment */
> >      struct segment *init_section;
> >  };
> > @@ -131,7 +131,7 @@ struct playlist {
> >      unsigned int init_sec_buf_read_offset;
> >  
> >      char key_url[MAX_URL_SIZE];
> > -    uint8_t key[16];
> > +    uint8_t key[KEYSIZE];
> >  
> >      /* ID3 timestamp handling (elementary audio streams have ID3 timestamps
> >       * (and possibly other ID3 tags) in the beginning of each segment) */
> > @@ -374,7 +374,7 @@ static void handle_variant_args(struct variant_info *info, const char *key,
> >  struct key_info {
> >       char uri[MAX_URL_SIZE];
> >       char method[11];
> > -     char iv[35];
> > +     char iv[KEY_STRING_SIZE + 2]; /* 2 -> 0x */
> >  };
> >  
> >  static void handle_key_args(struct key_info *info, const char *key,
> > @@ -714,7 +714,7 @@ static int parse_playlist(HLSContext *c, const char *url,
> >      int ret = 0, is_segment = 0, is_variant = 0;
> >      int64_t duration = 0;
> >      enum KeyType key_type = KEY_NONE;
> > -    uint8_t iv[16] = "";
> > +    uint8_t iv[KEYSIZE] = "";
> >      int has_iv = 0;
> >      char key[MAX_URL_SIZE] = "";
> >      char line[MAX_URL_SIZE];
> > @@ -1252,7 +1252,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
> >      if (seg->key_type == KEY_NONE) {
> >          ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
> >      } else if (seg->key_type == KEY_AES_128) {
> > -        char iv[33], key[33], url[MAX_URL_SIZE];
> > +        char iv[KEY_STRING_SIZE], key[KEY_STRING_SIZE], url[MAX_URL_SIZE];
> >          if (strcmp(seg->key, pls->key_url)) {
> >              AVIOContext *pb = NULL;
> >              if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
> > @@ -1270,7 +1270,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
> >          }
> >          ff_data_to_hex(iv, seg->iv, sizeof(seg->iv), 0);
> >          ff_data_to_hex(key, pls->key, sizeof(pls->key), 0);
> > -        iv[32] = key[32] = '\0';
> > +        iv[KEY_STRING_SIZE - 1] = key[KEY_STRING_SIZE - 1] = '\0';
> >          if (strstr(seg->url, "://"))
> >              snprintf(url, sizeof(url), "crypto+%s", seg->url);
> >          else
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index c43d02c..d72edcf 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -67,7 +67,6 @@ typedef enum {
> >      CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN,
> >  } CodecAttributeStatus;
> >  
> > -#define KEYSIZE 16
> >  #define LINE_BUFFER_SIZE MAX_URL_SIZE
> >  #define HLS_MICROSECOND_UNIT   1000000
> >  #define BUFSIZE (16 * 1024)
> > @@ -85,7 +84,7 @@ typedef struct HLSSegment {
> >      unsigned var_stream_idx;
> >  
> >      char key_uri[LINE_BUFFER_SIZE + 1];
> > -    char iv_string[KEYSIZE*2 + 1];
> > +    char iv_string[KEY_STRING_SIZE];
> >  
> >      struct HLSSegment *next;
> >      double discont_program_date_time;
> > @@ -169,8 +168,8 @@ typedef struct VariantStream {
> >  
> >      char key_file[LINE_BUFFER_SIZE + 1];
> >      char key_uri[LINE_BUFFER_SIZE + 1];
> > -    char key_string[KEYSIZE*2 + 1];
> > -    char iv_string[KEYSIZE*2 + 1];
> > +    char key_string[KEY_STRING_SIZE];
> > +    char iv_string[KEY_STRING_SIZE];
> >  
> >      AVStream **streams;
> >      char codec_attr[128];
> > @@ -228,8 +227,8 @@ typedef struct HLSContext {
> >      char *key_info_file;
> >      char key_file[LINE_BUFFER_SIZE + 1];
> >      char key_uri[LINE_BUFFER_SIZE + 1];
> > -    char key_string[KEYSIZE*2 + 1];
> > -    char iv_string[KEYSIZE*2 + 1];
> > +    char key_string[KEY_STRING_SIZE];
> > +    char iv_string[KEY_STRING_SIZE];
> >      AVDictionary *vtt_format_options;
> >  
> >      char *method;
> > @@ -734,8 +733,8 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> >      }
> >  
> >      if (!*hls->iv_string) {
> > -        uint8_t iv[16] = { 0 };
> > -        char buf[33];
> > +        uint8_t iv[KEYSIZE] = { 0 };
> > +        char buf[KEY_STRING_SIZE];
> >  
> >          if (!hls->iv) {
> >              AV_WB64(iv + 8, vs->sequence);
> > @@ -743,7 +742,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> >              memcpy(iv, hls->iv, sizeof(iv));
> >          }
> >          ff_data_to_hex(buf, iv, sizeof(iv), 0);
> > -        buf[32] = '\0';
> > +        buf[KEY_STRING_SIZE - 1] = '\0';
> >          memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
> >      }
> >  
> > @@ -1674,7 +1673,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
> >      AVDictionary *options = NULL;
> >      const char *proto = NULL;
> >      int use_temp_file = 0;
> > -    char iv_string[KEYSIZE*2 + 1];
> > +    char iv_string[KEY_STRING_SIZE];
> >      int err = 0;
> >  
> >      if (c->flags & HLS_SINGLE_FILE) {
> > diff --git a/libavformat/internal.h b/libavformat/internal.h
> > index d57e63c..2aefac0 100644
> > --- a/libavformat/internal.h
> 
> > +++ b/libavformat/internal.h
> > @@ -28,6 +28,8 @@
> >  #include "os_support.h"
> >  
> >  #define MAX_URL_SIZE 4096
> > +#define KEYSIZE 16
> > +#define KEY_STRING_SIZE (KEYSIZE * 2 + 1)
> 
> So, anything using internal.h can only deal with 128 bits keys? Seems
> wrong.

Of course, it's not my intention. It's used by hls only and I haven't find a common header
for hls.c and hlsenc.c.

> 
> >  
> >  /** size of probe buffer, for guessing file type from file contents */
> >  #define PROBE_BUF_MIN 2048
> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang May 11, 2021, 1:54 p.m. UTC | #4 
On Tue, May 11, 2021 at 02:45:07PM +0200, Nicolas George wrote:
> Nicolas George (12021-05-11):
> > So, anything using internal.h can only deal with 128 bits keys? Seems
> > wrong.
> 
> Are you actually trying to fix something?

I'm trying to test with AES-192 and  AES-256 support.

> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer May 11, 2021, 1:56 p.m. UTC | #5 
On 5/11/2021 10:21 AM, lance.lmwang@gmail.com wrote:
> On Tue, May 11, 2021 at 02:44:34PM +0200, Nicolas George wrote:
>> lance.lmwang@gmail.com (12021-05-11):
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>> ---
>>>   libavformat/hls.c      | 12 ++++++------
>>>   libavformat/hlsenc.c   | 19 +++++++++----------
>>>   libavformat/internal.h |  2 ++
>>>   3 files changed, 17 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>> index 8fc6924..a215c6a 100644
>>> --- a/libavformat/hls.c
>>> +++ b/libavformat/hls.c
>>> @@ -72,7 +72,7 @@ struct segment {
>>>       char *url;
>>>       char *key;
>>>       enum KeyType key_type;
>>> -    uint8_t iv[16];
>>> +    uint8_t iv[KEYSIZE];
>>>       /* associated Media Initialization Section, treated as a segment */
>>>       struct segment *init_section;
>>>   };
>>> @@ -131,7 +131,7 @@ struct playlist {
>>>       unsigned int init_sec_buf_read_offset;
>>>   
>>>       char key_url[MAX_URL_SIZE];
>>> -    uint8_t key[16];
>>> +    uint8_t key[KEYSIZE];
>>>   
>>>       /* ID3 timestamp handling (elementary audio streams have ID3 timestamps
>>>        * (and possibly other ID3 tags) in the beginning of each segment) */
>>> @@ -374,7 +374,7 @@ static void handle_variant_args(struct variant_info *info, const char *key,
>>>   struct key_info {
>>>        char uri[MAX_URL_SIZE];
>>>        char method[11];
>>> -     char iv[35];
>>> +     char iv[KEY_STRING_SIZE + 2]; /* 2 -> 0x */
>>>   };
>>>   
>>>   static void handle_key_args(struct key_info *info, const char *key,
>>> @@ -714,7 +714,7 @@ static int parse_playlist(HLSContext *c, const char *url,
>>>       int ret = 0, is_segment = 0, is_variant = 0;
>>>       int64_t duration = 0;
>>>       enum KeyType key_type = KEY_NONE;
>>> -    uint8_t iv[16] = "";
>>> +    uint8_t iv[KEYSIZE] = "";
>>>       int has_iv = 0;
>>>       char key[MAX_URL_SIZE] = "";
>>>       char line[MAX_URL_SIZE];
>>> @@ -1252,7 +1252,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
>>>       if (seg->key_type == KEY_NONE) {
>>>           ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
>>>       } else if (seg->key_type == KEY_AES_128) {
>>> -        char iv[33], key[33], url[MAX_URL_SIZE];
>>> +        char iv[KEY_STRING_SIZE], key[KEY_STRING_SIZE], url[MAX_URL_SIZE];
>>>           if (strcmp(seg->key, pls->key_url)) {
>>>               AVIOContext *pb = NULL;
>>>               if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
>>> @@ -1270,7 +1270,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
>>>           }
>>>           ff_data_to_hex(iv, seg->iv, sizeof(seg->iv), 0);
>>>           ff_data_to_hex(key, pls->key, sizeof(pls->key), 0);
>>> -        iv[32] = key[32] = '\0';
>>> +        iv[KEY_STRING_SIZE - 1] = key[KEY_STRING_SIZE - 1] = '\0';
>>>           if (strstr(seg->url, "://"))
>>>               snprintf(url, sizeof(url), "crypto+%s", seg->url);
>>>           else
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index c43d02c..d72edcf 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -67,7 +67,6 @@ typedef enum {
>>>       CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN,
>>>   } CodecAttributeStatus;
>>>   
>>> -#define KEYSIZE 16
>>>   #define LINE_BUFFER_SIZE MAX_URL_SIZE
>>>   #define HLS_MICROSECOND_UNIT   1000000
>>>   #define BUFSIZE (16 * 1024)
>>> @@ -85,7 +84,7 @@ typedef struct HLSSegment {
>>>       unsigned var_stream_idx;
>>>   
>>>       char key_uri[LINE_BUFFER_SIZE + 1];
>>> -    char iv_string[KEYSIZE*2 + 1];
>>> +    char iv_string[KEY_STRING_SIZE];
>>>   
>>>       struct HLSSegment *next;
>>>       double discont_program_date_time;
>>> @@ -169,8 +168,8 @@ typedef struct VariantStream {
>>>   
>>>       char key_file[LINE_BUFFER_SIZE + 1];
>>>       char key_uri[LINE_BUFFER_SIZE + 1];
>>> -    char key_string[KEYSIZE*2 + 1];
>>> -    char iv_string[KEYSIZE*2 + 1];
>>> +    char key_string[KEY_STRING_SIZE];
>>> +    char iv_string[KEY_STRING_SIZE];
>>>   
>>>       AVStream **streams;
>>>       char codec_attr[128];
>>> @@ -228,8 +227,8 @@ typedef struct HLSContext {
>>>       char *key_info_file;
>>>       char key_file[LINE_BUFFER_SIZE + 1];
>>>       char key_uri[LINE_BUFFER_SIZE + 1];
>>> -    char key_string[KEYSIZE*2 + 1];
>>> -    char iv_string[KEYSIZE*2 + 1];
>>> +    char key_string[KEY_STRING_SIZE];
>>> +    char iv_string[KEY_STRING_SIZE];
>>>       AVDictionary *vtt_format_options;
>>>   
>>>       char *method;
>>> @@ -734,8 +733,8 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>>>       }
>>>   
>>>       if (!*hls->iv_string) {
>>> -        uint8_t iv[16] = { 0 };
>>> -        char buf[33];
>>> +        uint8_t iv[KEYSIZE] = { 0 };
>>> +        char buf[KEY_STRING_SIZE];
>>>   
>>>           if (!hls->iv) {
>>>               AV_WB64(iv + 8, vs->sequence);
>>> @@ -743,7 +742,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>>>               memcpy(iv, hls->iv, sizeof(iv));
>>>           }
>>>           ff_data_to_hex(buf, iv, sizeof(iv), 0);
>>> -        buf[32] = '\0';
>>> +        buf[KEY_STRING_SIZE - 1] = '\0';
>>>           memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
>>>       }
>>>   
>>> @@ -1674,7 +1673,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
>>>       AVDictionary *options = NULL;
>>>       const char *proto = NULL;
>>>       int use_temp_file = 0;
>>> -    char iv_string[KEYSIZE*2 + 1];
>>> +    char iv_string[KEY_STRING_SIZE];
>>>       int err = 0;
>>>   
>>>       if (c->flags & HLS_SINGLE_FILE) {
>>> diff --git a/libavformat/internal.h b/libavformat/internal.h
>>> index d57e63c..2aefac0 100644
>>> --- a/libavformat/internal.h
>>
>>> +++ b/libavformat/internal.h
>>> @@ -28,6 +28,8 @@
>>>   #include "os_support.h"
>>>   
>>>   #define MAX_URL_SIZE 4096
>>> +#define KEYSIZE 16
>>> +#define KEY_STRING_SIZE (KEYSIZE * 2 + 1)
>>
>> So, anything using internal.h can only deal with 128 bits keys? Seems
>> wrong.
> 
> Of course, it's not my intention. It's used by hls only and I haven't find a common header
> for hls.c and hlsenc.c.

You can create a new hls.h header for this purpose just fine.
Lance Wang May 11, 2021, 2:47 p.m. UTC | #6 
On Tue, May 11, 2021 at 10:56:50AM -0300, James Almer wrote:
> On 5/11/2021 10:21 AM, lance.lmwang@gmail.com wrote:
> > On Tue, May 11, 2021 at 02:44:34PM +0200, Nicolas George wrote:
> > > lance.lmwang@gmail.com (12021-05-11):
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > 
> > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > >   libavformat/hls.c      | 12 ++++++------
> > > >   libavformat/hlsenc.c   | 19 +++++++++----------
> > > >   libavformat/internal.h |  2 ++
> > > >   3 files changed, 17 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > > > index 8fc6924..a215c6a 100644
> > > > --- a/libavformat/hls.c
> > > > +++ b/libavformat/hls.c
> > > > @@ -72,7 +72,7 @@ struct segment {
> > > >       char *url;
> > > >       char *key;
> > > >       enum KeyType key_type;
> > > > -    uint8_t iv[16];
> > > > +    uint8_t iv[KEYSIZE];
> > > >       /* associated Media Initialization Section, treated as a segment */
> > > >       struct segment *init_section;
> > > >   };
> > > > @@ -131,7 +131,7 @@ struct playlist {
> > > >       unsigned int init_sec_buf_read_offset;
> > > >       char key_url[MAX_URL_SIZE];
> > > > -    uint8_t key[16];
> > > > +    uint8_t key[KEYSIZE];
> > > >       /* ID3 timestamp handling (elementary audio streams have ID3 timestamps
> > > >        * (and possibly other ID3 tags) in the beginning of each segment) */
> > > > @@ -374,7 +374,7 @@ static void handle_variant_args(struct variant_info *info, const char *key,
> > > >   struct key_info {
> > > >        char uri[MAX_URL_SIZE];
> > > >        char method[11];
> > > > -     char iv[35];
> > > > +     char iv[KEY_STRING_SIZE + 2]; /* 2 -> 0x */
> > > >   };
> > > >   static void handle_key_args(struct key_info *info, const char *key,
> > > > @@ -714,7 +714,7 @@ static int parse_playlist(HLSContext *c, const char *url,
> > > >       int ret = 0, is_segment = 0, is_variant = 0;
> > > >       int64_t duration = 0;
> > > >       enum KeyType key_type = KEY_NONE;
> > > > -    uint8_t iv[16] = "";
> > > > +    uint8_t iv[KEYSIZE] = "";
> > > >       int has_iv = 0;
> > > >       char key[MAX_URL_SIZE] = "";
> > > >       char line[MAX_URL_SIZE];
> > > > @@ -1252,7 +1252,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
> > > >       if (seg->key_type == KEY_NONE) {
> > > >           ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
> > > >       } else if (seg->key_type == KEY_AES_128) {
> > > > -        char iv[33], key[33], url[MAX_URL_SIZE];
> > > > +        char iv[KEY_STRING_SIZE], key[KEY_STRING_SIZE], url[MAX_URL_SIZE];
> > > >           if (strcmp(seg->key, pls->key_url)) {
> > > >               AVIOContext *pb = NULL;
> > > >               if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
> > > > @@ -1270,7 +1270,7 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
> > > >           }
> > > >           ff_data_to_hex(iv, seg->iv, sizeof(seg->iv), 0);
> > > >           ff_data_to_hex(key, pls->key, sizeof(pls->key), 0);
> > > > -        iv[32] = key[32] = '\0';
> > > > +        iv[KEY_STRING_SIZE - 1] = key[KEY_STRING_SIZE - 1] = '\0';
> > > >           if (strstr(seg->url, "://"))
> > > >               snprintf(url, sizeof(url), "crypto+%s", seg->url);
> > > >           else
> > > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > > > index c43d02c..d72edcf 100644
> > > > --- a/libavformat/hlsenc.c
> > > > +++ b/libavformat/hlsenc.c
> > > > @@ -67,7 +67,6 @@ typedef enum {
> > > >       CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN,
> > > >   } CodecAttributeStatus;
> > > > -#define KEYSIZE 16
> > > >   #define LINE_BUFFER_SIZE MAX_URL_SIZE
> > > >   #define HLS_MICROSECOND_UNIT   1000000
> > > >   #define BUFSIZE (16 * 1024)
> > > > @@ -85,7 +84,7 @@ typedef struct HLSSegment {
> > > >       unsigned var_stream_idx;
> > > >       char key_uri[LINE_BUFFER_SIZE + 1];
> > > > -    char iv_string[KEYSIZE*2 + 1];
> > > > +    char iv_string[KEY_STRING_SIZE];
> > > >       struct HLSSegment *next;
> > > >       double discont_program_date_time;
> > > > @@ -169,8 +168,8 @@ typedef struct VariantStream {
> > > >       char key_file[LINE_BUFFER_SIZE + 1];
> > > >       char key_uri[LINE_BUFFER_SIZE + 1];
> > > > -    char key_string[KEYSIZE*2 + 1];
> > > > -    char iv_string[KEYSIZE*2 + 1];
> > > > +    char key_string[KEY_STRING_SIZE];
> > > > +    char iv_string[KEY_STRING_SIZE];
> > > >       AVStream **streams;
> > > >       char codec_attr[128];
> > > > @@ -228,8 +227,8 @@ typedef struct HLSContext {
> > > >       char *key_info_file;
> > > >       char key_file[LINE_BUFFER_SIZE + 1];
> > > >       char key_uri[LINE_BUFFER_SIZE + 1];
> > > > -    char key_string[KEYSIZE*2 + 1];
> > > > -    char iv_string[KEYSIZE*2 + 1];
> > > > +    char key_string[KEY_STRING_SIZE];
> > > > +    char iv_string[KEY_STRING_SIZE];
> > > >       AVDictionary *vtt_format_options;
> > > >       char *method;
> > > > @@ -734,8 +733,8 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> > > >       }
> > > >       if (!*hls->iv_string) {
> > > > -        uint8_t iv[16] = { 0 };
> > > > -        char buf[33];
> > > > +        uint8_t iv[KEYSIZE] = { 0 };
> > > > +        char buf[KEY_STRING_SIZE];
> > > >           if (!hls->iv) {
> > > >               AV_WB64(iv + 8, vs->sequence);
> > > > @@ -743,7 +742,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> > > >               memcpy(iv, hls->iv, sizeof(iv));
> > > >           }
> > > >           ff_data_to_hex(buf, iv, sizeof(iv), 0);
> > > > -        buf[32] = '\0';
> > > > +        buf[KEY_STRING_SIZE - 1] = '\0';
> > > >           memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
> > > >       }
> > > > @@ -1674,7 +1673,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
> > > >       AVDictionary *options = NULL;
> > > >       const char *proto = NULL;
> > > >       int use_temp_file = 0;
> > > > -    char iv_string[KEYSIZE*2 + 1];
> > > > +    char iv_string[KEY_STRING_SIZE];
> > > >       int err = 0;
> > > >       if (c->flags & HLS_SINGLE_FILE) {
> > > > diff --git a/libavformat/internal.h b/libavformat/internal.h
> > > > index d57e63c..2aefac0 100644
> > > > --- a/libavformat/internal.h
> > > 
> > > > +++ b/libavformat/internal.h
> > > > @@ -28,6 +28,8 @@
> > > >   #include "os_support.h"
> > > >   #define MAX_URL_SIZE 4096
> > > > +#define KEYSIZE 16
> > > > +#define KEY_STRING_SIZE (KEYSIZE * 2 + 1)
> > > 
> > > So, anything using internal.h can only deal with 128 bits keys? Seems
> > > wrong.
> > 
> > Of course, it's not my intention. It's used by hls only and I haven't find a common header
> > for hls.c and hlsenc.c.
> 
> You can create a new hls.h header for this purpose just fine.

That's good, will updated by your suggestion.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Nicolas George May 11, 2021, 3:04 p.m. UTC | #7 
lance.lmwang@gmail.com (12021-05-11):
> I'm trying to test with AES-192 and  AES-256 support.

Then you will need to differentiate places that are a buffer and you
need to know the maximum size of the key, and places where the size you
have is the actual key size. Introducing these macros will not help
much.

Regards,
Lance Wang May 11, 2021, 10:54 p.m. UTC | #8 
On Tue, May 11, 2021 at 05:04:57PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12021-05-11):
> > I'm trying to test with AES-192 and  AES-256 support.
> 
> Then you will need to differentiate places that are a buffer and you
> need to know the maximum size of the key, and places where the size you
> have is the actual key size. Introducing these macros will not help
> much.
The current code assume it's 128bit and what I want to change is the
maximum size of the fixed array and don't break any code first. The
actual key size will get by method and will change later. Maybe I should
change the macros to MAX_KEYSIZE?

> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 8fc6924..a215c6a 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -72,7 +72,7 @@  struct segment {
     char *url;
     char *key;
     enum KeyType key_type;
-    uint8_t iv[16];
+    uint8_t iv[KEYSIZE];
     /* associated Media Initialization Section, treated as a segment */
     struct segment *init_section;
 };
@@ -131,7 +131,7 @@  struct playlist {
     unsigned int init_sec_buf_read_offset;
 
     char key_url[MAX_URL_SIZE];
-    uint8_t key[16];
+    uint8_t key[KEYSIZE];
 
     /* ID3 timestamp handling (elementary audio streams have ID3 timestamps
      * (and possibly other ID3 tags) in the beginning of each segment) */
@@ -374,7 +374,7 @@  static void handle_variant_args(struct variant_info *info, const char *key,
 struct key_info {
      char uri[MAX_URL_SIZE];
      char method[11];
-     char iv[35];
+     char iv[KEY_STRING_SIZE + 2]; /* 2 -> 0x */
 };
 
 static void handle_key_args(struct key_info *info, const char *key,
@@ -714,7 +714,7 @@  static int parse_playlist(HLSContext *c, const char *url,
     int ret = 0, is_segment = 0, is_variant = 0;
     int64_t duration = 0;
     enum KeyType key_type = KEY_NONE;
-    uint8_t iv[16] = "";
+    uint8_t iv[KEYSIZE] = "";
     int has_iv = 0;
     char key[MAX_URL_SIZE] = "";
     char line[MAX_URL_SIZE];
@@ -1252,7 +1252,7 @@  static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
     if (seg->key_type == KEY_NONE) {
         ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
     } else if (seg->key_type == KEY_AES_128) {
-        char iv[33], key[33], url[MAX_URL_SIZE];
+        char iv[KEY_STRING_SIZE], key[KEY_STRING_SIZE], url[MAX_URL_SIZE];
         if (strcmp(seg->key, pls->key_url)) {
             AVIOContext *pb = NULL;
             if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
@@ -1270,7 +1270,7 @@  static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
         }
         ff_data_to_hex(iv, seg->iv, sizeof(seg->iv), 0);
         ff_data_to_hex(key, pls->key, sizeof(pls->key), 0);
-        iv[32] = key[32] = '\0';
+        iv[KEY_STRING_SIZE - 1] = key[KEY_STRING_SIZE - 1] = '\0';
         if (strstr(seg->url, "://"))
             snprintf(url, sizeof(url), "crypto+%s", seg->url);
         else
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index c43d02c..d72edcf 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -67,7 +67,6 @@  typedef enum {
     CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN,
 } CodecAttributeStatus;
 
-#define KEYSIZE 16
 #define LINE_BUFFER_SIZE MAX_URL_SIZE
 #define HLS_MICROSECOND_UNIT   1000000
 #define BUFSIZE (16 * 1024)
@@ -85,7 +84,7 @@  typedef struct HLSSegment {
     unsigned var_stream_idx;
 
     char key_uri[LINE_BUFFER_SIZE + 1];
-    char iv_string[KEYSIZE*2 + 1];
+    char iv_string[KEY_STRING_SIZE];
 
     struct HLSSegment *next;
     double discont_program_date_time;
@@ -169,8 +168,8 @@  typedef struct VariantStream {
 
     char key_file[LINE_BUFFER_SIZE + 1];
     char key_uri[LINE_BUFFER_SIZE + 1];
-    char key_string[KEYSIZE*2 + 1];
-    char iv_string[KEYSIZE*2 + 1];
+    char key_string[KEY_STRING_SIZE];
+    char iv_string[KEY_STRING_SIZE];
 
     AVStream **streams;
     char codec_attr[128];
@@ -228,8 +227,8 @@  typedef struct HLSContext {
     char *key_info_file;
     char key_file[LINE_BUFFER_SIZE + 1];
     char key_uri[LINE_BUFFER_SIZE + 1];
-    char key_string[KEYSIZE*2 + 1];
-    char iv_string[KEYSIZE*2 + 1];
+    char key_string[KEY_STRING_SIZE];
+    char iv_string[KEY_STRING_SIZE];
     AVDictionary *vtt_format_options;
 
     char *method;
@@ -734,8 +733,8 @@  static int do_encrypt(AVFormatContext *s, VariantStream *vs)
     }
 
     if (!*hls->iv_string) {
-        uint8_t iv[16] = { 0 };
-        char buf[33];
+        uint8_t iv[KEYSIZE] = { 0 };
+        char buf[KEY_STRING_SIZE];
 
         if (!hls->iv) {
             AV_WB64(iv + 8, vs->sequence);
@@ -743,7 +742,7 @@  static int do_encrypt(AVFormatContext *s, VariantStream *vs)
             memcpy(iv, hls->iv, sizeof(iv));
         }
         ff_data_to_hex(buf, iv, sizeof(iv), 0);
-        buf[32] = '\0';
+        buf[KEY_STRING_SIZE - 1] = '\0';
         memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
     }
 
@@ -1674,7 +1673,7 @@  static int hls_start(AVFormatContext *s, VariantStream *vs)
     AVDictionary *options = NULL;
     const char *proto = NULL;
     int use_temp_file = 0;
-    char iv_string[KEYSIZE*2 + 1];
+    char iv_string[KEY_STRING_SIZE];
     int err = 0;
 
     if (c->flags & HLS_SINGLE_FILE) {
diff --git a/libavformat/internal.h b/libavformat/internal.h
index d57e63c..2aefac0 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -28,6 +28,8 @@ 
 #include "os_support.h"
 
 #define MAX_URL_SIZE 4096
+#define KEYSIZE 16
+#define KEY_STRING_SIZE (KEYSIZE * 2 + 1)
 
 /** size of probe buffer, for guessing file type from file contents */
 #define PROBE_BUF_MIN 2048