Add full support for SHA-256 and SHA-512-256 digest algorithms #4118

gtjoseph · 2024-10-28T22:54:22Z

There are no breaking changes for this work however several structures
were extended with new fields. See below.

In order to use the new algorithms, you MUST set the new
pjsip_cred_info.ext.algorithm_type field to the appropriate value
when the credential data type is PJSIP_CRED_DATA_DIGEST and when
acting as a server, you must also use pjsip_auth_srv_challenge2()
to send challenges so you can specify algorithms other than MD5.

Summary of changes:

Added enum pjsip_auth_algorithm_type which list all digest algorithms
supported.
Added struct pjsip_auth_algorithm which defines parameters for each
algorithm including its IANA name, OpenSSL name, digest length and
digest string representation length.
Added pjsip_auth_algorithm_type to the pjsip_cred_info structure
so the digest algorithm can be specified when the cred data type
is PJSIP_CRED_DATA_DIGEST.
Added pjsip_auth_algorithm_type to the pjsip_cached_auth_hdr
structure so we can match on specific algorithm.
Added functions pjsip_auth_get_algorithm_by_type(),
pjsip_auth_get_algorithm_by_iana_name(), and
pjsip_auth_is_digest_algorithm_supported() to find and search
for supported algorithms.
Added pjsip_authorization_hdr to the pjsip_auth_lookup_cred_param
structure so we can look up credentiials by specific algorithm.
Added the pjsip_auth_srv_challenge2() function that takes
a pjsip_auth_algorithm_type so users can create challenges with
specific algorithms instead of defaulting to MD5.
pjsip_auth_create_digest() was heavily refactored to use the
new algorithm_type contained in pjsip_cred_info to determine the
algorithm to use when creating the digest. The function is now
generic and can use any supported algorithm. If OpenSSL isn't
available, it will fall back to the internal MD5 implementation.
pjsip_auth_create_digestSHA256() is now marked as deprecated and
simply calls the new function with PJSIP_AUTH_ALGORITHM_SHA256.
sip_auth_client.c and sip_auth_server.c were refactored to support
multiple digest algorithms.
sip_auth_client was updated to allow the AKEv2-MD5 algorithm
to pass through to the callback specified in pjsip_cred_info.
A bug was fixed with the PJSIP_AUTH_ALLOW_MULTIPLE_AUTH_HEADER
option where the default setting of 0 prevented sip_auth_client
from responding to WWW/Proxy-Authenticate headers from different
realms. The RFCs state that this behavior should be allowed.
The comment for this option in sip_config.h was also updated to
indicate that setting this option to 1 is probably not a good idea
for security reasons.

Resolves: #4119

gtjoseph · 2024-10-28T22:55:56Z

This PR is in draft mode because we're still testing with Asterisk and older versions of OpenSSL. I wanted to get it up to receive comments sooner rather than later though.

gtjoseph · 2024-10-28T23:08:29Z

Oh you're kidding. We still need to support building without OpenSSL?

gtjoseph · 2024-10-29T12:22:16Z

I still need to clean up the OpenSSL/nonm-OpenSSL things and remove some testing things I did to get a successful Windows build but it's looking much better.

gtjoseph · 2024-10-30T01:33:59Z

Rebased on current master. All asterisk tests pass.

gtjoseph · 2024-10-30T13:30:26Z

The last force push contained just documentation updates in sip_auth.h and sip_auth_aka.h

wosrediinanatour · 2024-11-02T07:49:09Z

pjsip/include/pjsip/sip_auth.h

+                                                   SIP headers               */
+    const char *openssl_name;                 /**< The name used by OpenSSL's
+                                                   EVP_get_digestbyname()    */
+    unsigned digest_length;                   /**< Length of the raw digest


Here and below I would use pj_size_t.

Digest lengths will never need a size_t so why allocate an extra 4 bytes for each of them?

wosrediinanatour · 2024-11-02T07:50:33Z

pjsip/include/pjsip/sip_auth.h

@@ -115,9 +179,15 @@ struct pjsip_cred_info
                                     challenges.                            */
    pj_str_t    scheme;         /**< Scheme (e.g. "digest").                */
    pj_str_t    username;       /**< User name.                             */
-    int         data_type;      /**< Type of data (0 for plaintext passwd). */
+    int         data_type;      /**< Type of data \ref pjsip_cred_data_type */


Why not using pjsip_cred_data_type instead of int?

Because that's the way it was historically and I didn't want to change the signature with this patch.

nanangizz · 2024-11-04T09:53:16Z

pjsip/src/pjsip/sip_auth_client.c

+    if (!(result && nonce && uri && realm && cred_info && method)) {
+        PJ_LOG(4, (THIS_FILE,
+                "result, nonce, uri, realm, cred_info and method are all required"));
        return PJ_EINVAL;
    }


Usually we apply assertion (i.e: PJ_ASSERT_RETURN()) for argument verifications, which may help pinpointing app issues earlier.

Odd. I remember using PJ_ASSERT_RETURN here. I wonder why I changed it to an "if". Anyway, will be fixed shortly.

nanangizz · 2024-11-04T10:19:26Z

pjsip/src/pjsip/sip_auth_client.c

         ***/
-#if OPENSSL_VERSION_NUMBER < 0x30000000L


Looks like all codes for OpenSSL < 3 is removed, we expect it is still compatible with OpenSSL 1.1.x, can you confirm or have you tested?

The code was a bit weird when it came to OpenSSL versions. The things that were only enabled with OpenSSL >= 3.0.0 (EVP_MD, EVP_Digest, etc) were all available with earlier versions back to 1.1.0 so I'm not sure why those SHA256_ alternatives were there.

I did build Asterisk with this patch on CentOS7 with OpenSSL 1.1.0 and tested both UAC and UAS authentication, including detecting that OpenSSL 1.1.0 didn't support SHA-512-256. Everything worked fine.

From a manual page here, EVP_DigestInit_ex() seems to be introduced in 0.9.7. So perhaps those alternatives were actually for OpenSSL older than 0.9.7, which is too old by now, especially for a security library :)

Also thanks for the confirmation that it works fine with 1.1.0.

There are no breaking changes for this work however several structures were extended with new fields. See below. In order to use the new algorithms, you MUST set the new pjsip_cred_info.ext.algorithm_type field to the appropriate value when the credential data type is PJSIP_CRED_DATA_DIGEST and when acting as a server, you must also use pjsip_auth_srv_challenge2() to send challenges so you can specify algorithms other than MD5. Summary of changes: * Added enum pjsip_auth_algorithm_type which list all digest algorithms supported. * Added struct pjsip_auth_algorithm which defines parameters for each algorithm including its IANA name, OpenSSL name, digest length and digest string representation length. * Added pjsip_auth_algorithm_type to the pjsip_cred_info structure so the digest algorithm can be specified when the cred data type is PJSIP_CRED_DATA_DIGEST. * Added pjsip_auth_algorithm_type to the pjsip_cached_auth_hdr structure so we can match on specific algorithm. * Added functions pjsip_auth_get_algorithm_by_type(), pjsip_auth_get_algorithm_by_iana_name(), and pjsip_auth_is_digest_algorithm_supported() to find and search for supported algorithms. * Added pjsip_authorization_hdr to the pjsip_auth_lookup_cred_param structure so we can look up credentiials by specific algorithm. * Added the pjsip_auth_srv_challenge2() function that takes a pjsip_auth_algorithm_type so users can create challenges with specific algorithms instead of defaulting to MD5. * pjsip_auth_create_digest() was heavily refactored to use the new algorithm_type contained in pjsip_cred_info to determine the algorithm to use when creating the digest. The function is now generic and can use any supported algorithm. If OpenSSL isn't available, it will fall back to the internal MD5 implementation. * pjsip_auth_create_digestSHA256() is now marked as deprecated and simply calls the new function with PJSIP_AUTH_ALGORITHM_SHA256. * sip_auth_client.c and sip_auth_server.c were refactored to support multiple digest algorithms. * sip_auth_client was updated to allow the AKEv2-MD5 algorithm to pass through to the callback specified in pjsip_cred_info. * A bug was fixed with the PJSIP_AUTH_ALLOW_MULTIPLE_AUTH_HEADER option where the default setting of 0 prevented sip_auth_client from responding to WWW/Proxy-Authenticate headers from different realms. The RFCs state that this behavior should be allowed. The comment for this option in sip_config.h was also updated to indicate that setting this option to 1 is probably not a good idea for security reasons. Resolves: pjsip#4119

gtjoseph force-pushed the master-new-digest-algorithms branch from 6cd74df to a3fe8c1 Compare October 28, 2024 23:07

gtjoseph force-pushed the master-new-digest-algorithms branch 7 times, most recently from c49669b to c037f3b Compare October 29, 2024 02:56

gtjoseph force-pushed the master-new-digest-algorithms branch 2 times, most recently from 6ef608d to 197ca22 Compare October 29, 2024 18:41

gtjoseph marked this pull request as ready for review October 29, 2024 18:43

gtjoseph force-pushed the master-new-digest-algorithms branch from 197ca22 to 78ada2f Compare October 30, 2024 13:29

nanangizz requested review from trengginas, sauwming and nanangizz November 4, 2024 07:24

nanangizz added type: enhancement component: pjsip labels Nov 4, 2024

nanangizz added this to the release-2.15 milestone Nov 4, 2024

sauwming approved these changes Nov 4, 2024

View reviewed changes

wosrediinanatour reviewed Nov 4, 2024

View reviewed changes

nanangizz reviewed Nov 4, 2024

View reviewed changes

gtjoseph force-pushed the master-new-digest-algorithms branch from 78ada2f to 3200187 Compare November 4, 2024 15:20

gtjoseph requested review from wosrediinanatour and nanangizz November 4, 2024 15:21

trengginas approved these changes Nov 5, 2024

View reviewed changes

nanangizz approved these changes Nov 5, 2024

View reviewed changes

nanangizz merged commit 2e00417 into pjsip:master Nov 7, 2024
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add full support for SHA-256 and SHA-512-256 digest algorithms #4118

Add full support for SHA-256 and SHA-512-256 digest algorithms #4118

gtjoseph commented Oct 28, 2024 •

edited

Loading

gtjoseph commented Oct 28, 2024

gtjoseph commented Oct 28, 2024

gtjoseph commented Oct 29, 2024

gtjoseph commented Oct 30, 2024

gtjoseph commented Oct 30, 2024

wosrediinanatour Nov 2, 2024

gtjoseph Nov 4, 2024

wosrediinanatour Nov 2, 2024

gtjoseph Nov 4, 2024

nanangizz Nov 4, 2024

gtjoseph Nov 4, 2024

nanangizz Nov 4, 2024

gtjoseph Nov 4, 2024

nanangizz Nov 5, 2024

Add full support for SHA-256 and SHA-512-256 digest algorithms #4118

Add full support for SHA-256 and SHA-512-256 digest algorithms #4118

Conversation

gtjoseph commented Oct 28, 2024 • edited Loading

gtjoseph commented Oct 28, 2024

gtjoseph commented Oct 28, 2024

gtjoseph commented Oct 29, 2024

gtjoseph commented Oct 30, 2024

gtjoseph commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtjoseph commented Oct 28, 2024 •

edited

Loading