[sr-dev] git:master:043ce0e7: stirshaken: Properly handle intermediary/chain certificates when caching certificates

[Henning Westerholt]

Module: kamailio
Branch: master
Commit: 043ce0e75eae04f356cd539f2146df6846a169e2
URL: https://github.com/kamailio/kamailio/commit/043ce0e75eae04f356cd539f2146df6846a169e2

Author: Trevor Peirce <trev at acrovoice.ca>
Committer: Henning Westerholt <hw at gilawa.com>
Date: 2022-12-12T07:48:38+01:00

stirshaken: Properly handle intermediary/chain certificates when caching certificates

- requires patch to libstirshaken (PR 124) to do anything
- if patched version of libstirshaken detected, uses new methods to store all intermediary certs
- unrelated minor logging tweaks

---

Modified: src/modules/stirshaken/stirshaken_mod.c

---

Diff:  https://github.com/kamailio/kamailio/commit/043ce0e75eae04f356cd539f2146df6846a169e2.diff
Patch: https://github.com/kamailio/kamailio/commit/043ce0e75eae04f356cd539f2146df6846a169e2.patch

---

diff --git a/src/modules/stirshaken/stirshaken_mod.c b/src/modules/stirshaken/stirshaken_mod.c
index 6f2ec8fa03e..b64396b0ab7 100644
--- a/src/modules/stirshaken/stirshaken_mod.c
+++ b/src/modules/stirshaken/stirshaken_mod.c
@@ -207,18 +207,21 @@ static stir_shaken_status_t shaken_callback(stir_shaken_callback_arg_t *arg)
 
 					diff = now_s - attr.st_mtime;
 
-					LM_DBG("Checking cached certificate against expiration setting of %zus (now is: %zu, file modification timestamp is: %zu, difference is: %zu)\n",
+					LM_DBG("Checking cached certificate against expiration setting of %zus (now is: %lu, file modification timestamp is: %lu, difference is: %lu)\n",
 						stirshaken_vs_cache_expire_s, now_s, attr.st_mtime, diff);
 
 					if (diff > stirshaken_vs_cache_expire_s) {
-						LM_WARN("Cached certificate %s is behind expiration threshold (%zu > %zu). Need to download new certificate...\n", cert_full_path, diff, stirshaken_vs_cache_expire_s);
+						LM_NOTICE("Cached certificate %s is behind expiration threshold (%lu > %zu). Need to download new certificate...\n", cert_full_path, diff, stirshaken_vs_cache_expire_s);
 						goto exit;
 					} else {
-						LM_WARN("Cached certificate %s is valid for next %zus\n", cert_full_path, stirshaken_vs_cache_expire_s - diff);
+						LM_NOTICE("Cached certificate %s is valid for next %lus\n", cert_full_path, stirshaken_vs_cache_expire_s - diff);
 					}
 				}
-
+#ifdef STIR_SHAKEN_CAN_RW_X509_FULLCHAIN
+				if (STIR_SHAKEN_STATUS_OK != stir_shaken_load_x509_from_file_fullchain(&ss, &cache_copy, cert_full_path)) {
+#else
 				if (!(cache_copy.x = stir_shaken_load_x509_from_file(&ss, cert_full_path))) {
+#endif
 					LM_ERR("Cannot load X509 from file %s\n", cert_full_path);
 					goto exit;
 				}
@@ -443,10 +446,14 @@ static int stirshaken_handle_cache(stir_shaken_context_t *ss, stir_shaken_passpo
 			}
 		}
 
-		LM_DBG("Saving fresh certificate %s in cache (with name: %s)...\n", x5u, cert_full_path);
-
+#ifdef STIR_SHAKEN_CAN_RW_X509_FULLCHAIN
+		LM_DBG("Saving fresh certificate+chain %s to cache as %s\n", x5u, cert_full_path);
+		if (STIR_SHAKEN_STATUS_OK != stir_shaken_x509_to_disk_fullchain(ss, cert->x, cert->xchain, cert_full_path)) {
+#else
+		LM_DBG("Saving fresh certificate %s to cache as %s\n", x5u, cert_full_path);
 		if (STIR_SHAKEN_STATUS_OK != stir_shaken_x509_to_disk(ss, cert->x, cert_full_path)) {
-			LM_ERR("Failed to write cert %s to disk (as: %s)", x5u, cert_full_path);
+#endif
+			LM_ERR("Failed to cache certificate %s to disk", x5u);
 		}
 
 	} else {
@@ -485,7 +492,8 @@ static int ki_stirshaken_check_identity(sip_msg_t *msg)
 	ibody = hf->body;
 
 	if (STIR_SHAKEN_STATUS_OK != stir_shaken_vs_sih_verify(&ss, vs, ibody.s, &cert_out, &passport_out)) {
-		LM_ERR("SIP Identity Header did not pass verification\n");
+		LM_ERR("SIP Identity Header did not pass verification: %s", stir_shaken_get_error(&ss, NULL));
+
 		stirshaken_print_error_details(&ss);
 		goto fail;
 	}