On Tue, Jan 04, 2022 at 04:57:23PM +0100, Theo Buehler wrote:
> On Tue, Jan 04, 2022 at 04:15:56PM +0100, Claudio Jeker wrote:
> > This is another diff on the way to having a validated repo.
> > Pass the filename of the entity which was parsed back to the parent.
> > With this we can move the filepath_add() call from entity_write_req()
> > to entity_process(). As a side-effect the "Already visited" check is moved
> > after parsing so a file may be reparsed before being ignored. I doubt this
> > causes an issue.
> >
> > On top of this change how entp->file is passed to the individual parser
> > functions. Just pass the filename to those functions. Only exception for
> > now is proc_parser_root_cert() since it accesses more of the entp.
> >
> > Again this is done to make it possible to have the parser decide which
> > path to use for accessing a file. For now this is just shuffling code but
> > once the code has two places to look for a file this will be all needed.
>
> ok tb
>
> This will need a little bit of work on top of the verify_cb() commit.
> Something like
>
> sed -i '/set_app_data/s/entp->/(char *)/' parser.c
>
> should do it (or whichever way you want to deal with the const issue
> arising from the fact X509_STORE_CTX_set_app_data() taking a non-const
> void * as a second argument).
I actually removed the const from the various function prototypes. While
nice it is not needed there and I prefer that over extra casts because of
const.
--
:wq Claudio
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.170
diff -u -p -r1.170 main.c
--- main.c 29 Dec 2021 11:37:57 -0000 1.170
+++ main.c 4 Jan 2022 13:39:31 -0000
@@ -132,12 +132,6 @@ entity_write_req(const struct entity *en
{
struct ibuf *b;
- if (filepath_add(&fpt, ent->file) == 0) {
- warnx("%s: File already visited", ent->file);
- entity_queue--;
- return;
- }
-
b = io_new_buffer();
io_simple_buffer(b, &ent->type, sizeof(ent->type));
io_simple_buffer(b, &ent->talid, sizeof(ent->talid));
@@ -467,6 +461,7 @@ entity_process(struct ibuf *b, struct st
struct cert *cert;
struct mft *mft;
struct roa *roa;
+ char *file;
int c;
/*
@@ -476,6 +471,14 @@ entity_process(struct ibuf *b, struct st
* We follow that up with whether the resources didn't parse.
*/
io_read_buf(b, &type, sizeof(type));
+ io_read_str(b, &file);
+
+ if (filepath_add(&fpt, file) == 0) {
+ warnx("%s: File already visited", file);
+ free(file);
+ entity_queue--;
+ return;
+ }
switch (type) {
case RTYPE_TAL:
@@ -544,6 +547,7 @@ entity_process(struct ibuf *b, struct st
errx(1, "unknown entity type %d", type);
}
+ free(file);
entity_queue--;
}
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.31
diff -u -p -r1.31 parser.c
--- parser.c 4 Jan 2022 15:37:23 -0000 1.31
+++ parser.c 4 Jan 2022 18:17:49 -0000
@@ -109,7 +109,7 @@ verify_cb(int ok, X509_STORE_CTX *store_
* Returns the roa on success, NULL on failure.
*/
static struct roa *
-proc_parser_roa(struct entity *entp, const unsigned char *der, size_t len)
+proc_parser_roa(char *file, const unsigned char *der, size_t len)
{
struct roa *roa;
X509 *x509;
@@ -119,10 +119,10 @@ proc_parser_roa(struct entity *entp, con
STACK_OF(X509_CRL) *crls;
struct crl *crl;
- if ((roa = roa_parse(&x509, entp->file, der, len)) == NULL)
+ if ((roa = roa_parse(&x509, file, der, len)) == NULL)
return NULL;
- a = valid_ski_aki(entp->file, &auths, roa->ski, roa->aki);
+ a = valid_ski_aki(file, &auths, roa->ski, roa->aki);
build_chain(a, &chain);
crl = get_crl(a);
build_crls(crl, &crls);
@@ -131,7 +131,7 @@ proc_parser_roa(struct entity *entp, con
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
cryptoerrx("X509_STORE_CTX_init");
X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
- if (!X509_STORE_CTX_set_app_data(ctx, entp->file))
+ if (!X509_STORE_CTX_set_app_data(ctx, file))
cryptoerrx("X509_STORE_CTX_set_app_data");
X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
@@ -142,8 +142,7 @@ proc_parser_roa(struct entity *entp, con
c = X509_STORE_CTX_get_error(ctx);
X509_STORE_CTX_cleanup(ctx);
if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL)
- warnx("%s: %s", entp->file,
- X509_verify_cert_error_string(c));
+ warnx("%s: %s", file, X509_verify_cert_error_string(c));
X509_free(x509);
roa_free(roa);
sk_X509_free(chain);
@@ -172,7 +171,7 @@ proc_parser_roa(struct entity *entp, con
* the code around roa_read() to check the "valid" field itself.
*/
- if (valid_roa(entp->file, &auths, roa))
+ if (valid_roa(file, &auths, roa))
roa->valid = 1;
sk_X509_free(chain);
@@ -193,7 +192,7 @@ proc_parser_roa(struct entity *entp, con
* Return the mft on success or NULL on failure.
*/
static struct mft *
-proc_parser_mft(struct entity *entp, const unsigned char *der, size_t len)
+proc_parser_mft(char *file, const unsigned char *der, size_t len)
{
struct mft *mft;
X509 *x509;
@@ -201,10 +200,10 @@ proc_parser_mft(struct entity *entp, con
struct auth *a;
STACK_OF(X509) *chain;
- if ((mft = mft_parse(&x509, entp->file, der, len)) == NULL)
+ if ((mft = mft_parse(&x509, file, der, len)) == NULL)
return NULL;
- a = valid_ski_aki(entp->file, &auths, mft->ski, mft->aki);
+ a = valid_ski_aki(file, &auths, mft->ski, mft->aki);
build_chain(a, &chain);
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
@@ -212,7 +211,7 @@ proc_parser_mft(struct entity *entp, con
/* CRL checks disabled here because CRL is referenced from mft */
X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
- if (!X509_STORE_CTX_set_app_data(ctx, entp->file))
+ if (!X509_STORE_CTX_set_app_data(ctx, file))
cryptoerrx("X509_STORE_CTX_set_app_data");
X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
X509_STORE_CTX_set0_trusted_stack(ctx, chain);
@@ -220,7 +219,7 @@ proc_parser_mft(struct entity *entp, con
if (X509_verify_cert(ctx) <= 0) {
c = X509_STORE_CTX_get_error(ctx);
X509_STORE_CTX_cleanup(ctx);
- warnx("%s: %s", entp->file, X509_verify_cert_error_string(c));
+ warnx("%s: %s", file, X509_verify_cert_error_string(c));
mft_free(mft);
X509_free(x509);
sk_X509_free(chain);
@@ -231,7 +230,7 @@ proc_parser_mft(struct entity *entp, con
sk_X509_free(chain);
X509_free(x509);
- if (!mft_check(entp->file, mft)) {
+ if (!mft_check(file, mft)) {
mft_free(mft);
return NULL;
}
@@ -247,7 +246,7 @@ proc_parser_mft(struct entity *entp, con
* parse failure.
*/
static struct cert *
-proc_parser_cert(const struct entity *entp, const unsigned char *der,
+proc_parser_cert(char *file, const unsigned char *der,
size_t len)
{
struct cert *cert;
@@ -257,15 +256,13 @@ proc_parser_cert(const struct entity *en
STACK_OF(X509) *chain;
STACK_OF(X509_CRL) *crls;
- assert(entp->data == NULL);
-
/* Extract certificate data and X509. */
- cert = cert_parse(&x509, entp->file, der, len);
+ cert = cert_parse(&x509, file, der, len);
if (cert == NULL)
return NULL;
- a = valid_ski_aki(entp->file, &auths, cert->ski, cert->aki);
+ a = valid_ski_aki(file, &auths, cert->ski, cert->aki);
build_chain(a, &chain);
build_crls(get_crl(a), &crls);
@@ -274,7 +271,7 @@ proc_parser_cert(const struct entity *en
cryptoerrx("X509_STORE_CTX_init");
X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
- if (!X509_STORE_CTX_set_app_data(ctx, entp->file))
+ if (!X509_STORE_CTX_set_app_data(ctx, file))
cryptoerrx("X509_STORE_CTX_set_app_data");
X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
@@ -283,8 +280,7 @@ proc_parser_cert(const struct entity *en
if (X509_verify_cert(ctx) <= 0) {
c = X509_STORE_CTX_get_error(ctx);
- warnx("%s: %s", entp->file,
- X509_verify_cert_error_string(c));
+ warnx("%s: %s", file, X509_verify_cert_error_string(c));
X509_STORE_CTX_cleanup(ctx);
cert_free(cert);
sk_X509_free(chain);
@@ -301,7 +297,7 @@ proc_parser_cert(const struct entity *en
cert->talid = a->cert->talid;
/* Validate the cert to get the parent */
- if (!valid_cert(entp->file, &auths, cert)) {
+ if (!valid_cert(file, &auths, cert)) {
cert_free(cert);
return NULL;
}
@@ -408,17 +404,17 @@ proc_parser_root_cert(const struct entit
* CRL tree.
*/
static void
-proc_parser_crl(struct entity *entp, const unsigned char *der, size_t len)
+proc_parser_crl(char *file, const unsigned char *der, size_t len)
{
X509_CRL *x509_crl;
struct crl *crl;
const ASN1_TIME *at;
struct tm expires_tm;
- if ((x509_crl = crl_parse(entp->file, der, len)) != NULL) {
+ if ((x509_crl = crl_parse(file, der, len)) != NULL) {
if ((crl = malloc(sizeof(*crl))) == NULL)
err(1, NULL);
- if ((crl->aki = x509_crl_get_aki(x509_crl, entp->file)) ==
+ if ((crl->aki = x509_crl_get_aki(x509_crl, file)) ==
NULL) {
warnx("x509_crl_get_aki failed");
goto err;
@@ -429,21 +425,20 @@ proc_parser_crl(struct entity *entp, con
/* extract expire time for later use */
at = X509_CRL_get0_nextUpdate(x509_crl);
if (at == NULL) {
- warnx("%s: X509_CRL_get0_nextUpdate failed",
- entp->file);
+ warnx("%s: X509_CRL_get0_nextUpdate failed", file);
goto err;
}
memset(&expires_tm, 0, sizeof(expires_tm));
if (ASN1_time_parse(at->data, at->length, &expires_tm,
0) == -1) {
- warnx("%s: ASN1_time_parse failed", entp->file);
+ warnx("%s: ASN1_time_parse failed", file);
goto err;
}
if ((crl->expires = mktime(&expires_tm)) == -1)
- errx(1, "%s: mktime failed", entp->file);
+ errx(1, "%s: mktime failed", file);
if (RB_INSERT(crl_tree, &crlt, crl) != NULL) {
- warnx("%s: duplicate AKI %s", entp->file, crl->aki);
+ warnx("%s: duplicate AKI %s", file, crl->aki);
goto err;
}
}
@@ -456,7 +451,7 @@ proc_parser_crl(struct entity *entp, con
* Parse a ghostbuster record
*/
static void
-proc_parser_gbr(struct entity *entp, const unsigned char *der, size_t len)
+proc_parser_gbr(char *file, const unsigned char *der, size_t len)
{
struct gbr *gbr;
X509 *x509;
@@ -465,10 +460,10 @@ proc_parser_gbr(struct entity *entp, con
STACK_OF(X509) *chain;
STACK_OF(X509_CRL) *crls;
- if ((gbr = gbr_parse(&x509, entp->file, der, len)) == NULL)
+ if ((gbr = gbr_parse(&x509, file, der, len)) == NULL)
return;
- a = valid_ski_aki(entp->file, &auths, gbr->ski, gbr->aki);
+ a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki);
build_chain(a, &chain);
build_crls(get_crl(a), &crls);
@@ -477,7 +472,7 @@ proc_parser_gbr(struct entity *entp, con
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
cryptoerrx("X509_STORE_CTX_init");
X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
- if (!X509_STORE_CTX_set_app_data(ctx, entp->file))
+ if (!X509_STORE_CTX_set_app_data(ctx, file))
cryptoerrx("X509_STORE_CTX_set_app_data");
X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
@@ -487,8 +482,7 @@ proc_parser_gbr(struct entity *entp, con
if (X509_verify_cert(ctx) <= 0) {
c = X509_STORE_CTX_get_error(ctx);
if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL)
- warnx("%s: %s", entp->file,
- X509_verify_cert_error_string(c));
+ warnx("%s: %s", file, X509_verify_cert_error_string(c));
}
X509_STORE_CTX_cleanup(ctx);
@@ -571,7 +565,6 @@ parse_entity(struct entityq *q, struct m
TAILQ_REMOVE(q, entp, entries);
b = io_new_buffer();
- io_simple_buffer(b, &entp->type, sizeof(entp->type));
f = NULL;
if (entp->type != RTYPE_TAL) {
@@ -580,6 +573,10 @@ parse_entity(struct entityq *q, struct m
warn("%s", entp->file);
}
+ /* pass back at least type and filename */
+ io_simple_buffer(b, &entp->type, sizeof(entp->type));
+ io_str_buffer(b, entp->file);
+
switch (entp->type) {
case RTYPE_TAL:
if ((tal = tal_parse(entp->file, entp->data,
@@ -594,7 +591,7 @@ parse_entity(struct entityq *q, struct m
if (entp->data != NULL)
cert = proc_parser_root_cert(entp, f, flen);
else
- cert = proc_parser_cert(entp, f, flen);
+ cert = proc_parser_cert(entp->file, f, flen);
c = (cert != NULL);
io_simple_buffer(b, &c, sizeof(int));
if (cert != NULL)
@@ -606,10 +603,10 @@ parse_entity(struct entityq *q, struct m
*/
break;
case RTYPE_CRL:
- proc_parser_crl(entp, f, flen);
+ proc_parser_crl(entp->file, f, flen);
break;
case RTYPE_MFT:
- mft = proc_parser_mft(entp, f, flen);
+ mft = proc_parser_mft(entp->file, f, flen);
c = (mft != NULL);
io_simple_buffer(b, &c, sizeof(int));
if (mft != NULL)
@@ -617,7 +614,7 @@ parse_entity(struct entityq *q, struct m
mft_free(mft);
break;
case RTYPE_ROA:
- roa = proc_parser_roa(entp, f, flen);
+ roa = proc_parser_roa(entp->file, f, flen);
c = (roa != NULL);
io_simple_buffer(b, &c, sizeof(int));
if (roa != NULL)
@@ -625,7 +622,7 @@ parse_entity(struct entityq *q, struct m
roa_free(roa);
break;
case RTYPE_GBR:
- proc_parser_gbr(entp, f, flen);
+ proc_parser_gbr(entp->file, f, flen);
break;
default:
abort();
