So here is the cleanup of filemode.c and also a bit of cleanup in parse.c
This should also fix a few bugs in parse_load_certchain() (mainly
memleaks).
--
:wq Claudio
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.70
diff -u -p -r1.70 cert.c
--- cert.c 21 Apr 2022 09:53:07 -0000 1.70
+++ cert.c 21 Apr 2022 10:45:17 -0000
@@ -999,6 +999,9 @@ out:
struct cert *
cert_parse(const char *fn, struct cert *p)
{
+ if (p == NULL)
+ return NULL;
+
if (p->aki == NULL) {
warnx("%s: RFC 6487 section 8.4.2: "
"non-trust anchor missing AKI", fn);
@@ -1032,6 +1035,9 @@ ta_parse(const char *fn, struct cert *p,
ASN1_TIME *notBefore, *notAfter;
EVP_PKEY *pk, *opk;
+ if (p == NULL)
+ return NULL;
+
/* first check pubkey against the one from the TAL */
pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
if (pk == NULL) {
@@ -1207,7 +1213,7 @@ auth_find(struct auth_tree *auths, const
return RB_FIND(auth_tree, auths, &a);
}
-void
+struct auth *
auth_insert(struct auth_tree *auths, struct cert *cert, struct auth *parent)
{
struct auth *na;
@@ -1221,6 +1227,8 @@ auth_insert(struct auth_tree *auths, str
if (RB_INSERT(auth_tree, auths, na) != NULL)
err(1, "auth tree corrupted");
+
+ return na;
}
static void
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.131
diff -u -p -r1.131 extern.h
--- extern.h 21 Apr 2022 09:53:07 -0000 1.131
+++ extern.h 21 Apr 2022 10:46:09 -0000
@@ -308,7 +308,7 @@ struct auth {
RB_HEAD(auth_tree, auth);
struct auth *auth_find(struct auth_tree *, const char *);
-void auth_insert(struct auth_tree *, struct cert *, struct auth *);
+struct auth *auth_insert(struct auth_tree *, struct cert *, struct auth *);
enum http_result {
HTTP_FAILED, /* anything else */
Index: filemode.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
retrieving revision 1.1
diff -u -p -r1.1 filemode.c
--- filemode.c 21 Apr 2022 09:53:07 -0000 1.1
+++ filemode.c 21 Apr 2022 10:47:24 -0000
@@ -46,80 +46,6 @@ static struct crl_tree crlt = RB_INITIA
struct tal *talobj[TALSZ_MAX];
/*
- * Validate a certificate, if invalid free the resouces and return NULL.
- */
-static struct cert *
-proc_parser_cert_validate(char *file, struct cert *cert)
-{
- struct auth *a;
- struct crl *crl;
-
- a = valid_ski_aki(file, &auths, cert->ski, cert->aki);
- crl = crl_get(&crlt, a);
-
- if (!valid_x509(file, ctx, cert->x509, a, crl, 0)) {
- cert_free(cert);
- return NULL;
- }
-
- cert->talid = a->cert->talid;
-
- /* Validate the cert */
- if (!valid_cert(file, a, cert)) {
- cert_free(cert);
- return NULL;
- }
-
- /*
- * Add validated CA certs to the RPKI auth tree.
- */
- if (cert->purpose == CERT_PURPOSE_CA)
- auth_insert(&auths, cert, a);
-
- return cert;
-}
-
-/*
- * Root certificates come from TALs (has a pkey and is self-signed).
- * Parse the certificate, ensure that its public key matches the
- * known public key from the TAL, and then validate the RPKI
- * content.
- *
- * This returns a certificate (which must not be freed) or NULL on
- * parse failure.
- */
-static struct cert *
-proc_parser_root_cert(char *file, const unsigned char *der, size_t len,
- unsigned char *pkey, size_t pkeysz, int talid)
-{
- struct cert *cert;
-
- /* Extract certificate data. */
-
- cert = cert_parse_pre(file, der, len);
- if (cert == NULL)
- return NULL;
- cert = ta_parse(file, cert, pkey, pkeysz);
- if (cert == NULL)
- return NULL;
-
- if (!valid_ta(file, &auths, cert)) {
- warnx("%s: certificate not a valid ta", file);
- cert_free(cert);
- return NULL;
- }
-
- cert->talid = talid;
-
- /*
- * Add valid roots to the RPKI auth tree.
- */
- auth_insert(&auths, cert, NULL);
-
- return cert;
-}
-
-/*
* Use the X509 CRL Distribution Points to locate the CRL needed for
* verification.
*/
@@ -207,25 +133,25 @@ parse_load_cert(char *uri)
static void
parse_load_certchain(char *uri)
{
- struct cert *stack[MAX_CERT_DEPTH];
+ struct cert *stack[MAX_CERT_DEPTH] = { 0 };
char *filestack[MAX_CERT_DEPTH];
struct cert *cert;
- int i, failed;
+ struct crl *crl;
+ struct auth *a;
+ int i;
for (i = 0; i < MAX_CERT_DEPTH; i++) {
- cert = parse_load_cert(uri);
- if (cert == NULL) {
+ filestack[i] = uri;
+ stack[i] = cert = parse_load_cert(uri);
+ if (cert == NULL || cert->purpose != CERT_PURPOSE_CA) {
warnx("failed to build authority chain");
- return;
+ goto fail;
}
if (auth_find(&auths, cert->ski) != NULL) {
assert(i == 0);
- cert_free(cert);
- return; /* cert already added */
+ goto fail;
}
- stack[i] = cert;
- filestack[i] = uri;
- if (auth_find(&auths, cert->aki) != NULL)
+ if ((a = auth_find(&auths, cert->aki)) != NULL)
break; /* found chain to TA */
uri = cert->aia;
}
@@ -233,47 +159,65 @@ parse_load_certchain(char *uri)
if (i >= MAX_CERT_DEPTH) {
warnx("authority chain exceeds max depth of %d",
MAX_CERT_DEPTH);
+fail:
for (i = 0; i < MAX_CERT_DEPTH; i++)
cert_free(stack[i]);
return;
}
/* TA found play back the stack and add all certs */
- for (failed = 0; i >= 0; i--) {
+ for (; i >= 0; i--) {
cert = stack[i];
uri = filestack[i];
- if (failed)
- cert_free(cert);
- else if (proc_parser_cert_validate(uri, cert) == NULL)
- failed = 1;
+ crl = crl_get(&crlt, a);
+ if (valid_x509(uri, ctx, cert->x509, a, crl, 0) &&
+ valid_cert(uri, a, cert)) {
+ cert->talid = a->cert->talid;
+ a = auth_insert(&auths, cert, a);
+ } else {
+ for (; i >= 0; i--)
+ cert_free(stack[i]);
+ }
}
}
static void
parse_load_ta(struct tal *tal)
{
- const char *file;
- char *nfile, *f;
+ const char *filename;
+ struct cert *cert;
+ unsigned char *f = NULL;
+ char *file;
size_t flen;
/* does not matter which URI, all end with same filename */
- file = strrchr(tal->uri[0], '/');
- assert(file);
+ filename = strrchr(tal->uri[0], '/');
+ assert(filename);
- if (asprintf(&nfile, "ta/%s%s", tal->descr, file) == -1)
+ if (asprintf(&file, "ta/%s%s", tal->descr, filename) == -1)
err(1, NULL);
- f = load_file(nfile, &flen);
+ f = load_file(file, &flen);
if (f == NULL) {
- warn("parse file %s", nfile);
- free(nfile);
- return;
+ warn("parse file %s", file);
+ goto out;
}
- /* if TA is valid it was added as a root which is all we need */
- proc_parser_root_cert(nfile, f, flen, tal->pkey, tal->pkeysz, tal->id);
- free(nfile);
+ /* Extract certificate data. */
+ cert = cert_parse_pre(file, f, flen);
+ cert = ta_parse(file, cert, tal->pkey, tal->pkeysz);
+ if (cert == NULL)
+ goto out;
+
+ cert->talid = tal->id;
+
+ if (!valid_ta(file, &auths, cert))
+ cert_free(cert);
+ else
+ auth_insert(&auths, cert, NULL);
+out:
+ free(file);
free(f);
}
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.72
diff -u -p -r1.72 parser.c
--- parser.c 21 Apr 2022 09:53:07 -0000 1.72
+++ parser.c 21 Apr 2022 10:45:53 -0000
@@ -389,30 +389,37 @@ proc_parser_mft(struct entity *entp, str
}
/*
- * Validate a certificate, if invalid free the resouces and return NULL.
+ * Certificates are from manifests (has a digest and is signed with
+ * another certificate) Parse the certificate, make sure its
+ * signatures are valid (with CRLs), then validate the RPKI content.
+ * This returns a certificate (which must not be freed) or NULL on
+ * parse failure.
*/
static struct cert *
-proc_parser_cert_validate(char *file, struct cert *cert)
+proc_parser_cert(char *file, const unsigned char *der, size_t len)
{
- struct auth *a;
+ struct cert *cert;
struct crl *crl;
+ struct auth *a;
+
+ /* Extract certificate data. */
+
+ cert = cert_parse_pre(file, der, len);
+ cert = cert_parse(file, cert);
+ if (cert == NULL)
+ return NULL;
a = valid_ski_aki(file, &auths, cert->ski, cert->aki);
crl = crl_get(&crlt, a);
- if (!valid_x509(file, ctx, cert->x509, a, crl, 0)) {
+ if (!valid_x509(file, ctx, cert->x509, a, crl, 0) ||
+ !valid_cert(file, a, cert)) {
cert_free(cert);
return NULL;
}
cert->talid = a->cert->talid;
- /* Validate the cert */
- if (!valid_cert(file, a, cert)) {
- cert_free(cert);
- return NULL;
- }
-
/*
* Add validated CA certs to the RPKI auth tree.
*/
@@ -423,31 +430,6 @@ proc_parser_cert_validate(char *file, st
}
/*
- * Certificates are from manifests (has a digest and is signed with
- * another certificate) Parse the certificate, make sure its
- * signatures are valid (with CRLs), then validate the RPKI content.
- * This returns a certificate (which must not be freed) or NULL on
- * parse failure.
- */
-static struct cert *
-proc_parser_cert(char *file, const unsigned char *der, size_t len)
-{
- struct cert *cert;
-
- /* Extract certificate data. */
-
- cert = cert_parse_pre(file, der, len);
- if (cert == NULL)
- return NULL;
- cert = cert_parse(file, cert);
- if (cert == NULL)
- return NULL;
-
- cert = proc_parser_cert_validate(file, cert);
- return cert;
-}
-
-/*
* Root certificates come from TALs (has a pkey and is self-signed).
* Parse the certificate, ensure that its public key matches the
* known public key from the TAL, and then validate the RPKI
@@ -465,8 +447,6 @@ proc_parser_root_cert(char *file, const
/* Extract certificate data. */
cert = cert_parse_pre(file, der, len);
- if (cert == NULL)
- return NULL;
cert = ta_parse(file, cert, pkey, pkeysz);
if (cert == NULL)
return NULL;
