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

[Kevin Harwell]

Kevin Harwell 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 2: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/c/asterisk/+/14031/2/include/asterisk/res_stir_shaken.h 
File include/asterisk/res_stir_shaken.h:

https://gerrit.asterisk.org/c/asterisk/+/14031/2/include/asterisk/res_stir_shaken.h@21 
PS2, Line 21: #include "asterisk/json.h"
Move this to the .c file and forward declare ast_json


https://gerrit.asterisk.org/c/asterisk/+/14031/2/include/asterisk/res_stir_shaken.h@26 
PS2, Line 26: struct ast_stir_shaken_payload {
            : 	/*! The JWT header */
            : 	struct ast_json *header;
            : 	/*! The JWT payload */
            : 	struct ast_json *payload;
            : 	/*! Signature for the payload */
            : 	unsigned char *signature;
            : 	/*! The algorithm used */
            : 	char *algorithm;
            : 	/*! THe URL to the public key for the certificate */
            : 	char *public_key_url;
            : };
make this opaque


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

https://gerrit.asterisk.org/c/asterisk/+/14031/2/res/res_stir_shaken/certificate.c@100 
PS2, Line 100: 		return cert->public_key_url;
This seems unsafe unless the cert object lifetime is guaranteed past the return? A pointer to the string is returned, but if the underlying object goes away it will then point to "trash", which will result in a crash (probably).


https://gerrit.asterisk.org/c/asterisk/+/14031/2/res/res_stir_shaken/certificate.c@110 
PS2, Line 110: 	cert = get_by_caller_id_number(caller_id_number);
             : 	if (cert) {
             : 		return cert->private_key;
Same for this. A pointer is returned, but if the underlying object goes away the sadness.



-- 
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: 2
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Corey Farrell <git at cfware.com>
Gerrit-Comment-Date: Tue, 31 Mar 2020 21:27:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200331/87fc32ed/attachment.html>