[Asterisk-code-review] res_stir_shaken: Add unit tests for signing and verification. (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14408 )

Change subject: res_stir_shaken: Add unit tests for signing and verification.
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

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

https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken.c@1199 
PS1, Line 1199: static void test_stir_shaken_add_fake_astdb_entry(const char *public_key_url, const char *file_path)
              : {
This and all the test methods below should be wrapped in a #ifdef TEST_FRAMEWORK check.


https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken.c@1254 
PS1, Line 1254: 		return -1;
close the file before returning here.


https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken/certificate.h 
File res/res_stir_shaken/certificate.h:

https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken/certificate.h@57 
PS1, Line 57: *!
            :  * \brief Clean up the certificate and mappings set up in test_stir_shaken_init
            :  *
            :  * \param caller_id_number The caller ID of the certificate to clean up
            :  *
            :  * \retval non-zero on failure
            :  * \retval 0 on success
            :  */
            : int test_stir_shaken_cleanup_cert(const char *caller_id_number);
            : 
            : /*!
            :  * \brief Initialize a test certificate through wizard mappings
            :  *
            :  * \note test_stir_shaken_cleanup should be called when done with this certificate
            :  *
            :  * \param caller_id_number The caller ID of the certificate to create
            :  * \param file_path The path to the private key for this certificate
            :  *
            :  * \retval non-zero on failure
            :  * \retval 0 on success
            :  */
            : int test_stir_shaken_create_cert(const char *caller_id_number, const char *file_path);
Since test only code wrap this in an #ifdef TEST_FRAMEWORK


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

https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken/certificate.c@260 
PS1, Line 260: 	cert = stir_shaken_certificate_get_by_caller_id_number(caller_id_number);
             : 	ao2_cleanup(cert);
This doesn't remove it from sorcery. I think you'll still have to call 'ast_sorcery_delete'?


https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken/certificate.c@247 
PS1, Line 247: * Name for test certificaate */
             : #define TEST_CONFIG_NAME "test_stir_shaken_certificate"
             : /* The public key URL to use for the test certificate */
             : #define TEST_CONFIG_URL "http://testing123"
             : 
             : int test_stir_shaken_cleanup_cert(const char *caller_id_number)
             : {
             : 	struct stir_shaken_certificate *cert;
             : 	struct ast_sorcery *sorcery;
             : 	int res = 0;
             : 
             : 	sorcery = ast_stir_shaken_sorcery();
             : 
             : 	cert = stir_shaken_certificate_get_by_caller_id_number(caller_id_number);
             : 	ao2_cleanup(cert);
             : 
             : 	res = ast_sorcery_remove_wizard_mapping(sorcery, CONFIG_TYPE, "memory");
             : 
             : 	return res;
             : }
             : 
             : int test_stir_shaken_create_cert(const char *caller_id_number, const char *file_path)
#ifdef the test stuff.


https://gerrit.asterisk.org/c/asterisk/+/14408/1/res/res_stir_shaken/certificate.c@296 
PS1, Line 296: 		return -1;
free "cert" here before returning.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I9fa43380f861ccf710cd0f6b6c102a517c86ea13
Gerrit-Change-Number: 14408
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Mon, 18 May 2020 23:16:20 +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/20200518/3dfdf0b4/attachment.html>