[Asterisk-code-review] STIR/SHAKEN: Fix certificate type and storage. (asterisk[16])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15822 )

Change subject: STIR/SHAKEN: Fix certificate type and storage.
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.asterisk.org/c/asterisk/+/15822/3/configs/samples/stir_shaken.conf.sample 
File configs/samples/stir_shaken.conf.sample:

https://gerrit.asterisk.org/c/asterisk/+/15822/3/configs/samples/stir_shaken.conf.sample@25 
PS3, Line 25: ; A certificate store is used to examine, and load all certificates found in a
            : ; given directory. When using this type the public key URL is generated based
            : ; upon the filename, and variable substitution.
> This functionality isn't yet implemented (we've got an issue for it), but these will be for our own storage, not the ones coming in since we don't have any control over that.

Not sure I understand about is not having control.  Don't we decide where to put certs we've retrieved?


https://gerrit.asterisk.org/c/asterisk/+/15822/3/configs/samples/stir_shaken.conf.sample@51 
PS3, Line 51: ; URL to the public certificate. Must be of type X509. This will be put in the identity header
            : ; when signing.
            : ;public_key_url=http://mycompany.com/alice.pem
> This is different from path. Path is the location of the private cert that we use to sign on outbound. The public_key_url is what we put in the Identity header to let the remote end know where to download the public certificate.

Yep, I meant to say that the key has to be the one that generated the cert.
I'm still confuses as to how this relates to both path and public_key_url in the store.


https://gerrit.asterisk.org/c/asterisk/+/15822/3/res/res_stir_shaken.c 
File res/res_stir_shaken.c:

https://gerrit.asterisk.org/c/asterisk/+/15822/3/res/res_stir_shaken.c@618 
PS3, Line 618: 	if (public_key_is_expired(public_key_url)) {
> Yeah, Richard is right here. There's a chance that it could be expired on the server itself. Nothing to do with us in that case, but a valid scenario.

Yeah this is confusing.  Expired refers to the cache settings in the HTTP response headers, should the sender choose to add them.  It's like a DNS TTL.  'Retrieve from us once an hour in case we replaced the cert".  The other check is the valid dates of the cert we received in the HTTP response payload. It could also be "expired".  In fact, it could expire while we have it cached. 😊

Maybe they should be renamed to cache_expiry and cert_expiry to avoid any further confusion.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ia00b20835f5f976e3603797f2f2fb19672d8114d
Gerrit-Change-Number: 15822
Gerrit-PatchSet: 3
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-CC: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Mon, 03 May 2021 12:10:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: George Joseph <gjoseph at digium.com>
Comment-In-Reply-To: Benjamin Keith Ford <bford at digium.com>
Comment-In-Reply-To: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210503/3b24b7e8/attachment-0001.html>