On date Monday 2024-04-15 02:32:14 +0000, ������ | 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 <eug...@bitsensing.com>
> ---
> 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);
> + 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_strings(hash_ctx, username, ":", state->realm, ":",
> password, NULL);
> + if (av_stristr(algorithm, "-sess")) {
> + update_hash_strings(hash_ctx, ":", digest->nonce, ":", cnonce, NULL);
> }
> - update_md5_strings(md5ctx, ":", A2hash, NULL);
> - av_md5_final(md5ctx, hash);
> - ff_data_to_hex(response, hash, 16, 1);
> -
> - av_free(md5ctx);
> -
> - if (!strcmp(digest->qop, "") || !strcmp(digest->qop, "auth")) {
> - } else if (!strcmp(digest->qop, "auth-int")) {
> - /* qop=auth-int not supported */
> - return NULL;
> - } else {
> - /* Unsupported qop value. */
> + av_hash_final_hex(hash_ctx, a1_hash, len_hash);
> +
> + /* a2 hash calculation */
> + av_hash_init(hash_ctx);
> + update_hash_strings(hash_ctx, method, ":", uri, NULL);
> + av_hash_final_hex(hash_ctx, a2_hash, len_hash);
> +
> + /* response hash calculation */
> + av_hash_init(hash_ctx);
> + update_hash_strings(hash_ctx, a1_hash, ":", digest->nonce, NULL);
> + if (!strcmp(digest->qop, "auth")) {
> + update_hash_strings(hash_ctx, ":", nc, ":", cnonce, ":",
> digest->qop, NULL);
> + } else if (!strcmp(digest->qop, "auth-int")) { // unsupported
> + av_hash_freep(&hash_ctx);
> return NULL;
> }
> + update_hash_strings(hash_ctx, ":", a2_hash, NULL);
> + update_hash_strings(hash_ctx, NULL);
> + av_hash_final_hex(hash_ctx, response, len_hash);
> + av_hash_freep(&hash_ctx);
>
> + /* Authorization header generation */
> len = strlen(username) + strlen(state->realm) + strlen(digest->nonce) +
> strlen(uri) + strlen(response) + strlen(digest->algorithm) +
> strlen(digest->opaque) + strlen(digest->qop) + strlen(cnonce) +
> --
> 2.42.0.windows.2
Looks good to me otherwise, I'll wait a few more days to let more
comments before applying this, thanks.
_______________________________________________
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".
