@piotrgregor requested changes on this pull request.


In src/modules/stirshaken/stirshaken_mod.c:

> @@ -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;
+    }

Please also check xchain for NULL and call sk_X509_num only if it's not.


In src/modules/stirshaken/stirshaken_mod.c:

> @@ -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;
+    }
+

Please also check cert_full_path against NULL pointer or pointer to an empty string.


In src/modules/stirshaken/stirshaken_mod.c:

> +    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);

Please move it below line 214, after stir_shaken_zstr check.


In src/modules/stirshaken/stirshaken_mod.c:

> +
+    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;
+

Please check cert against NULL.


In src/modules/stirshaken/stirshaken_mod.c:

> +        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));

Let's handle plural/singular form in a log message.
Maybe something like:

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);


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <kamailio/kamailio/pull/3175/review/1027482224@github.com>