Skip to content

Set default pjsip_cred_info.auth_algorithm to PJSIP_AUTH_ALGORITHM_MD5#4195

Merged
sauwming merged 1 commit intopjsip:masterfrom
asterisk:master-default-auth-algorithm
Dec 6, 2024
Merged

Set default pjsip_cred_info.auth_algorithm to PJSIP_AUTH_ALGORITHM_MD5#4195
sauwming merged 1 commit intopjsip:masterfrom
asterisk:master-default-auth-algorithm

Conversation

@gtjoseph
Copy link
Copy Markdown
Contributor

@gtjoseph gtjoseph commented Dec 4, 2024

The new algorithm_type field in the pjsip_cred_info structure wasn't being
defaulted to PJSIP_AUTH_ALGORITHM_MD5 as it should be. As a result if the
user specified a data_type of PJSIP_CRED_DATA_DIGEST but didn't explicitly
set algorithm_type, it was left at PJSIP_AUTH_ALGORITHM_NOT_SET which
will caused authentication to fail.

  • pjsip_cred_info.auth_algorithm is now correctly defaulted to
    PJSIP_AUTH_ALGORITHM_MD5 when supplied via the pjsip_auth_lookup_cred and
    pjsip_auth_lookup_cred2 callbacks and via the pjsip_auth_clt_set_credentials,
    and pjsip_auth_create_digest functions.

  • pjsip_cred_info.auth_algorithm now defaults to PJSIP_AUTH_ALGORITHM_SHA256
    for the pjsip_auth_create_digestSHA256 function.

  • Documentation was updated for those functions and callbacks to indicate the
    defaults.

NOTE: The pjsip_auth_create_digest and pjsip_auth_create_digestSHA256
defaults are actually set in pjsip_auth_create_digest2 because those two
functions are now deprecated wrappers that pass cred_info (which is a const) to
pjsip_auth_create_digest2. The documentation for pjsip_auth_create_digest2
however, doesn't mention defaults on purpose because it's a generic function
that handles multiple algorithms and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity.

Resolves: #4194

@nanangizz
Copy link
Copy Markdown
Member

This behavior seems to be specified in pjsip_auth_create_digest2() declaration:

* \note If left unset (0), pjsip_cred_info::algorithm_type will
* default to #PJSIP_AUTH_ALGORITHM_MD5.
)
while the 'defaulting' action seems to be done in pjsip_auth_clt_set_credentials(), so I think the specification needs to be moved to pjsip_auth_clt_set_credentials()?
Also, perhaps the spec should mention about "when data_type is set to PJSIP_CRED_DATA_DIGEST".

The new algorithm_type field in the pjsip_cred_info structure wasn't being
defaulted to PJSIP_AUTH_ALGORITHM_MD5 as it should be. As a result if the
user specified a data_type of PJSIP_CRED_DATA_DIGEST but didn't explicitly
set algorithm_type, it was left at PJSIP_AUTH_ALGORITHM_NOT_SET which
will caused authentication to fail.

* pjsip_cred_info.auth_algorithm is now correctly defaulted to
PJSIP_AUTH_ALGORITHM_MD5 when supplied via the pjsip_auth_lookup_cred and
pjsip_auth_lookup_cred2 callbacks and via the pjsip_auth_clt_set_credentials,
and pjsip_auth_create_digest functions.

* pjsip_cred_info.auth_algorithm now defaults to PJSIP_AUTH_ALGORITHM_SHA256
for the pjsip_auth_create_digestSHA256 function.

* Documentation was updated for those functions and callbacks to indicate the
defaults.

NOTE: The pjsip_auth_create_digest and pjsip_auth_create_digestSHA256
defaults are actually set in pjsip_auth_create_digest2 because those two
functions are now deprecated wrappers that pass cred_info (which is a const) to
pjsip_auth_create_digest2.  The documentation for pjsip_auth_create_digest2
however, doesn't mention defaults on purpose because it's a generic function
that handles multiple algorithms and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity.

Resolves: pjsip#4194
@gtjoseph gtjoseph force-pushed the master-default-auth-algorithm branch from 866d3af to c24463b Compare December 5, 2024 14:57
@gtjoseph
Copy link
Copy Markdown
Contributor Author

gtjoseph commented Dec 5, 2024

@nanangizz I updated the documentation, PR description and commit message. Since the pjsip_auth_create_digest* functions are public, I also updated them. See the "NOTE" in the PR description and see if that makes more sense.

@nanangizz
Copy link
Copy Markdown
Member

... and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity

Agree. Honestly I was wondering why not just enforce it, e.g: pjsip_auth_clt_set_credentials() returns error when pjsip_cred_info::algorithm_type is not set (or even when it is set but not supported).

@sauwming sauwming merged commit 4df9467 into pjsip:master Dec 6, 2024
@gtjoseph
Copy link
Copy Markdown
Contributor Author

gtjoseph commented Dec 6, 2024

... and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity

Agree. Honestly I was wondering why not just enforce it, e.g: pjsip_auth_clt_set_credentials() returns error when pjsip_cred_info::algorithm_type is not set (or even when it is set but not supported).

I didn't want to break backwards compatibility (any more than I already did). :)

nanangizz pushed a commit that referenced this pull request Dec 16, 2024
#4195)

The new algorithm_type field in the pjsip_cred_info structure wasn't being
defaulted to PJSIP_AUTH_ALGORITHM_MD5 as it should be. As a result if the
user specified a data_type of PJSIP_CRED_DATA_DIGEST but didn't explicitly
set algorithm_type, it was left at PJSIP_AUTH_ALGORITHM_NOT_SET which
will caused authentication to fail.

* pjsip_cred_info.auth_algorithm is now correctly defaulted to
PJSIP_AUTH_ALGORITHM_MD5 when supplied via the pjsip_auth_lookup_cred and
pjsip_auth_lookup_cred2 callbacks and via the pjsip_auth_clt_set_credentials,
and pjsip_auth_create_digest functions.

* pjsip_cred_info.auth_algorithm now defaults to PJSIP_AUTH_ALGORITHM_SHA256
for the pjsip_auth_create_digestSHA256 function.

* Documentation was updated for those functions and callbacks to indicate the
defaults.

NOTE: The pjsip_auth_create_digest and pjsip_auth_create_digestSHA256
defaults are actually set in pjsip_auth_create_digest2 because those two
functions are now deprecated wrappers that pass cred_info (which is a const) to
pjsip_auth_create_digest2.  The documentation for pjsip_auth_create_digest2
however, doesn't mention defaults on purpose because it's a generic function
that handles multiple algorithms and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity.

Resolves: #4194
BarryYin pushed a commit to BarryYin/pjproject that referenced this pull request Feb 3, 2026
pjsip#4195)

The new algorithm_type field in the pjsip_cred_info structure wasn't being
defaulted to PJSIP_AUTH_ALGORITHM_MD5 as it should be. As a result if the
user specified a data_type of PJSIP_CRED_DATA_DIGEST but didn't explicitly
set algorithm_type, it was left at PJSIP_AUTH_ALGORITHM_NOT_SET which
will caused authentication to fail.

* pjsip_cred_info.auth_algorithm is now correctly defaulted to
PJSIP_AUTH_ALGORITHM_MD5 when supplied via the pjsip_auth_lookup_cred and
pjsip_auth_lookup_cred2 callbacks and via the pjsip_auth_clt_set_credentials,
and pjsip_auth_create_digest functions.

* pjsip_cred_info.auth_algorithm now defaults to PJSIP_AUTH_ALGORITHM_SHA256
for the pjsip_auth_create_digestSHA256 function.

* Documentation was updated for those functions and callbacks to indicate the
defaults.

NOTE: The pjsip_auth_create_digest and pjsip_auth_create_digestSHA256
defaults are actually set in pjsip_auth_create_digest2 because those two
functions are now deprecated wrappers that pass cred_info (which is a const) to
pjsip_auth_create_digest2.  The documentation for pjsip_auth_create_digest2
however, doesn't mention defaults on purpose because it's a generic function
that handles multiple algorithms and we want users to specifify exactly what
algorithms they want to use to avoid ambiguity.

Resolves: pjsip#4194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default auth algorithm isn't being set when pjsip_cred_info.data_type is PJSIP_CRED_DATA_DIGEST

4 participants