[Asterisk-code-review] res_pjsip: Add mediasec capabilities. (asterisk[16])

[Maximilian Fridrich]

Attention is currently required from: Joshua Colp, N A.
Maximilian Fridrich has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18837 )

Change subject: res_pjsip: Add mediasec capabilities.
......................................................................


Patch Set 5:

(11 comments)

File res/res_pjsip/security_agreements.c:

https://gerrit.asterisk.org/c/asterisk/+/18837/comment/107c433c_46ab75b4 
PS4, Line 37: 	mech = ast_malloc(sizeof(struct ast_sip_security_mechanism));
> If you use calloc then it will be automatically zeroed out.
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/a3b40b7f_ace5976c 
PS4, Line 44: 	  ast_free(mech);
> Indentation
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/c173b69c_97375e7b 
PS4, Line 60: 	dst = ast_sip_security_mechanisms_alloc(n_params);
> This can fail
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/72538985_0d4ce3d6 
PS4, Line 89: 	if (AST_VECTOR_SIZE(dst) != 0) {
> Indentation
Done


File res/res_pjsip_outbound_registration.c:

https://gerrit.asterisk.org/c/asterisk/+/18837/comment/8e75e08b_b36154f3 
PS4, Line 596: 		(aor = ast_sip_location_retrieve_aor(endpt->aors)) &&
> What happens if there are multiple AORs and contacts? The behavior should be documented somewhere
Your comment made me realize that the endpoint would be the better place to store this information, so I refactored it and store the server security mechanisms in the endpoint now. The many-to-many relationships between endpoint and aors, and aors and contacts were a huge drawback.

The sip_session_supplement called on outgoing requests via this outbound registration are called with this endpoint, so it works.


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/7a733961_ca98d96c 
PS4, Line 609: 	if (client_state->status == SIP_REGISTRATION_REGISTERED || client_state->status == SIP_REGISTRATION_REJECTED_TEMPORARY 
> Redness
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/1f3ac888_a3cf74ec 
PS4, Line 632: 		ao2_ref(endpt, -1);
> Use ao2_cleanup
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/b87136c6_6eda6241 
PS4, Line 635: 		ao2_ref(reg, -1);
> Use ao2_cleanup
Done


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/2285591e_2ffa5238 
PS4, Line 1113: 			if (endpt) {
> Same applies here as above
Done


File res/res_pjsip_rfc3329.c:

https://gerrit.asterisk.org/c/asterisk/+/18837/comment/eb7988fd_83563ed6 
PS4, Line 74: 	if (contact_status) {
> It's impossible for contact_status to be NULL here
Refactored. contact_status is no longer used.


https://gerrit.asterisk.org/c/asterisk/+/18837/comment/06d23a12_b6555c2b 
PS4, Line 110: 	if (contact_status) {
> It's impossible for contact_status to be NULL here
Refactored. contact_status is no longer used.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18837
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ia7f5b5ba42db18074fdd5428c4e1838728586be2
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Fridrich <m.fridrich at commend.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Thu, 18 Aug 2022 12:03:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220818/5bc6e8da/attachment-0001.html>