Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]
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]
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]
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]
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