Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]

2024-04-22 Thread Stefano Sabatini
On date Thursday 2024-04-18 12:49:53 +0200, Stefano Sabatini wrote:
> On date Monday 2024-04-15 19:56:48 +0200, Stefano Sabatini wrote:
> > On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> > > Update digest authentication in httpauth.c
> > > 
> > > - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> > > - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> > > - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> > > - Replace AVMD5 structure with AVHashContext for hash algorithm 
> > > flexibility
> > > - Rename update_md5_strings() to update_hash_strings() for consistency
> > > - Address coding style feedback from reviewer:
> > > 
> > > This is a feature update focused on digest authentication enhancements.
> > > Some lint issues in the existing code remain unaddressed in this patch.
> > > 
> > > Signed-off-by: Eugene-bitsensing 
> > > ---
> > 
> > >  libavformat/httpauth.c | 102 +
> > >  1 file changed, 53 insertions(+), 49 deletions(-)
> > 
> > add entry to Changelog?
> 
> Updated the patch with a few fixes, please check in attachment.
> 
> [...] 
> > nit++: to avoid semantic overlap I'd rather name this
> > selected_algorithm
> > 
> > > +if (!*algorithm) {
> > > +algorithm = "MD5";  // if empty, use MD5 as Default 
> > > +hashing_algorithm = "MD5";
> > > +} else if (av_stristr(algorithm, "MD5")) {
> > > +hashing_algorithm = "MD5";
> > > +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm, 
> > > "sha-256")) {
> > > +hashing_algorithm = "SHA256";
> > > +len_hash = 65; // SHA-2: 64 characters.
> 
> > > +} else if (av_stristr(algorithm, "sha512-256") || 
> > > av_stristr(algorithm, "sha-512-256")) {
> > > +hashing_algorithm = "SHA512_256";
> > > +len_hash = 65; // SHA-2: 64 characters.
> 
> I'm concerned by this, as it will never be reached because the "sha256"
> branch is always selected instead, that's why this should be made an
> exact match?

Ping to Eugene. I think it would be safe with the use of stricmp.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]

2024-04-18 Thread Stefano Sabatini
On date Monday 2024-04-15 19:56:48 +0200, Stefano Sabatini wrote:
> On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> > Update digest authentication in httpauth.c
> > 
> > - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> > - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> > - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> > - Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
> > - Rename update_md5_strings() to update_hash_strings() for consistency
> > - Address coding style feedback from reviewer:
> > 
> > This is a feature update focused on digest authentication enhancements.
> > Some lint issues in the existing code remain unaddressed in this patch.
> > 
> > Signed-off-by: Eugene-bitsensing 
> > ---
> 
> >  libavformat/httpauth.c | 102 +
> >  1 file changed, 53 insertions(+), 49 deletions(-)
> 
> add entry to Changelog?

Updated the patch with a few fixes, please check in attachment.

[...] 
> nit++: to avoid semantic overlap I'd rather name this
> selected_algorithm
> 
> > +if (!*algorithm) {
> > +algorithm = "MD5";  // if empty, use MD5 as Default 
> > +hashing_algorithm = "MD5";
> > +} else if (av_stristr(algorithm, "MD5")) {
> > +hashing_algorithm = "MD5";
> > +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm, 
> > "sha-256")) {
> > +hashing_algorithm = "SHA256";
> > +len_hash = 65; // SHA-2: 64 characters.

> > +} else if (av_stristr(algorithm, "sha512-256") || 
> > av_stristr(algorithm, "sha-512-256")) {
> > +hashing_algorithm = "SHA512_256";
> > +len_hash = 65; // SHA-2: 64 characters.

I'm concerned by this, as it will never be reached because the "sha256"
branch is always selected instead, that's why this should be made an
exact match?
>From 2b88a2f64569bb83c0f519983385c35c217bbf9c Mon Sep 17 00:00:00 2001
From: "eug...@bitsensing.com" 
Date: Mon, 15 Apr 2024 02:32:14 +
Subject: [PATCH] avformat/httpauth.c: support RFC7616 authentication

- Refactor make_digest_auth() to support RFC 2617 and RFC 7617
- Add support for SHA-256 and SHA-512/256 algorithms along with MD5
- MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
- Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
- Rename update_md5_strings() to update_hash_strings() for consistency

Signed-off-by: Eugene-bitsensing 
---
 Changelog  |   1 +
 libavformat/httpauth.c | 104 +
 2 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/Changelog b/Changelog
index 8db14f02b4..5db501f8c4 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version :
 - ffmpeg CLI filtergraph chaining
 - LC3/LC3plus demuxer and muxer
 - pad_vaapi, drawbox_vaapi filters
+- HTTP protocol RFC7617 authentication
 
 
 version 7.0:
diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
index 9048362509..90210e6179 100644
--- a/libavformat/httpauth.c
+++ b/libavformat/httpauth.c
@@ -25,7 +25,7 @@
 #include "libavutil/mem.h"
 #include "internal.h"
 #include "libavutil/random_seed.h"
-#include "libavutil/md5.h"
+#include "libavutil/hash.h"
 #include "urldecode.h"
 
 static void handle_basic_params(HTTPAuthState *state, const char *key,
@@ -118,36 +118,36 @@ void ff_http_auth_handle_header(HTTPAuthState *state, const char *key,
 }
 }
 
-
-static void update_md5_strings(struct AVMD5 *md5ctx, ...)
+static void update_hash_strings(struct AVHashContext *hash_ctx, ...)
 {
 va_list vl;
 
-va_start(vl, md5ctx);
+va_start(vl, hash_ctx);
 while (1) {
-const char* str = va_arg(vl, const char*);
+const char *str = va_arg(vl, const char*);
 if (!str)
 break;
-av_md5_update(md5ctx, str, strlen(str));
+av_hash_update(hash_ctx, (const uint8_t *)str, strlen(str));
 }
 va_end(vl);
 }
 
-/* Generate a digest reply, according to RFC 2617. */
+/* Generate a digest reply, according to RFC 2617/7617 */
 static char *make_digest_auth(HTTPAuthState *state, const char *username,
   const char *password, const char *uri,
   const char *method)
 {
 DigestParams *digest = &state->digest_params;
-int len;
+size_t len;
 uint32_t cnonce_buf[2];
 char cnonce[17];
 char nc[9];
-int i;
-char A1hash[33], A2hash[33], response[33];
-struct AVMD5 *md5ctx;
-uint8_t hash[16];
+int i, ret;
+char a1_hash[65], a2_hash[65], response[65];
+struct AVHashContext *hash_ctx = NULL;
+size_t len_hash = 33; // Modifiable hash length, MD5:32, SHA-2:64
 char *authstr;
+const char *algorithm, *hashing_algorithm;
 
 digest->nc++;
 snprintf(nc, sizeof(nc), "%08x", digest->nc);
@@ -157,52 +157,56 @@ static char *make_digest_auth(HTTPAuthState *state, const char *username,
 cno

Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]

2024-04-15 Thread Stefano Sabatini
On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> Update digest authentication in httpauth.c
> 
> - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> - Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
> - Rename update_md5_strings() to update_hash_strings() for consistency
> - Address coding style feedback from reviewer:
> 
> This is a feature update focused on digest authentication enhancements.
> Some lint issues in the existing code remain unaddressed in this patch.
> 
> Signed-off-by: Eugene-bitsensing 
> ---

>  libavformat/httpauth.c | 102 +
>  1 file changed, 53 insertions(+), 49 deletions(-)

add entry to Changelog?

> 
> diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
> index 9780928357..2592140526 100644
> --- a/libavformat/httpauth.c
> +++ b/libavformat/httpauth.c
> @@ -24,7 +24,7 @@
>  #include "libavutil/avstring.h"
>  #include "internal.h"
>  #include "libavutil/random_seed.h"
> -#include "libavutil/md5.h"
> +#include "libavutil/hash.h"
>  #include "urldecode.h"
>  
>  static void handle_basic_params(HTTPAuthState *state, const char *key,
> @@ -117,35 +117,35 @@ void ff_http_auth_handle_header(HTTPAuthState *state, 
> const char *key,
>  }
>  }
>  
> -
> -static void update_md5_strings(struct AVMD5 *md5ctx, ...)
> +/* Generate hash string, updated to use AVHashContext to support other 
> algorithms */
> +static void update_hash_strings(struct AVHashContext *hash_ctx, ...)
>  {
>  va_list vl;
>  
> -va_start(vl, md5ctx);
> +va_start(vl, hash_ctx);
>  while (1) {
> -const char* str = va_arg(vl, const char*);
> +const char *str = va_arg(vl, const char*);
>  if (!str)
>  break;
> -av_md5_update(md5ctx, str, strlen(str));
> +av_hash_update(hash_ctx, (const uint8_t *)str, strlen(str));
>  }
>  va_end(vl);
>  }
>  
> -/* Generate a digest reply, according to RFC 2617. */
> +/* Generate a digest reply, according to RFC 2617. Update to support RFC 
> 7617*/
>  static char *make_digest_auth(HTTPAuthState *state, const char *username,
>const char *password, const char *uri,
>const char *method)
>  {
>  DigestParams *digest = &state->digest_params;
> -int len;
> +size_t len;
>  uint32_t cnonce_buf[2];
>  char cnonce[17];
>  char nc[9];
>  int i;
> -char A1hash[33], A2hash[33], response[33];
> -struct AVMD5 *md5ctx;
> -uint8_t hash[16];
> +char a1_hash[65], a2_hash[65], response[65];
> +struct AVHashContext *hash_ctx = NULL; // use AVHashContext for other 
> algorithm support
> +size_t len_hash = 33; // Modifiable hash length, MD5:32, SHA-2:64
>  char *authstr;
>  
>  digest->nc++;
> @@ -156,52 +156,56 @@ static char *make_digest_auth(HTTPAuthState *state, 
> const char *username,
>  cnonce_buf[i] = av_get_random_seed();
>  ff_data_to_hex(cnonce, (const uint8_t*) cnonce_buf, sizeof(cnonce_buf), 
> 1);
>  
> -md5ctx = av_md5_alloc();
> -if (!md5ctx)
> -return NULL;
> -
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, username, ":", state->realm, ":", password, 
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -
> -if (!strcmp(digest->algorithm, "") || !strcmp(digest->algorithm, "MD5")) 
> {
> -} else if (!strcmp(digest->algorithm, "MD5-sess")) {
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, A1hash, ":", digest->nonce, ":", cnonce, 
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -} else {
> -/* Unsupported algorithm */
> -av_free(md5ctx);
> +/* Generate hash context by algorithm. */
> +const char *algorithm = digest->algorithm;

> +const char *hashing_algorithm;

nit++: to avoid semantic overlap I'd rather name this
selected_algorithm

> +if (!*algorithm) {
> +algorithm = "MD5";  // if empty, use MD5 as Default 
> +hashing_algorithm = "MD5";
> +} else if (av_stristr(algorithm, "MD5")) {
> +hashing_algorithm = "MD5";
> +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm, 
> "sha-256")) {
> +hashing_algorithm = "SHA256";
> +len_hash = 65; // SHA-2: 64 characters.
> +} else if (av_stristr(algorithm, "sha512-256") || av_stristr(algorithm, 
> "sha-512-256")) {
> +hashing_algorithm = "SHA512_256";
> +len_hash = 65; // SHA-2: 64 characters.
> +} else { // Unsupported algorithm
>  return NULL;
>  }
>  
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, method, ":", uri, NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A2hash, hash, 16, 1);
> +   

[FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]

2024-04-14 Thread 정지우 | Eugene
Update digest authentication in httpauth.c

- Refactor make_digest_auth() to support RFC 2617 and RFC 7617
- Add support for SHA-256 and SHA-512/256 algorithms along with MD5
- MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
- Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
- Rename update_md5_strings() to update_hash_strings() for consistency
- Address coding style feedback from reviewer:

This is a feature update focused on digest authentication enhancements.
Some lint issues in the existing code remain unaddressed in this patch.

Signed-off-by: Eugene-bitsensing 
---
 libavformat/httpauth.c | 102 +
 1 file changed, 53 insertions(+), 49 deletions(-)

diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
index 9780928357..2592140526 100644
--- a/libavformat/httpauth.c
+++ b/libavformat/httpauth.c
@@ -24,7 +24,7 @@
 #include "libavutil/avstring.h"
 #include "internal.h"
 #include "libavutil/random_seed.h"
-#include "libavutil/md5.h"
+#include "libavutil/hash.h"
 #include "urldecode.h"
 
 static void handle_basic_params(HTTPAuthState *state, const char *key,
@@ -117,35 +117,35 @@ void ff_http_auth_handle_header(HTTPAuthState *state, 
const char *key,
 }
 }
 
-
-static void update_md5_strings(struct AVMD5 *md5ctx, ...)
+/* Generate hash string, updated to use AVHashContext to support other 
algorithms */
+static void update_hash_strings(struct AVHashContext *hash_ctx, ...)
 {
 va_list vl;
 
-va_start(vl, md5ctx);
+va_start(vl, hash_ctx);
 while (1) {
-const char* str = va_arg(vl, const char*);
+const char *str = va_arg(vl, const char*);
 if (!str)
 break;
-av_md5_update(md5ctx, str, strlen(str));
+av_hash_update(hash_ctx, (const uint8_t *)str, strlen(str));
 }
 va_end(vl);
 }
 
-/* Generate a digest reply, according to RFC 2617. */
+/* Generate a digest reply, according to RFC 2617. Update to support RFC 7617*/
 static char *make_digest_auth(HTTPAuthState *state, const char *username,
   const char *password, const char *uri,
   const char *method)
 {
 DigestParams *digest = &state->digest_params;
-int len;
+size_t len;
 uint32_t cnonce_buf[2];
 char cnonce[17];
 char nc[9];
 int i;
-char A1hash[33], A2hash[33], response[33];
-struct AVMD5 *md5ctx;
-uint8_t hash[16];
+char a1_hash[65], a2_hash[65], response[65];
+struct AVHashContext *hash_ctx = NULL; // use AVHashContext for other 
algorithm support
+size_t len_hash = 33; // Modifiable hash length, MD5:32, SHA-2:64
 char *authstr;
 
 digest->nc++;
@@ -156,52 +156,56 @@ static char *make_digest_auth(HTTPAuthState *state, const 
char *username,
 cnonce_buf[i] = av_get_random_seed();
 ff_data_to_hex(cnonce, (const uint8_t*) cnonce_buf, sizeof(cnonce_buf), 1);
 
-md5ctx = av_md5_alloc();
-if (!md5ctx)
-return NULL;
-
-av_md5_init(md5ctx);
-update_md5_strings(md5ctx, username, ":", state->realm, ":", password, 
NULL);
-av_md5_final(md5ctx, hash);
-ff_data_to_hex(A1hash, hash, 16, 1);
-
-if (!strcmp(digest->algorithm, "") || !strcmp(digest->algorithm, "MD5")) {
-} else if (!strcmp(digest->algorithm, "MD5-sess")) {
-av_md5_init(md5ctx);
-update_md5_strings(md5ctx, A1hash, ":", digest->nonce, ":", cnonce, 
NULL);
-av_md5_final(md5ctx, hash);
-ff_data_to_hex(A1hash, hash, 16, 1);
-} else {
-/* Unsupported algorithm */
-av_free(md5ctx);
+/* Generate hash context by algorithm. */
+const char *algorithm = digest->algorithm;
+const char *hashing_algorithm;
+if (!*algorithm) {
+algorithm = "MD5";  // if empty, use MD5 as Default 
+hashing_algorithm = "MD5";
+} else if (av_stristr(algorithm, "MD5")) {
+hashing_algorithm = "MD5";
+} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm, 
"sha-256")) {
+hashing_algorithm = "SHA256";
+len_hash = 65; // SHA-2: 64 characters.
+} else if (av_stristr(algorithm, "sha512-256") || av_stristr(algorithm, 
"sha-512-256")) {
+hashing_algorithm = "SHA512_256";
+len_hash = 65; // SHA-2: 64 characters.
+} else { // Unsupported algorithm
 return NULL;
 }
 
-av_md5_init(md5ctx);
-update_md5_strings(md5ctx, method, ":", uri, NULL);
-av_md5_final(md5ctx, hash);
-ff_data_to_hex(A2hash, hash, 16, 1);
+int ret = av_hash_alloc(&hash_ctx, hashing_algorithm);
+if (ret < 0)
+return NULL;
 
-av_md5_init(md5ctx);
-update_md5_strings(md5ctx, A1hash, ":", digest->nonce, NULL);
-if (!strcmp(digest->qop, "auth") || !strcmp(digest->qop, "auth-int")) {
-update_md5_strings(md5ctx, ":", nc, ":", cnonce, ":", digest->qop, 
NULL);
+/* a1 hash calculation */
+av_hash_init(hash_ctx);
+update_hash_string