Set default pjsip_cred_info.auth_algorithm to PJSIP_AUTH_ALGORITHM_MD5 #4195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sauwming
approved these changes
Dec 5, 2024
trengginas
approved these changes
Dec 5, 2024
|
This behavior seems to be specified in pjproject/pjsip/include/pjsip/sip_auth.h Lines 797 to 798 in 8117bc5 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
force-pushed
the
master-default-auth-algorithm
branch
from
866d3af to
c24463b
Compare
|
@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. |
Agree. Honestly I was wondering why not just enforce it, e.g: |
nanangizz
approved these changes
Dec 6, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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