Claudio Jeker([email protected]) on 2022.01.04 16:15:56 +0100:
> 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
>
> --
> :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.29
> diff -u -p -r1.29 parser.c
> --- parser.c 29 Dec 2021 11:37:57 -0000 1.29
> +++ parser.c 4 Jan 2022 13:19:06 -0000
> @@ -51,7 +51,7 @@ static struct crl_tree crlt = RB_INITIA
> * 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(const char *file, const unsigned char *der, size_t len)
> {
> struct roa *roa;
> X509 *x509;
> @@ -61,10 +61,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);
> @@ -82,8 +82,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);
> @@ -112,7 +111,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);
> @@ -133,7 +132,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(const char *file, const unsigned char *der, size_t len)
> {
> struct mft *mft;
> X509 *x509;
> @@ -141,10 +140,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))
> @@ -158,7 +157,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);
> @@ -169,7 +168,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;
> }
> @@ -185,7 +184,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(const char *file, const unsigned char *der,
> size_t len)
> {
> struct cert *cert;
> @@ -195,15 +194,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);
>
> @@ -219,8 +216,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);
> @@ -237,7 +233,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;
> }
> @@ -344,17 +340,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(const 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;
> @@ -365,21 +361,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;
> }
> }
> @@ -392,7 +387,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(const char *file, const unsigned char *der, size_t len)
> {
> struct gbr *gbr;
> X509 *x509;
> @@ -401,10 +396,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);
> @@ -421,8 +416,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);
> @@ -505,7 +499,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) {
> @@ -514,6 +507,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,
> @@ -528,7 +525,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)
> @@ -540,10 +537,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)
> @@ -551,7 +548,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)
> @@ -559,7 +556,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();
>
