<p></p>
<p><b>@piotrgregor</b> requested changes on this pull request.</p>

<hr>

<p>In <a href="https://github.com/kamailio/kamailio/pull/3175#discussion_r912953694">src/modules/stirshaken/stirshaken_mod.c</a>:</p>
<pre style='color:#555'>> @@ -158,6 +157,96 @@ static int get_cert_name_hashed(const char *name, char *buf, int buf_len)
        return 0;
 }
 
+static int stirshaken_handle_cache_to_disk(X509 *x, STACK_OF(X509) *xchain, const char *cert_full_path)
+{
+    int i = 0;
+    int w = 0;
+    FILE *fp = NULL;
+    X509 *xc = NULL;
+
+    if (!x) {
+        LM_ERR("Refusing to save an empty cert\n");
+        return -1;
+    }
</pre>
<p dir="auto">Please also check <code class="notranslate">xchain</code> for NULL and call <code class="notranslate">sk_X509_num</code> only if it's not.</p>

<hr>

<p>In <a href="https://github.com/kamailio/kamailio/pull/3175#discussion_r912954696">src/modules/stirshaken/stirshaken_mod.c</a>:</p>
<pre style='color:#555'>> @@ -158,6 +157,96 @@ static int get_cert_name_hashed(const char *name, char *buf, int buf_len)
        return 0;
 }
 
+static int stirshaken_handle_cache_to_disk(X509 *x, STACK_OF(X509) *xchain, const char *cert_full_path)
+{
+    int i = 0;
+    int w = 0;
+    FILE *fp = NULL;
+    X509 *xc = NULL;
+
+    if (!x) {
+        LM_ERR("Refusing to save an empty cert\n");
+        return -1;
+    }
+
</pre>
<p dir="auto">Please also check <code class="notranslate">cert_full_path</code> against NULL pointer or pointer to an empty string.</p>

<hr>

<p>In <a href="https://github.com/kamailio/kamailio/pull/3175#discussion_r912957140">src/modules/stirshaken/stirshaken_mod.c</a>:</p>
<pre style='color:#555'>> +    if (fp) fclose(fp);
+    fp = NULL;
+
+    return 0;
+
+fail:
+       if (fp) fclose(fp);
+       return -1;
+}
+
+static int stirshaken_handle_cache_from_disk(stir_shaken_context_t *ss, stir_shaken_cert_t *cert, const char *name)
+{
+    FILE *fp = NULL;
+    X509 *wcert = NULL;
+
+    LM_DBG("Handle cache from disk; %s", name);
</pre>
<p dir="auto">Please move it below line 214, after <code class="notranslate">stir_shaken_zstr</code> check.</p>

<hr>

<p>In <a href="https://github.com/kamailio/kamailio/pull/3175#discussion_r912957546">src/modules/stirshaken/stirshaken_mod.c</a>:</p>
<pre style='color:#555'>> +
+    if (fp) fclose(fp);
+    fp = NULL;
+
+    return 0;
+
+fail:
+       if (fp) fclose(fp);
+       return -1;
+}
+
+static int stirshaken_handle_cache_from_disk(stir_shaken_context_t *ss, stir_shaken_cert_t *cert, const char *name)
+{
+    FILE *fp = NULL;
+    X509 *wcert = NULL;
+
</pre>
<p dir="auto">Please check <code class="notranslate">cert</code> against NULL.</p>

<hr>

<p>In <a href="https://github.com/kamailio/kamailio/pull/3175#discussion_r912962676">src/modules/stirshaken/stirshaken_mod.c</a>:</p>
<pre style='color:#555'>> +        LM_ERR("Failed to open %s: %s", name, strerror(errno));
+        goto fail;
+    }
+
+    cert->xchain = sk_X509_new_null();
+
+    while ((wcert = PEM_read_X509(fp, NULL, NULL, NULL))) {
+               if (!cert->x) {
+                       cert->x = wcert;
+               }
+               else {
+                       sk_X509_push(cert->xchain, wcert);
+               }
+       }
+
+       LM_DBG("done reading file, got %d certs and %d chains", cert->x ? 1 : 0, sk_X509_num(cert->xchain));
</pre>
<p dir="auto">Let's handle plural/singular form in a log message.<br>
Maybe something like:</p>
<pre class="notranslate"><code class="notranslate">n = sk_X509_num(cert->xchain));
LM_DBG("done reading file, got %d cert%s and %d chain%s", cert->x ? 1 : 0, cert->x ? "", "s", n == 1 ? "" : "s", n);
</code></pre>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/pull/3175#pullrequestreview-1027482224">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABO7UZKBX5DFB6O2FTSIUTLVSLMMLANCNFSM52P2GZPA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/ABO7UZL3VVEZ32FWE6QTOW3VSLMMLA5CNFSM52P2GZPKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOHU7CE4A.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><kamailio/kamailio/pull/3175/review/1027482224</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/3175#pullrequestreview-1027482224",
"url": "https://github.com/kamailio/kamailio/pull/3175#pullrequestreview-1027482224",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>