[Asterisk-code-review] res_stir_shaken: Implemented signing of JSON payload. (asterisk[master])

[Benjamin Keith Ford]

Benjamin Keith Ford has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14031 )

Change subject: res_stir_shaken: Implemented signing of JSON payload.
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.asterisk.org/c/asterisk/+/14031/1/res/res_stir_shaken.c 
File res/res_stir_shaken.c:

https://gerrit.asterisk.org/c/asterisk/+/14031/1/res/res_stir_shaken.c@236 
PS1, Line 236: 	encoded_signature = ast_calloc(1, sizeof(unsigned char) * (signature_length + 1));
> Does this actually provide enough space?
I'm looking into this. From what I can tell, there doesn't appear to be any kind of calculation to determine what the length would be. Should we just allocate a "reasonable" amount of space?


https://gerrit.asterisk.org/c/asterisk/+/14031/1/res/res_stir_shaken.c@324 
PS1, Line 324: 	payload = stir_shaken_verify_json(json);
> If you are signing some parts won't be present which is critical, because they will be added by us. […]
Which parts should it not be checking for? Currently it's checking 'ppt', 'typ', 'alg', 'orign->tn', 'x5u' (public key URL), and of course for the two different sections ('header' and 'payload') in the JWT. I thought the only values added by us included 'attest', 'iat', and 'origid'. I can add or drop any checks that are / aren't necessary


https://gerrit.asterisk.org/c/asterisk/+/14031/1/res/res_stir_shaken.c@329 
PS1, Line 329: 	/* From the payload section of the HSON, get the orig section, and then get
> JSON
I knew I would miss one...


https://gerrit.asterisk.org/c/asterisk/+/14031/1/res/res_stir_shaken.c@370 
PS1, Line 370: 
> The public key URL also needs to be added
The public key URL should already be there as 'x5u' unless I'm misunderstanding



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I74fa41c0640ab2a64a1a80110155bd7062f13393
Gerrit-Change-Number: 14031
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Mon, 30 Mar 2020 15:38:47 +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/20200330/577cbe8c/attachment.html>