Re: rpki-client remove double checking of hashes
On Thu, Jan 28, 2021 at 05:19:31PM +0100, Theo Buehler wrote:
> On Thu, Jan 28, 2021 at 04:42:00PM +0100, Claudio Jeker wrote:
> > Initially rpki-client checked the file hash while parsing the file (.roa,
> > .cert or .crl) but since a while rpki-client does the hash check early
> > during the .mft parsing with mft_check(). After that all files in the
> > fileandhash attribute are verified and so there is no need to do it again.
> >
> > All in all this simplifies the code a fair bit. The only problematic case
> > was the distinction between root cert and regular cert based on the
> > presence of the digest. Instead use the presence of the public key (from
> > the TAL). Result is the same, logic is inverse.
> >
> > So this still works for me.
>
> Makes sense, ok tb
>
> Please add the diff below to adjust regress when you land this.
I had the same already prepped in my tree.
> Index: test-cert.c
> ===
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 test-cert.c
> --- test-cert.c 9 Dec 2020 11:22:47 - 1.6
> +++ test-cert.c 28 Jan 2021 16:14:30 -
> @@ -145,7 +145,7 @@ main(int argc, char *argv[])
> }
> } else {
> for (i = 0; i < argc; i++) {
> - p = cert_parse(&xp, argv[i], NULL);
> + p = cert_parse(&xp, argv[i]);
> if (p == NULL)
> break;
> if (verb)
> Index: test-roa.c
> ===
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-roa.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 test-roa.c
> --- test-roa.c9 Nov 2020 16:13:02 - 1.7
> +++ test-roa.c28 Jan 2021 16:14:44 -
> @@ -87,7 +87,7 @@ main(int argc, char *argv[])
> errx(1, "argument missing");
>
> for (i = 0; i < argc; i++) {
> - if ((p = roa_parse(&xp, argv[i], NULL)) == NULL)
> + if ((p = roa_parse(&xp, argv[i])) == NULL)
> break;
> if (verb)
> roa_print(p);
>
--
:wq Claudio
Re: rpki-client remove double checking of hashes
On Thu, Jan 28, 2021 at 04:42:00PM +0100, Claudio Jeker wrote:
> Initially rpki-client checked the file hash while parsing the file (.roa,
> .cert or .crl) but since a while rpki-client does the hash check early
> during the .mft parsing with mft_check(). After that all files in the
> fileandhash attribute are verified and so there is no need to do it again.
>
> All in all this simplifies the code a fair bit. The only problematic case
> was the distinction between root cert and regular cert based on the
> presence of the digest. Instead use the presence of the public key (from
> the TAL). Result is the same, logic is inverse.
>
> So this still works for me.
Makes sense, ok tb
Please add the diff below to adjust regress when you land this.
Index: test-cert.c
===
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v
retrieving revision 1.6
diff -u -p -r1.6 test-cert.c
--- test-cert.c 9 Dec 2020 11:22:47 - 1.6
+++ test-cert.c 28 Jan 2021 16:14:30 -
@@ -145,7 +145,7 @@ main(int argc, char *argv[])
}
} else {
for (i = 0; i < argc; i++) {
- p = cert_parse(&xp, argv[i], NULL);
+ p = cert_parse(&xp, argv[i]);
if (p == NULL)
break;
if (verb)
Index: test-roa.c
===
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-roa.c,v
retrieving revision 1.7
diff -u -p -r1.7 test-roa.c
--- test-roa.c 9 Nov 2020 16:13:02 - 1.7
+++ test-roa.c 28 Jan 2021 16:14:44 -
@@ -87,7 +87,7 @@ main(int argc, char *argv[])
errx(1, "argument missing");
for (i = 0; i < argc; i++) {
- if ((p = roa_parse(&xp, argv[i], NULL)) == NULL)
+ if ((p = roa_parse(&xp, argv[i])) == NULL)
break;
if (verb)
roa_print(p);
rpki-client remove double checking of hashes
Initially rpki-client checked the file hash while parsing the file (.roa,
.cert or .crl) but since a while rpki-client does the hash check early
during the .mft parsing with mft_check(). After that all files in the
fileandhash attribute are verified and so there is no need to do it again.
All in all this simplifies the code a fair bit. The only problematic case
was the distinction between root cert and regular cert based on the
presence of the digest. Instead use the presence of the public key (from
the TAL). Result is the same, logic is inverse.
So this still works for me.
--
:wq Claudio
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.22
diff -u -p -r1.22 cert.c
--- cert.c 8 Jan 2021 08:09:07 - 1.22
+++ cert.c 28 Jan 2021 14:56:49 -
@@ -973,18 +973,16 @@ out:
* is also dereferenced.
*/
static struct cert *
-cert_parse_inner(X509 **xp, const char *fn, const unsigned char *dgst, int ta)
+cert_parse_inner(X509 **xp, const char *fn, int ta)
{
- int rc = 0, extsz, c, sz;
+ int rc = 0, extsz, c;
size_t i;
X509*x = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj;
struct parse p;
- BIO *bio = NULL, *shamd;
+ BIO *bio = NULL;
FILE*f;
- EVP_MD *md;
- char mdbuf[EVP_MAX_MD_SIZE];
*xp = NULL;
@@ -1004,49 +1002,11 @@ cert_parse_inner(X509 **xp, const char *
if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
err(1, NULL);
- /*
-* If we have a digest specified, create an MD chain that will
-* automatically compute a digest during the X509 creation.
-*/
-
- if (dgst != NULL) {
- if ((shamd = BIO_new(BIO_f_md())) == NULL)
- cryptoerrx("BIO_new");
- if (!BIO_set_md(shamd, EVP_sha256()))
- cryptoerrx("BIO_set_md");
- if ((bio = BIO_push(shamd, bio)) == NULL)
- cryptoerrx("BIO_push");
- }
-
if ((x = *xp = d2i_X509_bio(bio, NULL)) == NULL) {
cryptowarnx("%s: d2i_X509_bio", p.fn);
goto out;
}
- /*
-* If we have a digest, find it in the chain (we'll already have
-* made it, so assert otherwise) and verify it.
-*/
-
- if (dgst != NULL) {
- shamd = BIO_find_type(bio, BIO_TYPE_MD);
- assert(shamd != NULL);
-
- if (!BIO_get_md(shamd, &md))
- cryptoerrx("BIO_get_md");
- assert(EVP_MD_type(md) == NID_sha256);
-
- if ((sz = BIO_gets(shamd, mdbuf, EVP_MAX_MD_SIZE)) < 0)
- cryptoerrx("BIO_gets");
- assert(sz == SHA256_DIGEST_LENGTH);
-
- if (memcmp(mdbuf, dgst, SHA256_DIGEST_LENGTH)) {
- if (verbose > 0)
- warnx("%s: bad message digest", p.fn);
- goto out;
- }
- }
-
/* Look for X509v3 extensions. */
if ((extsz = X509_get_ext_count(x)) < 0)
@@ -1156,10 +1116,10 @@ out:
}
struct cert *
-cert_parse(X509 **xp, const char *fn, const unsigned char *dgst)
+cert_parse(X509 **xp, const char *fn)
{
- return cert_parse_inner(xp, fn, dgst, 0);
+ return cert_parse_inner(xp, fn, 0);
}
struct cert *
@@ -1169,7 +1129,7 @@ ta_parse(X509 **xp, const char *fn, cons
struct cert *p;
int rc = 0;
- if ((p = cert_parse_inner(xp, fn, NULL, 1)) == NULL)
+ if ((p = cert_parse_inner(xp, fn, 1)) == NULL)
return NULL;
if (pkey != NULL) {
Index: cms.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
retrieving revision 1.7
diff -u -p -r1.7 cms.c
--- cms.c 2 Apr 2020 09:16:43 - 1.7
+++ cms.c 28 Jan 2021 15:01:17 -
@@ -36,17 +36,16 @@
*/
unsigned char *
cms_parse_validate(X509 **xp, const char *fn,
-const char *oid, const unsigned char *dgst, size_t *rsz)
+const char *oid, size_t *rsz)
{
const ASN1_OBJECT *obj;
ASN1_OCTET_STRING **os = NULL;
- BIO *bio = NULL, *shamd;
+ BIO *bio = NULL;
CMS_ContentInfo *cms;
FILE*f;
- char buf[128], mdbuf[EVP_MAX_MD_SIZE];
+ char buf[128];
int rc = 0, sz;
STACK_OF(X509) *certs = NULL;
- EVP_MD *md;
unsigned char *res = NULL;
*rsz = 0;
@@ -66,46 +65,9 @@ cms_parse_validate(X509 **xp, const char
return
