Patch bad initialization of CRL stack

diff --git a/docs/doc/installation.md b/docs/doc/installation.md
index 06be678593634273e4c55cbdf31f55566fcb664d..6dd3ee749968983c572fffce10de353ff54e9915 100644 (file)
--- a/docs/doc/installation.md
+++ b/docs/doc/installation.md
@@ -6,8 +6,6 @@ title: Compilation and Installation
 
 # {{ page.title }}
 
-> TODO update with proper .tar.gz releases, once they are created
-
 ## Index
 
 1. [Dependencies](#dependencies)

diff --git a/src/object/certificate.c b/src/object/certificate.c
index bbffe29aa06024bb0a89896a12127de1aaf53246..e3600c69755a1ac72bf59a8c6a19c5c0bcfc404b 100644 (file)
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -1386,6 +1386,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri)
 
        struct validation *state;
        int total_parents;
+       STACK_OF(X509_CRL) *rpp_parent_crl;
        X509 *cert;
        struct rpki_uri *mft;
        struct certificate_refs refs;
@@ -1405,11 +1406,15 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri)
        fnstack_push_uri(cert_uri);
        memset(&refs, 0, sizeof(refs));
 
+       error = rpp_crl(rpp_parent, &rpp_parent_crl);
+       if (error)
+               goto revert_fnstack_and_debug;
+
        /* -- Validate the certificate (@cert) -- */
        error = certificate_load(cert_uri, &cert);
        if (error)
                goto revert_fnstack_and_debug;
-       error = certificate_validate_chain(cert, rpp_crl(rpp_parent));
+       error = certificate_validate_chain(cert, rpp_parent_crl);
        if (error)
                goto revert_cert;
        error = certificate_validate_rfc6487(cert, IS_TA);
@@ -1432,7 +1437,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri)
                goto revert_uri_and_refs;
        cert = NULL; /* Ownership stolen */
 
-       error = handle_manifest(mft, rpp_crl(rpp_parent), &pp);
+       error = handle_manifest(mft, rpp_parent_crl, &pp);
        if (error) {
                x509stack_cancel(validation_certstack(state));
                goto revert_uri_and_refs;

diff --git a/src/object/ghostbusters.c b/src/object/ghostbusters.c
index 9ace5baa302c34ca3ac6b1112c19ae7d4589fefc..42f82ca0fffdd9fdddb815588f3d69e9ab82db61 100644 (file)
--- a/src/object/ghostbusters.c
+++ b/src/object/ghostbusters.c
@@ -18,12 +18,17 @@ ghostbusters_traverse(struct rpki_uri *uri, struct rpp *pp)
        static OID oid = OID_GHOSTBUSTERS;
        struct oid_arcs arcs = OID2ARCS("ghostbusters", oid);
        struct signed_object_args sobj_args;
+       STACK_OF(X509_CRL) *crl;
        int error;
 
        pr_debug_add("Ghostbusters '%s' {", uri_get_printable(uri));
        fnstack_push_uri(uri);
 
-       error = signed_object_args_init(&sobj_args, uri, rpp_crl(pp), true);
+       error = rpp_crl(pp, &crl);
+       if (error)
+               goto end1;
+
+       error = signed_object_args_init(&sobj_args, uri, crl, true);
        if (error)
                goto end1;
 

diff --git a/src/object/roa.c b/src/object/roa.c
index 10919eb1c39e6557d6333f48d7429b77838a3f1c..bc3862656c12a9c5bc356c29d7a4dd786df1bade 100644 (file)
--- a/src/object/roa.c
+++ b/src/object/roa.c
@@ -191,12 +191,17 @@ roa_traverse(struct rpki_uri *uri, struct rpp *pp)
        struct oid_arcs arcs = OID2ARCS("roa", oid);
        struct signed_object_args sobj_args;
        struct RouteOriginAttestation *roa;
+       STACK_OF(X509_CRL) *crl;
        int error;
 
        pr_debug_add("ROA '%s' {", uri_get_printable(uri));
        fnstack_push_uri(uri);
 
-       error = signed_object_args_init(&sobj_args, uri, rpp_crl(pp), false);
+       error = rpp_crl(pp, &crl);
+       if (error)
+               goto revert_fnstack;
+
+       error = signed_object_args_init(&sobj_args, uri, crl, false);
        if (error)
                goto revert_fnstack;
 

diff --git a/src/rpp.c b/src/rpp.c
index 50160bbed5d72b74e766ddf94a044ff0456bcacf..4f1b5a278efc930ae8745202e7787f951d995a09 100644 (file)
--- a/src/rpp.c
+++ b/src/rpp.c
@@ -145,25 +145,54 @@ end:
        return error;
 }
 
-STACK_OF(X509_CRL) *
-rpp_crl(struct rpp *pp)
+/**
+ * Returns the pp's CRL in stack form (which is how libcrypto functions want
+ * it).
+ * The stack belongs to @pp and should not be released. Can be NULL, in which
+ * case you're currently validating the TA (since it lacks governing CRL).
+ */
+int
+rpp_crl(struct rpp *pp, STACK_OF(X509_CRL) **result)
 {
-       if (pp == NULL)
-               return NULL;
-       if (pp->crl == NULL)
-               return NULL;
-       if (pp->crl_stack != NULL)
-               return pp->crl_stack;
+       STACK_OF(X509_CRL) *stack;
+       int error;
 
-       pp->crl_stack = sk_X509_CRL_new_null();
-       if (pp->crl_stack == NULL)
-               return NULL;
-       if (add_crl_to_stack(pp, pp->crl_stack) != 0) {
-               sk_X509_CRL_pop_free(pp->crl_stack, X509_CRL_free);
-               return NULL;
+       if (pp == NULL) {
+               /* No pp = currently validating TA. There's no CRL. */
+               *result = NULL;
+               return 0;
+       }
+       if (pp->crl == NULL) {
+               /* rpp_crl() assumes the rpp has been populated already. */
+               pr_crit("RPP lacks a CRL.");
+       }
+       if (pp->crl_stack != NULL) {
+               /* Result already cached. */
+               *result = pp->crl_stack;
+               return 0;
+       }
+
+       /*
+        * TODO (performance) ghostbusters_traverse() and roa_traverse() can
+        * call rpp_crl() repeatedly, which means that, if it errors here,
+        * it will keep trying to recreate the stack and log the same error
+        * over and over.
+        * Consider creating a flag which will keep track of the CRL cache
+        * status, and prevent this code if it knows it's going to fail.
+        * Or maybe some other solution.
+        */
+       stack = sk_X509_CRL_new_null();
+       if (stack == NULL)
+               return pr_enomem();
+       error = add_crl_to_stack(pp, stack);
+       if (error) {
+               sk_X509_CRL_pop_free(stack, X509_CRL_free);
+               return error;
        }
 
-       return pp->crl_stack;
+       pp->crl_stack = stack; /* Cache result */
+       *result = stack;
+       return 0;
 }
 
 static int

diff --git a/src/rpp.h b/src/rpp.h
index 3b310c2448ebb1316550d9f0a7b183c01f23ecdb..546ad684d7e72713153b89656c5dd17e3242932b 100644 (file)
--- a/src/rpp.h
+++ b/src/rpp.h
@@ -15,7 +15,7 @@ int rpp_add_roa(struct rpp *, struct rpki_uri *);
 int rpp_add_ghostbusters(struct rpp *, struct rpki_uri *);
 
 struct rpki_uri *rpp_get_crl(struct rpp const *);
-STACK_OF(X509_CRL) *rpp_crl(struct rpp *);
+int rpp_crl(struct rpp *, STACK_OF(X509_CRL) **);
 
 void rpp_traverse(struct rpp *);