On Mon, Mar 06, 2023 at 08:10:49PM +0000, Job Snijders wrote:
> Upon re-reading RFC 6487 section 4.8.2, SKIs are not at all arbitary
> identifiers: they must be the SHA-1 hash of the 'Subject Public Key'.
Ah, good.
> The below changeset adds a SPK digest calculation and comparison to the
> X509v3 extension containing the SKI.
>
> OK?
ok with a small nit below
>
> Index: x509.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 x509.c
> --- x509.c 16 Feb 2023 14:34:34 -0000 1.65
> +++ x509.c 6 Mar 2023 20:06:24 -0000
> @@ -186,15 +186,17 @@ out:
>
> /*
> * Parse X509v3 subject key identifier (SKI), RFC 6487 sec. 4.8.2.
> - * Returns the SKI or NULL if it could not be parsed.
> - * The SKI is formatted as a hex string.
> + * The SKI must be the SHA1 hash of the Subject Public Key.
> + * Returns the SKI formatted as hex string, or NULL if it couldn't be parsed.
> */
> int
> x509_get_ski(X509 *x, const char *fn, char **ski)
> {
> - const unsigned char *d;
> + const unsigned char *d, *spk;
> ASN1_OCTET_STRING *os;
> - int dsz, crit, rc = 0;
> + X509_PUBKEY *pkey;
> + unsigned char spkd[SHA_DIGEST_LENGTH];
> + int spkz, dsz, crit, rc = 0;
>
> *ski = NULL;
> os = X509_get_ext_d2i(x, NID_subject_key_identifier, &crit, NULL);
> @@ -216,9 +218,28 @@ x509_get_ski(X509 *x, const char *fn, ch
> goto out;
> }
>
> + if ((pkey = X509_get_X509_PUBKEY(x)) == NULL) {
> + warnx("%s: X509_get_X509_PUBKEY", fn);
> + goto out;
> + }
> + if (X509_PUBKEY_get0_param(NULL, &spk, &spkz, NULL, pkey) != 1) {
This can't really fail. The check should be (documentation says 0 on error):
if (!X509_PUBKEY_get0_param()) {
> + warnx("%s: X509_PUBKEY_get0_param", fn);
> + goto out;
> + }
> +
> + if (!EVP_Digest(spk, spkz, spkd, NULL, EVP_sha1(), NULL)) {
> + warnx("%s: EVP_Digest failed", fn);
> + goto out;
> + }
> +
> + if (memcmp(spkd, d, dsz) != 0) {
> + warnx("%s: SKI does not match SHA1 hash of SPK", fn);
> + goto out;
> + }
> +
> *ski = hex_encode(d, dsz);
> rc = 1;
> -out:
> + out:
> ASN1_OCTET_STRING_free(os);
> return rc;
> }
>
