2014-10-02 15:49:52

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

Hi David,

I just took latest #next branch from James's security tree which
includes latest KEYs patches and noticed following:

[ 9.812332] Request for unknown module key 'Magrathea: Glacier
signing key: 926305d6dda66f47139eb4e3cb25a6adef527f08' err -11

Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
also has changed in respect of certificate ids..

Those ids does not look any close to my kernel X509 X509v3 Subject Key
Identifier, which is:
92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08

proc/keys shows

symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
6865612e68326732 []

Very different ids..

How could I match certificate now?
Module verification code cannot find needed key..

- Dmitry

On 08/09/14 18:38, David Howells wrote:
> Make use of the new match string preparsing to overhaul key identification
> when searching for asymmetric keys. The following changes are made:
>
> (1) Use the previously created asymmetric_key_id struct to hold the following
> key IDs derived from the X.509 certificate or PKCS#7 message:
>
> id: serial number + issuer
> skid: subjKeyId + subject
> authority: authKeyId + issuer
>
> (2) Replace the hex fingerprint attached to key->type_data[1] with an
> asymmetric_key_ids struct containing the id and the skid (if present).
>
> (3) Make the asymmetric_type match data preparse select one of two searches:
>
> (a) An iterative search for the key ID given if prefixed with "id:". The
> prefix is expected to be followed by a hex string giving the ID to
> search for. The criterion key ID is checked against all key IDs
> recorded on the key.
>
> (b) A direct search if the key ID is not prefixed with "id:". This will
> look for an exact match on the key description.
>
> (4) Make x509_request_asymmetric_key() take a key ID. This is then converted
> into "id:<hex>" and passed into keyring_search() where match preparsing
> will turn it back into a binary ID.
>
> (5) X.509 certificate verification then takes the authority key ID and looks
> up a key that matches it to find the public key for the certificate
> signature.
>
> (6) PKCS#7 certificate verification then takes the id key ID and looks up a
> key that matches it to find the public key for the signed information
> block signature.
>
> Additional changes:
>
> (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
> cert to be rejected with -EBADMSG.
>
> (2) The 'fingerprint' ID is gone. This was primarily intended to convey PGP
> public key fingerprints. If PGP is supported in future, this should
> generate a key ID that carries the fingerprint.
>
> (3) Th ca_keyid= kernel command line option is now converted to a key ID and
> used to match the authority key ID. Possibly this should only match the
> actual authKeyId part and not the issuer as well.
>
> Signed-off-by: David Howells <[email protected]>
> ---
>
> crypto/asymmetric_keys/asymmetric_keys.h | 4 -
> crypto/asymmetric_keys/asymmetric_type.c | 133 ++++++++++++-----------------
> crypto/asymmetric_keys/pkcs7_parser.c | 38 ++++++--
> crypto/asymmetric_keys/pkcs7_parser.h | 5 -
> crypto/asymmetric_keys/pkcs7_trust.c | 6 -
> crypto/asymmetric_keys/pkcs7_verify.c | 44 ++++------
> crypto/asymmetric_keys/x509_cert_parser.c | 55 +++++++-----
> crypto/asymmetric_keys/x509_parser.h | 5 +
> crypto/asymmetric_keys/x509_public_key.c | 89 +++++++++++--------
> include/crypto/public_key.h | 5 +
> 10 files changed, 198 insertions(+), 186 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
> index 917be6b985e7..fd21ac28e0a0 100644
> --- a/crypto/asymmetric_keys/asymmetric_keys.h
> +++ b/crypto/asymmetric_keys/asymmetric_keys.h
> @@ -9,13 +9,13 @@
> * 2 of the Licence, or (at your option) any later version.
> */
>
> -int asymmetric_keyid_match(const char *kid, const char *id);
> extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
> const struct asymmetric_key_id *match_id);
>
> extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
>
> -static inline const char *asymmetric_key_id(const struct key *key)
> +static inline
> +const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
> {
> return key->type_data.p[1];
> }
> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> index 3bc71b4e1eed..6f16f647d21b 100644
> --- a/crypto/asymmetric_keys/asymmetric_type.c
> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> @@ -105,76 +105,15 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
> }
>
> /*
> - * Match asymmetric key id with partial match
> - * @id: key id to match in a form "id:<id>"
> - */
> -int asymmetric_keyid_match(const char *kid, const char *id)
> -{
> - size_t idlen, kidlen;
> -
> - if (!kid || !id)
> - return 0;
> -
> - /* make it possible to use id as in the request: "id:<id>" */
> - if (strncmp(id, "id:", 3) == 0)
> - id += 3;
> -
> - /* Anything after here requires a partial match on the ID string */
> - idlen = strlen(id);
> - kidlen = strlen(kid);
> - if (idlen > kidlen)
> - return 0;
> -
> - kid += kidlen - idlen;
> - if (strcasecmp(id, kid) != 0)
> - return 0;
> -
> - return 1;
> -}
> -EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
> -
> -/*
> - * Match asymmetric keys on (part of) their name
> - * We have some shorthand methods for matching keys. We allow:
> - *
> - * "<desc>" - request a key by description
> - * "id:<id>" - request a key matching the ID
> - * "<subtype>:<id>" - request a key of a subtype
> + * Match asymmetric keys by ID.
> */
> static bool asymmetric_key_cmp(const struct key *key,
> const struct key_match_data *match_data)
> {
> - const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> - const char *description = match_data->raw_data;
> - const char *spec = description;
> - const char *id;
> - ptrdiff_t speclen;
> -
> - if (!subtype || !spec || !*spec)
> - return 0;
> -
> - /* See if the full key description matches as is */
> - if (key->description && strcmp(key->description, description) == 0)
> - return 1;
> -
> - /* All tests from here on break the criterion description into a
> - * specifier, a colon and then an identifier.
> - */
> - id = strchr(spec, ':');
> - if (!id)
> - return 0;
> -
> - speclen = id - spec;
> - id++;
> -
> - if (speclen == 2 && memcmp(spec, "id", 2) == 0)
> - return asymmetric_keyid_match(asymmetric_key_id(key), id);
> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> + const struct asymmetric_key_id *match_id = match_data->preparsed;
>
> - if (speclen == subtype->name_len &&
> - memcmp(spec, subtype->name, speclen) == 0)
> - return 1;
> -
> - return 0;
> + return asymmetric_match_key_ids(kids, match_id);
> }
>
> /*
> @@ -191,8 +130,30 @@ static bool asymmetric_key_cmp(const struct key *key,
> */
> static int asymmetric_key_match_preparse(struct key_match_data *match_data)
> {
> - match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
> + struct asymmetric_key_id *match_id;
> + const char *spec = match_data->raw_data;
> + const char *id;
> +
> + if (!spec || !*spec)
> + return -EINVAL;
> + if (spec[0] == 'i' &&
> + spec[1] == 'd' &&
> + spec[2] == ':') {
> + id = spec + 3;
> + } else {
> + goto default_match;
> + }
> +
> + match_id = asymmetric_key_hex_to_key_id(id);
> + if (!match_id)
> + return -ENOMEM;
> +
> + match_data->preparsed = match_id;
> match_data->cmp = asymmetric_key_cmp;
> + match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
> + return 0;
> +
> +default_match:
> return 0;
> }
>
> @@ -201,6 +162,7 @@ static int asymmetric_key_match_preparse(struct key_match_data *match_data)
> */
> static void asymmetric_key_match_free(struct key_match_data *match_data)
> {
> + kfree(match_data->preparsed);
> }
>
> /*
> @@ -209,8 +171,10 @@ static void asymmetric_key_match_free(struct key_match_data *match_data)
> static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
> {
> const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> - const char *kid = asymmetric_key_id(key);
> - size_t n;
> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> + const struct asymmetric_key_id *kid;
> + const unsigned char *p;
> + int n;
>
> seq_puts(m, key->description);
>
> @@ -218,13 +182,16 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
> seq_puts(m, ": ");
> subtype->describe(key, m);
>
> - if (kid) {
> + if (kids && kids->id[0]) {
> + kid = kids->id[0];
> seq_putc(m, ' ');
> - n = strlen(kid);
> - if (n <= 8)
> - seq_puts(m, kid);
> - else
> - seq_puts(m, kid + n - 8);
> + n = kid->len;
> + p = kid->data;
> + if (n > 8) {
> + p += n - 8;
> + n = 8;
> + }
> + seq_printf(m, "%*phN", n, p);
> }
>
> seq_puts(m, " [");
> @@ -275,6 +242,7 @@ static int asymmetric_key_preparse(struct key_preparsed_payload *prep)
> static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> {
> struct asymmetric_key_subtype *subtype = prep->type_data[0];
> + struct asymmetric_key_ids *kids = prep->type_data[1];
>
> pr_devel("==>%s()\n", __func__);
>
> @@ -282,7 +250,11 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> subtype->destroy(prep->payload[0]);
> module_put(subtype->owner);
> }
> - kfree(prep->type_data[1]);
> + if (kids) {
> + kfree(kids->id[0]);
> + kfree(kids->id[1]);
> + kfree(kids);
> + }
> kfree(prep->description);
> }
>
> @@ -292,13 +264,20 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> static void asymmetric_key_destroy(struct key *key)
> {
> struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> + struct asymmetric_key_ids *kids = key->type_data.p[1];
> +
> if (subtype) {
> subtype->destroy(key->payload.data);
> module_put(subtype->owner);
> key->type_data.p[0] = NULL;
> }
> - kfree(key->type_data.p[1]);
> - key->type_data.p[1] = NULL;
> +
> + if (kids) {
> + kfree(kids->id[0]);
> + kfree(kids->id[1]);
> + kfree(kids);
> + key->type_data.p[1] = NULL;
> + }
> }
>
> struct key_type key_type_asymmetric = {
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index 459d2077c61b..ad6ae9d7c884 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> @@ -29,6 +29,10 @@ struct pkcs7_parse_context {
> enum OID last_oid; /* Last OID encountered */
> unsigned x509_index;
> unsigned sinfo_index;
> + const void *raw_serial;
> + unsigned raw_serial_size;
> + unsigned raw_issuer_size;
> + const void *raw_issuer;
> };
>
> /*
> @@ -39,6 +43,7 @@ static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
> if (sinfo) {
> mpi_free(sinfo->sig.mpi[0]);
> kfree(sinfo->sig.digest);
> + kfree(sinfo->signing_cert_id);
> kfree(sinfo);
> }
> }
> @@ -256,10 +261,10 @@ int pkcs7_extract_cert(void *context, size_t hdrlen,
> if (IS_ERR(x509))
> return PTR_ERR(x509);
>
> - pr_debug("Got cert for %s\n", x509->subject);
> - pr_debug("- fingerprint %s\n", x509->fingerprint);
> -
> x509->index = ++ctx->x509_index;
> + pr_debug("Got cert %u for %s\n", x509->index, x509->subject);
> + pr_debug("- fingerprint %*phN\n", x509->id->len, x509->id->data);
> +
> *ctx->ppcerts = x509;
> ctx->ppcerts = &x509->next;
> return 0;
> @@ -348,8 +353,8 @@ int pkcs7_sig_note_serial(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct pkcs7_parse_context *ctx = context;
> - ctx->sinfo->raw_serial = value;
> - ctx->sinfo->raw_serial_size = vlen;
> + ctx->raw_serial = value;
> + ctx->raw_serial_size = vlen;
> return 0;
> }
>
> @@ -361,8 +366,8 @@ int pkcs7_sig_note_issuer(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct pkcs7_parse_context *ctx = context;
> - ctx->sinfo->raw_issuer = value;
> - ctx->sinfo->raw_issuer_size = vlen;
> + ctx->raw_issuer = value;
> + ctx->raw_issuer_size = vlen;
> return 0;
> }
>
> @@ -395,10 +400,21 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct pkcs7_parse_context *ctx = context;
> -
> - ctx->sinfo->index = ++ctx->sinfo_index;
> - *ctx->ppsinfo = ctx->sinfo;
> - ctx->ppsinfo = &ctx->sinfo->next;
> + struct pkcs7_signed_info *sinfo = ctx->sinfo;
> + struct asymmetric_key_id *kid;
> +
> + /* Generate cert issuer + serial number key ID */
> + kid = asymmetric_key_generate_id(ctx->raw_serial,
> + ctx->raw_serial_size,
> + ctx->raw_issuer,
> + ctx->raw_issuer_size);
> + if (IS_ERR(kid))
> + return PTR_ERR(kid);
> +
> + sinfo->signing_cert_id = kid;
> + sinfo->index = ++ctx->sinfo_index;
> + *ctx->ppsinfo = sinfo;
> + ctx->ppsinfo = &sinfo->next;
> ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
> if (!ctx->sinfo)
> return -ENOMEM;
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
> index d25f4d15370f..91949f92bc72 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.h
> +++ b/crypto/asymmetric_keys/pkcs7_parser.h
> @@ -33,10 +33,7 @@ struct pkcs7_signed_info {
> const void *authattrs;
>
> /* Issuing cert serial number and issuer's name */
> - const void *raw_serial;
> - unsigned raw_serial_size;
> - unsigned raw_issuer_size;
> - const void *raw_issuer;
> + struct asymmetric_key_id *signing_cert_id;
>
> /* Message signature.
> *
> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> index e666eb011a85..4e8dd7214753 100644
> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> @@ -49,8 +49,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> /* Look to see if this certificate is present in the trusted
> * keys.
> */
> - key = x509_request_asymmetric_key(trust_keyring, x509->subject,
> - x509->fingerprint);
> + key = x509_request_asymmetric_key(trust_keyring, x509->id);
> if (!IS_ERR(key))
> /* One of the X.509 certificates in the PKCS#7 message
> * is apparently the same as one we already trust.
> @@ -82,8 +81,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> return -ENOKEY;
> }
>
> - key = x509_request_asymmetric_key(trust_keyring, last->issuer,
> - last->authority);
> + key = x509_request_asymmetric_key(trust_keyring, last->authority);
> if (IS_ERR(key))
> return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
> x509 = last;
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index c62cf8006e1f..57e90fa17f2b 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -131,8 +131,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> struct x509_certificate *x509;
> unsigned certix = 1;
>
> - kenter("%u,%u,%u",
> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_issuer_size);
> + kenter("%u", sinfo->index);
>
> for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
> /* I'm _assuming_ that the generator of the PKCS#7 message will
> @@ -140,21 +139,11 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> * PKCS#7 message - but I can't be 100% sure of that. It's
> * possible this will need element-by-element comparison.
> */
> - if (x509->raw_serial_size != sinfo->raw_serial_size ||
> - memcmp(x509->raw_serial, sinfo->raw_serial,
> - sinfo->raw_serial_size) != 0)
> + if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
> continue;
> pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
> sinfo->index, certix);
>
> - if (x509->raw_issuer_size != sinfo->raw_issuer_size ||
> - memcmp(x509->raw_issuer, sinfo->raw_issuer,
> - sinfo->raw_issuer_size) != 0) {
> - pr_warn("Sig %u: X.509 subject and PKCS#7 issuer don't match\n",
> - sinfo->index);
> - continue;
> - }
> -
> if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
> pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
> sinfo->index);
> @@ -164,8 +153,10 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> sinfo->signer = x509;
> return 0;
> }
> +
> pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_serial);
> + sinfo->index,
> + sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
> return -ENOKEY;
> }
>
> @@ -184,7 +175,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> p->seen = false;
>
> for (;;) {
> - pr_debug("verify %s: %s\n", x509->subject, x509->fingerprint);
> + pr_debug("verify %s: %*phN\n",
> + x509->subject,
> + x509->raw_serial_size, x509->raw_serial);
> x509->seen = true;
> ret = x509_get_sig_params(x509);
> if (ret < 0)
> @@ -192,7 +185,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>
> pr_debug("- issuer %s\n", x509->issuer);
> if (x509->authority)
> - pr_debug("- authkeyid %s\n", x509->authority);
> + pr_debug("- authkeyid %*phN\n",
> + x509->authority->len, x509->authority->data);
>
> if (!x509->authority ||
> strcmp(x509->subject, x509->issuer) == 0) {
> @@ -218,13 +212,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> /* Look through the X.509 certificates in the PKCS#7 message's
> * list to see if the next one is there.
> */
> - pr_debug("- want %s\n", x509->authority);
> + pr_debug("- want %*phN\n",
> + x509->authority->len, x509->authority->data);
> for (p = pkcs7->certs; p; p = p->next) {
> - pr_debug("- cmp [%u] %s\n", p->index, p->fingerprint);
> - if (p->raw_subject_size == x509->raw_issuer_size &&
> - strcmp(p->fingerprint, x509->authority) == 0 &&
> - memcmp(p->raw_subject, x509->raw_issuer,
> - x509->raw_issuer_size) == 0)
> + if (!p->skid)
> + continue;
> + pr_debug("- cmp [%u] %*phN\n",
> + p->index, p->skid->len, p->skid->data);
> + if (asymmetric_key_id_same(p->skid, x509->authority))
> goto found_issuer;
> }
>
> @@ -233,7 +228,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> return 0;
>
> found_issuer:
> - pr_debug("- issuer %s\n", p->subject);
> + pr_debug("- subject %s\n", p->subject);
> if (p->seen) {
> pr_warn("Sig %u: X.509 chain contains loop\n",
> sinfo->index);
> @@ -304,7 +299,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
> ret = x509_get_sig_params(x509);
> if (ret < 0)
> return ret;
> - pr_debug("X.509[%u] %s\n", n, x509->authority);
> + pr_debug("X.509[%u] %*phN\n",
> + n, x509->authority->len, x509->authority->data);
> }
>
> for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index ac72348c186a..96151b2b91a2 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -46,7 +46,8 @@ void x509_free_certificate(struct x509_certificate *cert)
> public_key_destroy(cert->pub);
> kfree(cert->issuer);
> kfree(cert->subject);
> - kfree(cert->fingerprint);
> + kfree(cert->id);
> + kfree(cert->skid);
> kfree(cert->authority);
> kfree(cert->sig.digest);
> mpi_free(cert->sig.rsa.s);
> @@ -62,6 +63,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> {
> struct x509_certificate *cert;
> struct x509_parse_context *ctx;
> + struct asymmetric_key_id *kid;
> long ret;
>
> ret = -ENOMEM;
> @@ -89,6 +91,17 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> if (ret < 0)
> goto error_decode;
>
> + /* Generate cert issuer + serial number key ID */
> + kid = asymmetric_key_generate_id(cert->raw_serial,
> + cert->raw_serial_size,
> + cert->raw_issuer,
> + cert->raw_issuer_size);
> + if (IS_ERR(kid)) {
> + ret = PTR_ERR(kid);
> + goto error_decode;
> + }
> + cert->id = kid;
> +
> kfree(ctx);
> return cert;
>
> @@ -407,36 +420,34 @@ int x509_process_extension(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct x509_parse_context *ctx = context;
> + struct asymmetric_key_id *kid;
> const unsigned char *v = value;
> - char *f;
> int i;
>
> pr_debug("Extension: %u\n", ctx->last_oid);
>
> if (ctx->last_oid == OID_subjectKeyIdentifier) {
> /* Get hold of the key fingerprint */
> - if (vlen < 3)
> + if (ctx->cert->skid || vlen < 3)
> return -EBADMSG;
> if (v[0] != ASN1_OTS || v[1] != vlen - 2)
> return -EBADMSG;
> v += 2;
> vlen -= 2;
>
> - f = kmalloc(vlen * 2 + 1, GFP_KERNEL);
> - if (!f)
> - return -ENOMEM;
> - for (i = 0; i < vlen; i++)
> - sprintf(f + i * 2, "%02x", v[i]);
> - pr_debug("fingerprint %s\n", f);
> - ctx->cert->fingerprint = f;
> + kid = asymmetric_key_generate_id(v, vlen,
> + ctx->cert->raw_subject,
> + ctx->cert->raw_subject_size);
> + if (IS_ERR(kid))
> + return PTR_ERR(kid);
> + ctx->cert->skid = kid;
> + pr_debug("subjkeyid %*phN\n", kid->len, kid->data);
> return 0;
> }
>
> if (ctx->last_oid == OID_authorityKeyIdentifier) {
> - size_t key_len;
> -
> /* Get hold of the CA key fingerprint */
> - if (vlen < 5)
> + if (ctx->cert->authority || vlen < 5)
> return -EBADMSG;
>
> /* Authority Key Identifier must be a Constructed SEQUENCE */
> @@ -454,7 +465,7 @@ int x509_process_extension(void *context, size_t hdrlen,
> v[3] > vlen - 4)
> return -EBADMSG;
>
> - key_len = v[3];
> + vlen = v[3];
> v += 4;
> } else {
> /* Long Form length */
> @@ -476,17 +487,17 @@ int x509_process_extension(void *context, size_t hdrlen,
> v[sub + 1] > vlen - 4 - sub)
> return -EBADMSG;
>
> - key_len = v[sub + 1];
> + vlen = v[sub + 1];
> v += (sub + 2);
> }
>
> - f = kmalloc(key_len * 2 + 1, GFP_KERNEL);
> - if (!f)
> - return -ENOMEM;
> - for (i = 0; i < key_len; i++)
> - sprintf(f + i * 2, "%02x", v[i]);
> - pr_debug("authority %s\n", f);
> - ctx->cert->authority = f;
> + kid = asymmetric_key_generate_id(v, vlen,
> + ctx->cert->raw_issuer,
> + ctx->cert->raw_issuer_size);
> + if (IS_ERR(kid))
> + return PTR_ERR(kid);
> + pr_debug("authkeyid %*phN\n", kid->len, kid->data);
> + ctx->cert->authority = kid;
> return 0;
> }
>
> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
> index 1b76f207c1f3..0e8d59b010fb 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -19,8 +19,9 @@ struct x509_certificate {
> struct public_key_signature sig; /* Signature parameters */
> char *issuer; /* Name of certificate issuer */
> char *subject; /* Name of certificate subject */
> - char *fingerprint; /* Key fingerprint as hex */
> - char *authority; /* Authority key fingerprint as hex */
> + struct asymmetric_key_id *id; /* Issuer + serial number */
> + struct asymmetric_key_id *skid; /* Subject key identifier */
> + struct asymmetric_key_id *authority; /* Authority key identifier */
> struct tm valid_from;
> struct tm valid_to;
> const void *tbs; /* Signed data */
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index f3d62307e6ee..c60905c3f4d2 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -25,7 +25,7 @@
> #include "x509_parser.h"
>
> static bool use_builtin_keys;
> -static char *ca_keyid;
> +static struct asymmetric_key_id *ca_keyid;
>
> #ifndef MODULE
> static int __init ca_keys_setup(char *str)
> @@ -33,10 +33,16 @@ static int __init ca_keys_setup(char *str)
> if (!str) /* default system keyring */
> return 1;
>
> - if (strncmp(str, "id:", 3) == 0)
> - ca_keyid = str; /* owner key 'id:xxxxxx' */
> - else if (strcmp(str, "builtin") == 0)
> + if (strncmp(str, "id:", 3) == 0) {
> + struct asymmetric_key_id *p;
> + p = asymmetric_key_hex_to_key_id(str);
> + if (p == ERR_PTR(-EINVAL))
> + pr_err("Unparsable hex string in ca_keys\n");
> + else if (!IS_ERR(p))
> + ca_keyid = p; /* owner key 'id:xxxxxx' */
> + } else if (strcmp(str, "builtin") == 0) {
> use_builtin_keys = true;
> + }
>
> return 1;
> }
> @@ -46,31 +52,28 @@ __setup("ca_keys=", ca_keys_setup);
> /**
> * x509_request_asymmetric_key - Request a key by X.509 certificate params.
> * @keyring: The keys to search.
> - * @subject: The name of the subject to whom the key belongs.
> - * @key_id: The subject key ID as a hex string.
> + * @kid: The key ID.
> *
> * Find a key in the given keyring by subject name and key ID. These might,
> * for instance, be the issuer name and the authority key ID of an X.509
> * certificate that needs to be verified.
> */
> struct key *x509_request_asymmetric_key(struct key *keyring,
> - const char *subject,
> - const char *key_id)
> + const struct asymmetric_key_id *kid)
> {
> key_ref_t key;
> - size_t subject_len = strlen(subject), key_id_len = strlen(key_id);
> - char *id;
> + char *id, *p;
>
> - /* Construct an identifier "<subjname>:<keyid>". */
> - id = kmalloc(subject_len + 2 + key_id_len + 1, GFP_KERNEL);
> + /* Construct an identifier "id:<keyid>". */
> + p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
> if (!id)
> return ERR_PTR(-ENOMEM);
>
> - memcpy(id, subject, subject_len);
> - id[subject_len + 0] = ':';
> - id[subject_len + 1] = ' ';
> - memcpy(id + subject_len + 2, key_id, key_id_len);
> - id[subject_len + 2 + key_id_len] = 0;
> + *p++ = 'i';
> + *p++ = 'd';
> + *p++ = ':';
> + p = bin2hex(p, kid->data, kid->len);
> + *p = 0;
>
> pr_debug("Look up: \"%s\"\n", id);
>
> @@ -195,11 +198,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
> if (!trust_keyring)
> return -EOPNOTSUPP;
>
> - if (ca_keyid && !asymmetric_keyid_match(cert->authority, ca_keyid))
> + if (ca_keyid && !asymmetric_key_id_same(cert->authority, ca_keyid))
> return -EPERM;
>
> - key = x509_request_asymmetric_key(trust_keyring,
> - cert->issuer, cert->authority);
> + key = x509_request_asymmetric_key(trust_keyring, cert->authority);
> if (!IS_ERR(key)) {
> if (!use_builtin_keys
> || test_bit(KEY_FLAG_BUILTIN, &key->flags))
> @@ -214,9 +216,11 @@ static int x509_validate_trust(struct x509_certificate *cert,
> */
> static int x509_key_preparse(struct key_preparsed_payload *prep)
> {
> + struct asymmetric_key_ids *kids;
> struct x509_certificate *cert;
> + const char *q;
> size_t srlen, sulen;
> - char *desc = NULL;
> + char *desc = NULL, *p;
> int ret;
>
> cert = x509_cert_parse(prep->data, prep->datalen);
> @@ -249,19 +253,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> pkey_algo_name[cert->sig.pkey_algo],
> hash_algo_name[cert->sig.pkey_hash_algo]);
>
> - if (!cert->fingerprint) {
> - pr_warn("Cert for '%s' must have a SubjKeyId extension\n",
> - cert->subject);
> - ret = -EKEYREJECTED;
> - goto error_free_cert;
> - }
> -
> cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
> cert->pub->id_type = PKEY_ID_X509;
>
> /* Check the signature on the key if it appears to be self-signed */
> if (!cert->authority ||
> - strcmp(cert->fingerprint, cert->authority) == 0) {
> + asymmetric_key_id_same(cert->skid, cert->authority)) {
> ret = x509_check_signature(cert->pub, cert); /* self-signed */
> if (ret < 0)
> goto error_free_cert;
> @@ -273,31 +270,47 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>
> /* Propose a description */
> sulen = strlen(cert->subject);
> - srlen = strlen(cert->fingerprint);
> + srlen = cert->raw_serial_size;
> + q = cert->raw_serial;
> + if (srlen > 1 && *q == 0) {
> + srlen--;
> + q++;
> + }
> +
> ret = -ENOMEM;
> - desc = kmalloc(sulen + 2 + srlen + 1, GFP_KERNEL);
> + desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
> if (!desc)
> goto error_free_cert;
> - memcpy(desc, cert->subject, sulen);
> - desc[sulen] = ':';
> - desc[sulen + 1] = ' ';
> - memcpy(desc + sulen + 2, cert->fingerprint, srlen);
> - desc[sulen + 2 + srlen] = 0;
> + p = memcpy(desc, cert->subject, sulen);
> + p += sulen;
> + *p++ = ':';
> + *p++ = ' ';
> + p = bin2hex(p, q, srlen);
> + *p = 0;
> +
> + kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
> + if (!kids)
> + goto error_free_desc;
> + kids->id[0] = cert->id;
> + kids->id[1] = cert->skid;
>
> /* We're pinning the module by being linked against it */
> __module_get(public_key_subtype.owner);
> prep->type_data[0] = &public_key_subtype;
> - prep->type_data[1] = cert->fingerprint;
> + prep->type_data[1] = kids;
> prep->payload[0] = cert->pub;
> prep->description = desc;
> prep->quotalen = 100;
>
> /* We've finished with the certificate */
> cert->pub = NULL;
> - cert->fingerprint = NULL;
> + cert->id = NULL;
> + cert->skid = NULL;
> desc = NULL;
> ret = 0;
>
> +error_free_desc:
> + kfree(desc);
> error_free_cert:
> x509_free_certificate(cert);
> return ret;
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 0d164c6af539..fa73a6fd536c 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -15,6 +15,7 @@
> #define _LINUX_PUBLIC_KEY_H
>
> #include <linux/mpi.h>
> +#include <keys/asymmetric-type.h>
> #include <crypto/hash_info.h>
>
> enum pkey_algo {
> @@ -98,8 +99,8 @@ struct key;
> extern int verify_signature(const struct key *key,
> const struct public_key_signature *sig);
>
> +struct asymmetric_key_id;
> extern struct key *x509_request_asymmetric_key(struct key *keyring,
> - const char *issuer,
> - const char *key_id);
> + const struct asymmetric_key_id *kid);
>
> #endif /* _LINUX_PUBLIC_KEY_H */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2014-10-02 16:04:54

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

On 02/10/14 18:49, Dmitry Kasatkin wrote:
> Hi David,
>
> I just took latest #next branch from James's security tree which
> includes latest KEYs patches and noticed following:
>
> [ 9.812332] Request for unknown module key 'Magrathea: Glacier
> signing key: 926305d6dda66f47139eb4e3cb25a6adef527f08' err -11
>
> Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
> also has changed in respect of certificate ids..
>
> Those ids does not look any close to my kernel X509 X509v3 Subject Key
> Identifier, which is:
> 92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08
>
> proc/keys shows
>
> symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
> 6865612e68326732 []
>
> Very different ids..
>
> How could I match certificate now?
> Module verification code cannot find needed key..
>
> - Dmitry


Hehe. Also now I get kernel Oops in asymmetric_key_id_same...

-------------------------
[ 132.816522] BUG: unable to handle kernel paging request at
ffffffffffffffea
[ 132.819902] IP: [<ffffffff812bfc20>] asymmetric_key_id_same+0x14/0x36
[ 132.820302] PGD 1a12067 PUD 1a14067 PMD 0
[ 132.820302] Oops: 0000 [#1] SMP
[ 132.820302] Modules linked in: bridge(E) stp(E) llc(E) evdev(E)
serio_raw(E) i2c_piix4(E) button(E) fuse(E)
[ 132.820302] CPU: 0 PID: 2993 Comm: cat Tainted: G E
3.16.0-kds+ #2847
[ 132.820302] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 132.820302] task: ffff88004249a430 ti: ffff880056640000 task.ti:
ffff880056640000
[ 132.820302] RIP: 0010:[<ffffffff812bfc20>] [<ffffffff812bfc20>]
asymmetric_key_id_same+0x14/0x36
[ 132.820302] RSP: 0018:ffff880056643930 EFLAGS: 00010246
[ 132.820302] RAX: 0000000000000000 RBX: ffffffffffffffea RCX:
ffff880056643ae0
[ 132.820302] RDX: 000000000000005e RSI: ffffffffffffffea RDI:
ffff88005bac9300
[ 132.820302] RBP: ffff880056643948 R08: 0000000000000003 R09:
00000007504aa01a
[ 132.820302] R10: 0000000000000000 R11: 0000000000000000 R12:
ffff88005d68ca40
[ 132.820302] R13: 0000000000000101 R14: 0000000000000000 R15:
ffff88005bac5280
[ 132.820302] FS: 00007f67a153c740(0000) GS:ffff88005da00000(0000)
knlGS:0000000000000000
[ 132.820302] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 132.820302] CR2: ffffffffffffffea CR3: 000000002e663000 CR4:
00000000000006f0
[ 132.820302] Stack:
[ 132.820302] ffffffff812bfc66 ffff880056643ae0 ffff88005bac5280
ffff880056643958
[ 132.820302] ffffffff812bfc9d ffff880056643980 ffffffff812971d9
ffff88005ce930c1
[ 132.820302] ffff88005ce930c0 0000000000000000 ffff8800566439c8
ffffffff812fb753
[ 132.820302] Call Trace:
[ 132.820302] [<ffffffff812bfc66>] ? asymmetric_match_key_ids+0x24/0x42
[ 132.820302] [<ffffffff812bfc9d>] asymmetric_key_cmp+0x19/0x1b
[ 132.820302] [<ffffffff812971d9>] keyring_search_iterator+0x74/0xd7
[ 132.820302] [<ffffffff812fb753>] assoc_array_subtree_iterate+0x67/0xd2
[ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
[ 132.820302] [<ffffffff812fbaa1>] assoc_array_iterate+0x19/0x1e
[ 132.820302] [<ffffffff81297332>] search_nested_keyrings+0xf6/0x2b6
[ 132.820302] [<ffffffff810728da>] ? sched_clock_cpu+0x91/0xa2
[ 132.820302] [<ffffffff810860d2>] ? mark_held_locks+0x58/0x6e
[ 132.820302] [<ffffffff810a137d>] ? current_kernel_time+0x77/0xb8
[ 132.820302] [<ffffffff81297871>] keyring_search_aux+0xe1/0x14c
[ 132.820302] [<ffffffff812977fc>] ? keyring_search_aux+0x6c/0x14c
[ 132.820302] [<ffffffff8129796b>] keyring_search+0x8f/0xb6
[ 132.820302] [<ffffffff812bfc84>] ? asymmetric_match_key_ids+0x42/0x42
[ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
[ 132.820302] [<ffffffff812ab9e3>] asymmetric_verify+0xa4/0x214
[ 132.820302] [<ffffffff812ab90e>] integrity_digsig_verify+0xb1/0xe2
[ 132.820302] [<ffffffff812abe41>] ? evm_verifyxattr+0x6a/0x7a
[ 132.820302] [<ffffffff812b0390>] ima_appraise_measurement+0x160/0x370
[ 132.820302] [<ffffffff81161db2>] ? d_absolute_path+0x5b/0x7a
[ 132.820302] [<ffffffff812ada30>] process_measurement+0x322/0x404


> On 08/09/14 18:38, David Howells wrote:
>> Make use of the new match string preparsing to overhaul key identification
>> when searching for asymmetric keys. The following changes are made:
>>
>> (1) Use the previously created asymmetric_key_id struct to hold the following
>> key IDs derived from the X.509 certificate or PKCS#7 message:
>>
>> id: serial number + issuer
>> skid: subjKeyId + subject
>> authority: authKeyId + issuer
>>
>> (2) Replace the hex fingerprint attached to key->type_data[1] with an
>> asymmetric_key_ids struct containing the id and the skid (if present).
>>
>> (3) Make the asymmetric_type match data preparse select one of two searches:
>>
>> (a) An iterative search for the key ID given if prefixed with "id:". The
>> prefix is expected to be followed by a hex string giving the ID to
>> search for. The criterion key ID is checked against all key IDs
>> recorded on the key.
>>
>> (b) A direct search if the key ID is not prefixed with "id:". This will
>> look for an exact match on the key description.
>>
>> (4) Make x509_request_asymmetric_key() take a key ID. This is then converted
>> into "id:<hex>" and passed into keyring_search() where match preparsing
>> will turn it back into a binary ID.
>>
>> (5) X.509 certificate verification then takes the authority key ID and looks
>> up a key that matches it to find the public key for the certificate
>> signature.
>>
>> (6) PKCS#7 certificate verification then takes the id key ID and looks up a
>> key that matches it to find the public key for the signed information
>> block signature.
>>
>> Additional changes:
>>
>> (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
>> cert to be rejected with -EBADMSG.
>>
>> (2) The 'fingerprint' ID is gone. This was primarily intended to convey PGP
>> public key fingerprints. If PGP is supported in future, this should
>> generate a key ID that carries the fingerprint.
>>
>> (3) Th ca_keyid= kernel command line option is now converted to a key ID and
>> used to match the authority key ID. Possibly this should only match the
>> actual authKeyId part and not the issuer as well.
>>
>> Signed-off-by: David Howells <[email protected]>
>> ---
>>
>> crypto/asymmetric_keys/asymmetric_keys.h | 4 -
>> crypto/asymmetric_keys/asymmetric_type.c | 133 ++++++++++++-----------------
>> crypto/asymmetric_keys/pkcs7_parser.c | 38 ++++++--
>> crypto/asymmetric_keys/pkcs7_parser.h | 5 -
>> crypto/asymmetric_keys/pkcs7_trust.c | 6 -
>> crypto/asymmetric_keys/pkcs7_verify.c | 44 ++++------
>> crypto/asymmetric_keys/x509_cert_parser.c | 55 +++++++-----
>> crypto/asymmetric_keys/x509_parser.h | 5 +
>> crypto/asymmetric_keys/x509_public_key.c | 89 +++++++++++--------
>> include/crypto/public_key.h | 5 +
>> 10 files changed, 198 insertions(+), 186 deletions(-)
>>
>> diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
>> index 917be6b985e7..fd21ac28e0a0 100644
>> --- a/crypto/asymmetric_keys/asymmetric_keys.h
>> +++ b/crypto/asymmetric_keys/asymmetric_keys.h
>> @@ -9,13 +9,13 @@
>> * 2 of the Licence, or (at your option) any later version.
>> */
>>
>> -int asymmetric_keyid_match(const char *kid, const char *id);
>> extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
>> const struct asymmetric_key_id *match_id);
>>
>> extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
>>
>> -static inline const char *asymmetric_key_id(const struct key *key)
>> +static inline
>> +const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
>> {
>> return key->type_data.p[1];
>> }
>> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
>> index 3bc71b4e1eed..6f16f647d21b 100644
>> --- a/crypto/asymmetric_keys/asymmetric_type.c
>> +++ b/crypto/asymmetric_keys/asymmetric_type.c
>> @@ -105,76 +105,15 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
>> }
>>
>> /*
>> - * Match asymmetric key id with partial match
>> - * @id: key id to match in a form "id:<id>"
>> - */
>> -int asymmetric_keyid_match(const char *kid, const char *id)
>> -{
>> - size_t idlen, kidlen;
>> -
>> - if (!kid || !id)
>> - return 0;
>> -
>> - /* make it possible to use id as in the request: "id:<id>" */
>> - if (strncmp(id, "id:", 3) == 0)
>> - id += 3;
>> -
>> - /* Anything after here requires a partial match on the ID string */
>> - idlen = strlen(id);
>> - kidlen = strlen(kid);
>> - if (idlen > kidlen)
>> - return 0;
>> -
>> - kid += kidlen - idlen;
>> - if (strcasecmp(id, kid) != 0)
>> - return 0;
>> -
>> - return 1;
>> -}
>> -EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
>> -
>> -/*
>> - * Match asymmetric keys on (part of) their name
>> - * We have some shorthand methods for matching keys. We allow:
>> - *
>> - * "<desc>" - request a key by description
>> - * "id:<id>" - request a key matching the ID
>> - * "<subtype>:<id>" - request a key of a subtype
>> + * Match asymmetric keys by ID.
>> */
>> static bool asymmetric_key_cmp(const struct key *key,
>> const struct key_match_data *match_data)
>> {
>> - const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>> - const char *description = match_data->raw_data;
>> - const char *spec = description;
>> - const char *id;
>> - ptrdiff_t speclen;
>> -
>> - if (!subtype || !spec || !*spec)
>> - return 0;
>> -
>> - /* See if the full key description matches as is */
>> - if (key->description && strcmp(key->description, description) == 0)
>> - return 1;
>> -
>> - /* All tests from here on break the criterion description into a
>> - * specifier, a colon and then an identifier.
>> - */
>> - id = strchr(spec, ':');
>> - if (!id)
>> - return 0;
>> -
>> - speclen = id - spec;
>> - id++;
>> -
>> - if (speclen == 2 && memcmp(spec, "id", 2) == 0)
>> - return asymmetric_keyid_match(asymmetric_key_id(key), id);
>> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>> + const struct asymmetric_key_id *match_id = match_data->preparsed;
>>
>> - if (speclen == subtype->name_len &&
>> - memcmp(spec, subtype->name, speclen) == 0)
>> - return 1;
>> -
>> - return 0;
>> + return asymmetric_match_key_ids(kids, match_id);
>> }
>>
>> /*
>> @@ -191,8 +130,30 @@ static bool asymmetric_key_cmp(const struct key *key,
>> */
>> static int asymmetric_key_match_preparse(struct key_match_data *match_data)
>> {
>> - match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
>> + struct asymmetric_key_id *match_id;
>> + const char *spec = match_data->raw_data;
>> + const char *id;
>> +
>> + if (!spec || !*spec)
>> + return -EINVAL;
>> + if (spec[0] == 'i' &&
>> + spec[1] == 'd' &&
>> + spec[2] == ':') {
>> + id = spec + 3;
>> + } else {
>> + goto default_match;
>> + }
>> +
>> + match_id = asymmetric_key_hex_to_key_id(id);
>> + if (!match_id)
>> + return -ENOMEM;
>> +
>> + match_data->preparsed = match_id;
>> match_data->cmp = asymmetric_key_cmp;
>> + match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
>> + return 0;
>> +
>> +default_match:
>> return 0;
>> }
>>
>> @@ -201,6 +162,7 @@ static int asymmetric_key_match_preparse(struct key_match_data *match_data)
>> */
>> static void asymmetric_key_match_free(struct key_match_data *match_data)
>> {
>> + kfree(match_data->preparsed);
>> }
>>
>> /*
>> @@ -209,8 +171,10 @@ static void asymmetric_key_match_free(struct key_match_data *match_data)
>> static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
>> {
>> const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>> - const char *kid = asymmetric_key_id(key);
>> - size_t n;
>> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>> + const struct asymmetric_key_id *kid;
>> + const unsigned char *p;
>> + int n;
>>
>> seq_puts(m, key->description);
>>
>> @@ -218,13 +182,16 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
>> seq_puts(m, ": ");
>> subtype->describe(key, m);
>>
>> - if (kid) {
>> + if (kids && kids->id[0]) {
>> + kid = kids->id[0];
>> seq_putc(m, ' ');
>> - n = strlen(kid);
>> - if (n <= 8)
>> - seq_puts(m, kid);
>> - else
>> - seq_puts(m, kid + n - 8);
>> + n = kid->len;
>> + p = kid->data;
>> + if (n > 8) {
>> + p += n - 8;
>> + n = 8;
>> + }
>> + seq_printf(m, "%*phN", n, p);
>> }
>>
>> seq_puts(m, " [");
>> @@ -275,6 +242,7 @@ static int asymmetric_key_preparse(struct key_preparsed_payload *prep)
>> static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>> {
>> struct asymmetric_key_subtype *subtype = prep->type_data[0];
>> + struct asymmetric_key_ids *kids = prep->type_data[1];
>>
>> pr_devel("==>%s()\n", __func__);
>>
>> @@ -282,7 +250,11 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>> subtype->destroy(prep->payload[0]);
>> module_put(subtype->owner);
>> }
>> - kfree(prep->type_data[1]);
>> + if (kids) {
>> + kfree(kids->id[0]);
>> + kfree(kids->id[1]);
>> + kfree(kids);
>> + }
>> kfree(prep->description);
>> }
>>
>> @@ -292,13 +264,20 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>> static void asymmetric_key_destroy(struct key *key)
>> {
>> struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>> + struct asymmetric_key_ids *kids = key->type_data.p[1];
>> +
>> if (subtype) {
>> subtype->destroy(key->payload.data);
>> module_put(subtype->owner);
>> key->type_data.p[0] = NULL;
>> }
>> - kfree(key->type_data.p[1]);
>> - key->type_data.p[1] = NULL;
>> +
>> + if (kids) {
>> + kfree(kids->id[0]);
>> + kfree(kids->id[1]);
>> + kfree(kids);
>> + key->type_data.p[1] = NULL;
>> + }
>> }
>>
>> struct key_type key_type_asymmetric = {
>> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
>> index 459d2077c61b..ad6ae9d7c884 100644
>> --- a/crypto/asymmetric_keys/pkcs7_parser.c
>> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
>> @@ -29,6 +29,10 @@ struct pkcs7_parse_context {
>> enum OID last_oid; /* Last OID encountered */
>> unsigned x509_index;
>> unsigned sinfo_index;
>> + const void *raw_serial;
>> + unsigned raw_serial_size;
>> + unsigned raw_issuer_size;
>> + const void *raw_issuer;
>> };
>>
>> /*
>> @@ -39,6 +43,7 @@ static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
>> if (sinfo) {
>> mpi_free(sinfo->sig.mpi[0]);
>> kfree(sinfo->sig.digest);
>> + kfree(sinfo->signing_cert_id);
>> kfree(sinfo);
>> }
>> }
>> @@ -256,10 +261,10 @@ int pkcs7_extract_cert(void *context, size_t hdrlen,
>> if (IS_ERR(x509))
>> return PTR_ERR(x509);
>>
>> - pr_debug("Got cert for %s\n", x509->subject);
>> - pr_debug("- fingerprint %s\n", x509->fingerprint);
>> -
>> x509->index = ++ctx->x509_index;
>> + pr_debug("Got cert %u for %s\n", x509->index, x509->subject);
>> + pr_debug("- fingerprint %*phN\n", x509->id->len, x509->id->data);
>> +
>> *ctx->ppcerts = x509;
>> ctx->ppcerts = &x509->next;
>> return 0;
>> @@ -348,8 +353,8 @@ int pkcs7_sig_note_serial(void *context, size_t hdrlen,
>> const void *value, size_t vlen)
>> {
>> struct pkcs7_parse_context *ctx = context;
>> - ctx->sinfo->raw_serial = value;
>> - ctx->sinfo->raw_serial_size = vlen;
>> + ctx->raw_serial = value;
>> + ctx->raw_serial_size = vlen;
>> return 0;
>> }
>>
>> @@ -361,8 +366,8 @@ int pkcs7_sig_note_issuer(void *context, size_t hdrlen,
>> const void *value, size_t vlen)
>> {
>> struct pkcs7_parse_context *ctx = context;
>> - ctx->sinfo->raw_issuer = value;
>> - ctx->sinfo->raw_issuer_size = vlen;
>> + ctx->raw_issuer = value;
>> + ctx->raw_issuer_size = vlen;
>> return 0;
>> }
>>
>> @@ -395,10 +400,21 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
>> const void *value, size_t vlen)
>> {
>> struct pkcs7_parse_context *ctx = context;
>> -
>> - ctx->sinfo->index = ++ctx->sinfo_index;
>> - *ctx->ppsinfo = ctx->sinfo;
>> - ctx->ppsinfo = &ctx->sinfo->next;
>> + struct pkcs7_signed_info *sinfo = ctx->sinfo;
>> + struct asymmetric_key_id *kid;
>> +
>> + /* Generate cert issuer + serial number key ID */
>> + kid = asymmetric_key_generate_id(ctx->raw_serial,
>> + ctx->raw_serial_size,
>> + ctx->raw_issuer,
>> + ctx->raw_issuer_size);
>> + if (IS_ERR(kid))
>> + return PTR_ERR(kid);
>> +
>> + sinfo->signing_cert_id = kid;
>> + sinfo->index = ++ctx->sinfo_index;
>> + *ctx->ppsinfo = sinfo;
>> + ctx->ppsinfo = &sinfo->next;
>> ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
>> if (!ctx->sinfo)
>> return -ENOMEM;
>> diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
>> index d25f4d15370f..91949f92bc72 100644
>> --- a/crypto/asymmetric_keys/pkcs7_parser.h
>> +++ b/crypto/asymmetric_keys/pkcs7_parser.h
>> @@ -33,10 +33,7 @@ struct pkcs7_signed_info {
>> const void *authattrs;
>>
>> /* Issuing cert serial number and issuer's name */
>> - const void *raw_serial;
>> - unsigned raw_serial_size;
>> - unsigned raw_issuer_size;
>> - const void *raw_issuer;
>> + struct asymmetric_key_id *signing_cert_id;
>>
>> /* Message signature.
>> *
>> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
>> index e666eb011a85..4e8dd7214753 100644
>> --- a/crypto/asymmetric_keys/pkcs7_trust.c
>> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
>> @@ -49,8 +49,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>> /* Look to see if this certificate is present in the trusted
>> * keys.
>> */
>> - key = x509_request_asymmetric_key(trust_keyring, x509->subject,
>> - x509->fingerprint);
>> + key = x509_request_asymmetric_key(trust_keyring, x509->id);
>> if (!IS_ERR(key))
>> /* One of the X.509 certificates in the PKCS#7 message
>> * is apparently the same as one we already trust.
>> @@ -82,8 +81,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>> return -ENOKEY;
>> }
>>
>> - key = x509_request_asymmetric_key(trust_keyring, last->issuer,
>> - last->authority);
>> + key = x509_request_asymmetric_key(trust_keyring, last->authority);
>> if (IS_ERR(key))
>> return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
>> x509 = last;
>> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
>> index c62cf8006e1f..57e90fa17f2b 100644
>> --- a/crypto/asymmetric_keys/pkcs7_verify.c
>> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
>> @@ -131,8 +131,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>> struct x509_certificate *x509;
>> unsigned certix = 1;
>>
>> - kenter("%u,%u,%u",
>> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_issuer_size);
>> + kenter("%u", sinfo->index);
>>
>> for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
>> /* I'm _assuming_ that the generator of the PKCS#7 message will
>> @@ -140,21 +139,11 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>> * PKCS#7 message - but I can't be 100% sure of that. It's
>> * possible this will need element-by-element comparison.
>> */
>> - if (x509->raw_serial_size != sinfo->raw_serial_size ||
>> - memcmp(x509->raw_serial, sinfo->raw_serial,
>> - sinfo->raw_serial_size) != 0)
>> + if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
>> continue;
>> pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
>> sinfo->index, certix);
>>
>> - if (x509->raw_issuer_size != sinfo->raw_issuer_size ||
>> - memcmp(x509->raw_issuer, sinfo->raw_issuer,
>> - sinfo->raw_issuer_size) != 0) {
>> - pr_warn("Sig %u: X.509 subject and PKCS#7 issuer don't match\n",
>> - sinfo->index);
>> - continue;
>> - }
>> -
>> if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
>> pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
>> sinfo->index);
>> @@ -164,8 +153,10 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>> sinfo->signer = x509;
>> return 0;
>> }
>> +
>> pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
>> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_serial);
>> + sinfo->index,
>> + sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
>> return -ENOKEY;
>> }
>>
>> @@ -184,7 +175,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>> p->seen = false;
>>
>> for (;;) {
>> - pr_debug("verify %s: %s\n", x509->subject, x509->fingerprint);
>> + pr_debug("verify %s: %*phN\n",
>> + x509->subject,
>> + x509->raw_serial_size, x509->raw_serial);
>> x509->seen = true;
>> ret = x509_get_sig_params(x509);
>> if (ret < 0)
>> @@ -192,7 +185,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>>
>> pr_debug("- issuer %s\n", x509->issuer);
>> if (x509->authority)
>> - pr_debug("- authkeyid %s\n", x509->authority);
>> + pr_debug("- authkeyid %*phN\n",
>> + x509->authority->len, x509->authority->data);
>>
>> if (!x509->authority ||
>> strcmp(x509->subject, x509->issuer) == 0) {
>> @@ -218,13 +212,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>> /* Look through the X.509 certificates in the PKCS#7 message's
>> * list to see if the next one is there.
>> */
>> - pr_debug("- want %s\n", x509->authority);
>> + pr_debug("- want %*phN\n",
>> + x509->authority->len, x509->authority->data);
>> for (p = pkcs7->certs; p; p = p->next) {
>> - pr_debug("- cmp [%u] %s\n", p->index, p->fingerprint);
>> - if (p->raw_subject_size == x509->raw_issuer_size &&
>> - strcmp(p->fingerprint, x509->authority) == 0 &&
>> - memcmp(p->raw_subject, x509->raw_issuer,
>> - x509->raw_issuer_size) == 0)
>> + if (!p->skid)
>> + continue;
>> + pr_debug("- cmp [%u] %*phN\n",
>> + p->index, p->skid->len, p->skid->data);
>> + if (asymmetric_key_id_same(p->skid, x509->authority))
>> goto found_issuer;
>> }
>>
>> @@ -233,7 +228,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>> return 0;
>>
>> found_issuer:
>> - pr_debug("- issuer %s\n", p->subject);
>> + pr_debug("- subject %s\n", p->subject);
>> if (p->seen) {
>> pr_warn("Sig %u: X.509 chain contains loop\n",
>> sinfo->index);
>> @@ -304,7 +299,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
>> ret = x509_get_sig_params(x509);
>> if (ret < 0)
>> return ret;
>> - pr_debug("X.509[%u] %s\n", n, x509->authority);
>> + pr_debug("X.509[%u] %*phN\n",
>> + n, x509->authority->len, x509->authority->data);
>> }
>>
>> for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
>> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
>> index ac72348c186a..96151b2b91a2 100644
>> --- a/crypto/asymmetric_keys/x509_cert_parser.c
>> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
>> @@ -46,7 +46,8 @@ void x509_free_certificate(struct x509_certificate *cert)
>> public_key_destroy(cert->pub);
>> kfree(cert->issuer);
>> kfree(cert->subject);
>> - kfree(cert->fingerprint);
>> + kfree(cert->id);
>> + kfree(cert->skid);
>> kfree(cert->authority);
>> kfree(cert->sig.digest);
>> mpi_free(cert->sig.rsa.s);
>> @@ -62,6 +63,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>> {
>> struct x509_certificate *cert;
>> struct x509_parse_context *ctx;
>> + struct asymmetric_key_id *kid;
>> long ret;
>>
>> ret = -ENOMEM;
>> @@ -89,6 +91,17 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>> if (ret < 0)
>> goto error_decode;
>>
>> + /* Generate cert issuer + serial number key ID */
>> + kid = asymmetric_key_generate_id(cert->raw_serial,
>> + cert->raw_serial_size,
>> + cert->raw_issuer,
>> + cert->raw_issuer_size);
>> + if (IS_ERR(kid)) {
>> + ret = PTR_ERR(kid);
>> + goto error_decode;
>> + }
>> + cert->id = kid;
>> +
>> kfree(ctx);
>> return cert;
>>
>> @@ -407,36 +420,34 @@ int x509_process_extension(void *context, size_t hdrlen,
>> const void *value, size_t vlen)
>> {
>> struct x509_parse_context *ctx = context;
>> + struct asymmetric_key_id *kid;
>> const unsigned char *v = value;
>> - char *f;
>> int i;
>>
>> pr_debug("Extension: %u\n", ctx->last_oid);
>>
>> if (ctx->last_oid == OID_subjectKeyIdentifier) {
>> /* Get hold of the key fingerprint */
>> - if (vlen < 3)
>> + if (ctx->cert->skid || vlen < 3)
>> return -EBADMSG;
>> if (v[0] != ASN1_OTS || v[1] != vlen - 2)
>> return -EBADMSG;
>> v += 2;
>> vlen -= 2;
>>
>> - f = kmalloc(vlen * 2 + 1, GFP_KERNEL);
>> - if (!f)
>> - return -ENOMEM;
>> - for (i = 0; i < vlen; i++)
>> - sprintf(f + i * 2, "%02x", v[i]);
>> - pr_debug("fingerprint %s\n", f);
>> - ctx->cert->fingerprint = f;
>> + kid = asymmetric_key_generate_id(v, vlen,
>> + ctx->cert->raw_subject,
>> + ctx->cert->raw_subject_size);
>> + if (IS_ERR(kid))
>> + return PTR_ERR(kid);
>> + ctx->cert->skid = kid;
>> + pr_debug("subjkeyid %*phN\n", kid->len, kid->data);
>> return 0;
>> }
>>
>> if (ctx->last_oid == OID_authorityKeyIdentifier) {
>> - size_t key_len;
>> -
>> /* Get hold of the CA key fingerprint */
>> - if (vlen < 5)
>> + if (ctx->cert->authority || vlen < 5)
>> return -EBADMSG;
>>
>> /* Authority Key Identifier must be a Constructed SEQUENCE */
>> @@ -454,7 +465,7 @@ int x509_process_extension(void *context, size_t hdrlen,
>> v[3] > vlen - 4)
>> return -EBADMSG;
>>
>> - key_len = v[3];
>> + vlen = v[3];
>> v += 4;
>> } else {
>> /* Long Form length */
>> @@ -476,17 +487,17 @@ int x509_process_extension(void *context, size_t hdrlen,
>> v[sub + 1] > vlen - 4 - sub)
>> return -EBADMSG;
>>
>> - key_len = v[sub + 1];
>> + vlen = v[sub + 1];
>> v += (sub + 2);
>> }
>>
>> - f = kmalloc(key_len * 2 + 1, GFP_KERNEL);
>> - if (!f)
>> - return -ENOMEM;
>> - for (i = 0; i < key_len; i++)
>> - sprintf(f + i * 2, "%02x", v[i]);
>> - pr_debug("authority %s\n", f);
>> - ctx->cert->authority = f;
>> + kid = asymmetric_key_generate_id(v, vlen,
>> + ctx->cert->raw_issuer,
>> + ctx->cert->raw_issuer_size);
>> + if (IS_ERR(kid))
>> + return PTR_ERR(kid);
>> + pr_debug("authkeyid %*phN\n", kid->len, kid->data);
>> + ctx->cert->authority = kid;
>> return 0;
>> }
>>
>> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
>> index 1b76f207c1f3..0e8d59b010fb 100644
>> --- a/crypto/asymmetric_keys/x509_parser.h
>> +++ b/crypto/asymmetric_keys/x509_parser.h
>> @@ -19,8 +19,9 @@ struct x509_certificate {
>> struct public_key_signature sig; /* Signature parameters */
>> char *issuer; /* Name of certificate issuer */
>> char *subject; /* Name of certificate subject */
>> - char *fingerprint; /* Key fingerprint as hex */
>> - char *authority; /* Authority key fingerprint as hex */
>> + struct asymmetric_key_id *id; /* Issuer + serial number */
>> + struct asymmetric_key_id *skid; /* Subject key identifier */
>> + struct asymmetric_key_id *authority; /* Authority key identifier */
>> struct tm valid_from;
>> struct tm valid_to;
>> const void *tbs; /* Signed data */
>> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
>> index f3d62307e6ee..c60905c3f4d2 100644
>> --- a/crypto/asymmetric_keys/x509_public_key.c
>> +++ b/crypto/asymmetric_keys/x509_public_key.c
>> @@ -25,7 +25,7 @@
>> #include "x509_parser.h"
>>
>> static bool use_builtin_keys;
>> -static char *ca_keyid;
>> +static struct asymmetric_key_id *ca_keyid;
>>
>> #ifndef MODULE
>> static int __init ca_keys_setup(char *str)
>> @@ -33,10 +33,16 @@ static int __init ca_keys_setup(char *str)
>> if (!str) /* default system keyring */
>> return 1;
>>
>> - if (strncmp(str, "id:", 3) == 0)
>> - ca_keyid = str; /* owner key 'id:xxxxxx' */
>> - else if (strcmp(str, "builtin") == 0)
>> + if (strncmp(str, "id:", 3) == 0) {
>> + struct asymmetric_key_id *p;
>> + p = asymmetric_key_hex_to_key_id(str);
>> + if (p == ERR_PTR(-EINVAL))
>> + pr_err("Unparsable hex string in ca_keys\n");
>> + else if (!IS_ERR(p))
>> + ca_keyid = p; /* owner key 'id:xxxxxx' */
>> + } else if (strcmp(str, "builtin") == 0) {
>> use_builtin_keys = true;
>> + }
>>
>> return 1;
>> }
>> @@ -46,31 +52,28 @@ __setup("ca_keys=", ca_keys_setup);
>> /**
>> * x509_request_asymmetric_key - Request a key by X.509 certificate params.
>> * @keyring: The keys to search.
>> - * @subject: The name of the subject to whom the key belongs.
>> - * @key_id: The subject key ID as a hex string.
>> + * @kid: The key ID.
>> *
>> * Find a key in the given keyring by subject name and key ID. These might,
>> * for instance, be the issuer name and the authority key ID of an X.509
>> * certificate that needs to be verified.
>> */
>> struct key *x509_request_asymmetric_key(struct key *keyring,
>> - const char *subject,
>> - const char *key_id)
>> + const struct asymmetric_key_id *kid)
>> {
>> key_ref_t key;
>> - size_t subject_len = strlen(subject), key_id_len = strlen(key_id);
>> - char *id;
>> + char *id, *p;
>>
>> - /* Construct an identifier "<subjname>:<keyid>". */
>> - id = kmalloc(subject_len + 2 + key_id_len + 1, GFP_KERNEL);
>> + /* Construct an identifier "id:<keyid>". */
>> + p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
>> if (!id)
>> return ERR_PTR(-ENOMEM);
>>
>> - memcpy(id, subject, subject_len);
>> - id[subject_len + 0] = ':';
>> - id[subject_len + 1] = ' ';
>> - memcpy(id + subject_len + 2, key_id, key_id_len);
>> - id[subject_len + 2 + key_id_len] = 0;
>> + *p++ = 'i';
>> + *p++ = 'd';
>> + *p++ = ':';
>> + p = bin2hex(p, kid->data, kid->len);
>> + *p = 0;
>>
>> pr_debug("Look up: \"%s\"\n", id);
>>
>> @@ -195,11 +198,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
>> if (!trust_keyring)
>> return -EOPNOTSUPP;
>>
>> - if (ca_keyid && !asymmetric_keyid_match(cert->authority, ca_keyid))
>> + if (ca_keyid && !asymmetric_key_id_same(cert->authority, ca_keyid))
>> return -EPERM;
>>
>> - key = x509_request_asymmetric_key(trust_keyring,
>> - cert->issuer, cert->authority);
>> + key = x509_request_asymmetric_key(trust_keyring, cert->authority);
>> if (!IS_ERR(key)) {
>> if (!use_builtin_keys
>> || test_bit(KEY_FLAG_BUILTIN, &key->flags))
>> @@ -214,9 +216,11 @@ static int x509_validate_trust(struct x509_certificate *cert,
>> */
>> static int x509_key_preparse(struct key_preparsed_payload *prep)
>> {
>> + struct asymmetric_key_ids *kids;
>> struct x509_certificate *cert;
>> + const char *q;
>> size_t srlen, sulen;
>> - char *desc = NULL;
>> + char *desc = NULL, *p;
>> int ret;
>>
>> cert = x509_cert_parse(prep->data, prep->datalen);
>> @@ -249,19 +253,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>> pkey_algo_name[cert->sig.pkey_algo],
>> hash_algo_name[cert->sig.pkey_hash_algo]);
>>
>> - if (!cert->fingerprint) {
>> - pr_warn("Cert for '%s' must have a SubjKeyId extension\n",
>> - cert->subject);
>> - ret = -EKEYREJECTED;
>> - goto error_free_cert;
>> - }
>> -
>> cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
>> cert->pub->id_type = PKEY_ID_X509;
>>
>> /* Check the signature on the key if it appears to be self-signed */
>> if (!cert->authority ||
>> - strcmp(cert->fingerprint, cert->authority) == 0) {
>> + asymmetric_key_id_same(cert->skid, cert->authority)) {
>> ret = x509_check_signature(cert->pub, cert); /* self-signed */
>> if (ret < 0)
>> goto error_free_cert;
>> @@ -273,31 +270,47 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>>
>> /* Propose a description */
>> sulen = strlen(cert->subject);
>> - srlen = strlen(cert->fingerprint);
>> + srlen = cert->raw_serial_size;
>> + q = cert->raw_serial;
>> + if (srlen > 1 && *q == 0) {
>> + srlen--;
>> + q++;
>> + }
>> +
>> ret = -ENOMEM;
>> - desc = kmalloc(sulen + 2 + srlen + 1, GFP_KERNEL);
>> + desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
>> if (!desc)
>> goto error_free_cert;
>> - memcpy(desc, cert->subject, sulen);
>> - desc[sulen] = ':';
>> - desc[sulen + 1] = ' ';
>> - memcpy(desc + sulen + 2, cert->fingerprint, srlen);
>> - desc[sulen + 2 + srlen] = 0;
>> + p = memcpy(desc, cert->subject, sulen);
>> + p += sulen;
>> + *p++ = ':';
>> + *p++ = ' ';
>> + p = bin2hex(p, q, srlen);
>> + *p = 0;
>> +
>> + kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
>> + if (!kids)
>> + goto error_free_desc;
>> + kids->id[0] = cert->id;
>> + kids->id[1] = cert->skid;
>>
>> /* We're pinning the module by being linked against it */
>> __module_get(public_key_subtype.owner);
>> prep->type_data[0] = &public_key_subtype;
>> - prep->type_data[1] = cert->fingerprint;
>> + prep->type_data[1] = kids;
>> prep->payload[0] = cert->pub;
>> prep->description = desc;
>> prep->quotalen = 100;
>>
>> /* We've finished with the certificate */
>> cert->pub = NULL;
>> - cert->fingerprint = NULL;
>> + cert->id = NULL;
>> + cert->skid = NULL;
>> desc = NULL;
>> ret = 0;
>>
>> +error_free_desc:
>> + kfree(desc);
>> error_free_cert:
>> x509_free_certificate(cert);
>> return ret;
>> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
>> index 0d164c6af539..fa73a6fd536c 100644
>> --- a/include/crypto/public_key.h
>> +++ b/include/crypto/public_key.h
>> @@ -15,6 +15,7 @@
>> #define _LINUX_PUBLIC_KEY_H
>>
>> #include <linux/mpi.h>
>> +#include <keys/asymmetric-type.h>
>> #include <crypto/hash_info.h>
>>
>> enum pkey_algo {
>> @@ -98,8 +99,8 @@ struct key;
>> extern int verify_signature(const struct key *key,
>> const struct public_key_signature *sig);
>>
>> +struct asymmetric_key_id;
>> extern struct key *x509_request_asymmetric_key(struct key *keyring,
>> - const char *issuer,
>> - const char *key_id);
>> + const struct asymmetric_key_id *kid);
>>
>> #endif /* _LINUX_PUBLIC_KEY_H */
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-10-02 18:32:34

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

On 2 October 2014 19:04, Dmitry Kasatkin <[email protected]> wrote:
> On 02/10/14 18:49, Dmitry Kasatkin wrote:
>> Hi David,
>>
>> I just took latest #next branch from James's security tree which
>> includes latest KEYs patches and noticed following:
>>
>> [ 9.812332] Request for unknown module key 'Magrathea: Glacier
>> signing key: 926305d6dda66f47139eb4e3cb25a6adef527f08' err -11
>>
>> Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
>> also has changed in respect of certificate ids..
>>
>> Those ids does not look any close to my kernel X509 X509v3 Subject Key
>> Identifier, which is:
>> 92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08
>>
>> proc/keys shows
>>
>> symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
>> 6865612e68326732 []
>>


Ok.. d9e2e4c6951f1e83 is serial number.

What is this 6865612e68326732?

Does it still support searching for the key by partially matching
Subject Key Identifier?


- Dmitry

>> Very different ids..
>>
>> How could I match certificate now?
>> Module verification code cannot find needed key..
>>
>> - Dmitry
>
>
> Hehe. Also now I get kernel Oops in asymmetric_key_id_same...
>
> -------------------------
> [ 132.816522] BUG: unable to handle kernel paging request at
> ffffffffffffffea
> [ 132.819902] IP: [<ffffffff812bfc20>] asymmetric_key_id_same+0x14/0x36
> [ 132.820302] PGD 1a12067 PUD 1a14067 PMD 0
> [ 132.820302] Oops: 0000 [#1] SMP
> [ 132.820302] Modules linked in: bridge(E) stp(E) llc(E) evdev(E)
> serio_raw(E) i2c_piix4(E) button(E) fuse(E)
> [ 132.820302] CPU: 0 PID: 2993 Comm: cat Tainted: G E
> 3.16.0-kds+ #2847
> [ 132.820302] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 132.820302] task: ffff88004249a430 ti: ffff880056640000 task.ti:
> ffff880056640000
> [ 132.820302] RIP: 0010:[<ffffffff812bfc20>] [<ffffffff812bfc20>]
> asymmetric_key_id_same+0x14/0x36
> [ 132.820302] RSP: 0018:ffff880056643930 EFLAGS: 00010246
> [ 132.820302] RAX: 0000000000000000 RBX: ffffffffffffffea RCX:
> ffff880056643ae0
> [ 132.820302] RDX: 000000000000005e RSI: ffffffffffffffea RDI:
> ffff88005bac9300
> [ 132.820302] RBP: ffff880056643948 R08: 0000000000000003 R09:
> 00000007504aa01a
> [ 132.820302] R10: 0000000000000000 R11: 0000000000000000 R12:
> ffff88005d68ca40
> [ 132.820302] R13: 0000000000000101 R14: 0000000000000000 R15:
> ffff88005bac5280
> [ 132.820302] FS: 00007f67a153c740(0000) GS:ffff88005da00000(0000)
> knlGS:0000000000000000
> [ 132.820302] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 132.820302] CR2: ffffffffffffffea CR3: 000000002e663000 CR4:
> 00000000000006f0
> [ 132.820302] Stack:
> [ 132.820302] ffffffff812bfc66 ffff880056643ae0 ffff88005bac5280
> ffff880056643958
> [ 132.820302] ffffffff812bfc9d ffff880056643980 ffffffff812971d9
> ffff88005ce930c1
> [ 132.820302] ffff88005ce930c0 0000000000000000 ffff8800566439c8
> ffffffff812fb753
> [ 132.820302] Call Trace:
> [ 132.820302] [<ffffffff812bfc66>] ? asymmetric_match_key_ids+0x24/0x42
> [ 132.820302] [<ffffffff812bfc9d>] asymmetric_key_cmp+0x19/0x1b
> [ 132.820302] [<ffffffff812971d9>] keyring_search_iterator+0x74/0xd7
> [ 132.820302] [<ffffffff812fb753>] assoc_array_subtree_iterate+0x67/0xd2
> [ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
> [ 132.820302] [<ffffffff812fbaa1>] assoc_array_iterate+0x19/0x1e
> [ 132.820302] [<ffffffff81297332>] search_nested_keyrings+0xf6/0x2b6
> [ 132.820302] [<ffffffff810728da>] ? sched_clock_cpu+0x91/0xa2
> [ 132.820302] [<ffffffff810860d2>] ? mark_held_locks+0x58/0x6e
> [ 132.820302] [<ffffffff810a137d>] ? current_kernel_time+0x77/0xb8
> [ 132.820302] [<ffffffff81297871>] keyring_search_aux+0xe1/0x14c
> [ 132.820302] [<ffffffff812977fc>] ? keyring_search_aux+0x6c/0x14c
> [ 132.820302] [<ffffffff8129796b>] keyring_search+0x8f/0xb6
> [ 132.820302] [<ffffffff812bfc84>] ? asymmetric_match_key_ids+0x42/0x42
> [ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
> [ 132.820302] [<ffffffff812ab9e3>] asymmetric_verify+0xa4/0x214
> [ 132.820302] [<ffffffff812ab90e>] integrity_digsig_verify+0xb1/0xe2
> [ 132.820302] [<ffffffff812abe41>] ? evm_verifyxattr+0x6a/0x7a
> [ 132.820302] [<ffffffff812b0390>] ima_appraise_measurement+0x160/0x370
> [ 132.820302] [<ffffffff81161db2>] ? d_absolute_path+0x5b/0x7a
> [ 132.820302] [<ffffffff812ada30>] process_measurement+0x322/0x404
>
>
>> On 08/09/14 18:38, David Howells wrote:
>>> Make use of the new match string preparsing to overhaul key identification
>>> when searching for asymmetric keys. The following changes are made:
>>>
>>> (1) Use the previously created asymmetric_key_id struct to hold the following
>>> key IDs derived from the X.509 certificate or PKCS#7 message:
>>>
>>> id: serial number + issuer
>>> skid: subjKeyId + subject
>>> authority: authKeyId + issuer
>>>
>>> (2) Replace the hex fingerprint attached to key->type_data[1] with an
>>> asymmetric_key_ids struct containing the id and the skid (if present).
>>>
>>> (3) Make the asymmetric_type match data preparse select one of two searches:
>>>
>>> (a) An iterative search for the key ID given if prefixed with "id:". The
>>> prefix is expected to be followed by a hex string giving the ID to
>>> search for. The criterion key ID is checked against all key IDs
>>> recorded on the key.
>>>
>>> (b) A direct search if the key ID is not prefixed with "id:". This will
>>> look for an exact match on the key description.
>>>
>>> (4) Make x509_request_asymmetric_key() take a key ID. This is then converted
>>> into "id:<hex>" and passed into keyring_search() where match preparsing
>>> will turn it back into a binary ID.
>>>
>>> (5) X.509 certificate verification then takes the authority key ID and looks
>>> up a key that matches it to find the public key for the certificate
>>> signature.
>>>
>>> (6) PKCS#7 certificate verification then takes the id key ID and looks up a
>>> key that matches it to find the public key for the signed information
>>> block signature.
>>>
>>> Additional changes:
>>>
>>> (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
>>> cert to be rejected with -EBADMSG.
>>>
>>> (2) The 'fingerprint' ID is gone. This was primarily intended to convey PGP
>>> public key fingerprints. If PGP is supported in future, this should
>>> generate a key ID that carries the fingerprint.
>>>
>>> (3) Th ca_keyid= kernel command line option is now converted to a key ID and
>>> used to match the authority key ID. Possibly this should only match the
>>> actual authKeyId part and not the issuer as well.
>>>
>>> Signed-off-by: David Howells <[email protected]>
>>> ---
>>>
>>> crypto/asymmetric_keys/asymmetric_keys.h | 4 -
>>> crypto/asymmetric_keys/asymmetric_type.c | 133 ++++++++++++-----------------
>>> crypto/asymmetric_keys/pkcs7_parser.c | 38 ++++++--
>>> crypto/asymmetric_keys/pkcs7_parser.h | 5 -
>>> crypto/asymmetric_keys/pkcs7_trust.c | 6 -
>>> crypto/asymmetric_keys/pkcs7_verify.c | 44 ++++------
>>> crypto/asymmetric_keys/x509_cert_parser.c | 55 +++++++-----
>>> crypto/asymmetric_keys/x509_parser.h | 5 +
>>> crypto/asymmetric_keys/x509_public_key.c | 89 +++++++++++--------
>>> include/crypto/public_key.h | 5 +
>>> 10 files changed, 198 insertions(+), 186 deletions(-)
>>>
>>> diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
>>> index 917be6b985e7..fd21ac28e0a0 100644
>>> --- a/crypto/asymmetric_keys/asymmetric_keys.h
>>> +++ b/crypto/asymmetric_keys/asymmetric_keys.h
>>> @@ -9,13 +9,13 @@
>>> * 2 of the Licence, or (at your option) any later version.
>>> */
>>>
>>> -int asymmetric_keyid_match(const char *kid, const char *id);
>>> extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
>>> const struct asymmetric_key_id *match_id);
>>>
>>> extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
>>>
>>> -static inline const char *asymmetric_key_id(const struct key *key)
>>> +static inline
>>> +const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
>>> {
>>> return key->type_data.p[1];
>>> }
>>> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
>>> index 3bc71b4e1eed..6f16f647d21b 100644
>>> --- a/crypto/asymmetric_keys/asymmetric_type.c
>>> +++ b/crypto/asymmetric_keys/asymmetric_type.c
>>> @@ -105,76 +105,15 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
>>> }
>>>
>>> /*
>>> - * Match asymmetric key id with partial match
>>> - * @id: key id to match in a form "id:<id>"
>>> - */
>>> -int asymmetric_keyid_match(const char *kid, const char *id)
>>> -{
>>> - size_t idlen, kidlen;
>>> -
>>> - if (!kid || !id)
>>> - return 0;
>>> -
>>> - /* make it possible to use id as in the request: "id:<id>" */
>>> - if (strncmp(id, "id:", 3) == 0)
>>> - id += 3;
>>> -
>>> - /* Anything after here requires a partial match on the ID string */
>>> - idlen = strlen(id);
>>> - kidlen = strlen(kid);
>>> - if (idlen > kidlen)
>>> - return 0;
>>> -
>>> - kid += kidlen - idlen;
>>> - if (strcasecmp(id, kid) != 0)
>>> - return 0;
>>> -
>>> - return 1;
>>> -}
>>> -EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
>>> -
>>> -/*
>>> - * Match asymmetric keys on (part of) their name
>>> - * We have some shorthand methods for matching keys. We allow:
>>> - *
>>> - * "<desc>" - request a key by description
>>> - * "id:<id>" - request a key matching the ID
>>> - * "<subtype>:<id>" - request a key of a subtype
>>> + * Match asymmetric keys by ID.
>>> */
>>> static bool asymmetric_key_cmp(const struct key *key,
>>> const struct key_match_data *match_data)
>>> {
>>> - const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>>> - const char *description = match_data->raw_data;
>>> - const char *spec = description;
>>> - const char *id;
>>> - ptrdiff_t speclen;
>>> -
>>> - if (!subtype || !spec || !*spec)
>>> - return 0;
>>> -
>>> - /* See if the full key description matches as is */
>>> - if (key->description && strcmp(key->description, description) == 0)
>>> - return 1;
>>> -
>>> - /* All tests from here on break the criterion description into a
>>> - * specifier, a colon and then an identifier.
>>> - */
>>> - id = strchr(spec, ':');
>>> - if (!id)
>>> - return 0;
>>> -
>>> - speclen = id - spec;
>>> - id++;
>>> -
>>> - if (speclen == 2 && memcmp(spec, "id", 2) == 0)
>>> - return asymmetric_keyid_match(asymmetric_key_id(key), id);
>>> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>>> + const struct asymmetric_key_id *match_id = match_data->preparsed;
>>>
>>> - if (speclen == subtype->name_len &&
>>> - memcmp(spec, subtype->name, speclen) == 0)
>>> - return 1;
>>> -
>>> - return 0;
>>> + return asymmetric_match_key_ids(kids, match_id);
>>> }
>>>
>>> /*
>>> @@ -191,8 +130,30 @@ static bool asymmetric_key_cmp(const struct key *key,
>>> */
>>> static int asymmetric_key_match_preparse(struct key_match_data *match_data)
>>> {
>>> - match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
>>> + struct asymmetric_key_id *match_id;
>>> + const char *spec = match_data->raw_data;
>>> + const char *id;
>>> +
>>> + if (!spec || !*spec)
>>> + return -EINVAL;
>>> + if (spec[0] == 'i' &&
>>> + spec[1] == 'd' &&
>>> + spec[2] == ':') {
>>> + id = spec + 3;
>>> + } else {
>>> + goto default_match;
>>> + }
>>> +
>>> + match_id = asymmetric_key_hex_to_key_id(id);
>>> + if (!match_id)
>>> + return -ENOMEM;
>>> +
>>> + match_data->preparsed = match_id;
>>> match_data->cmp = asymmetric_key_cmp;
>>> + match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
>>> + return 0;
>>> +
>>> +default_match:
>>> return 0;
>>> }
>>>
>>> @@ -201,6 +162,7 @@ static int asymmetric_key_match_preparse(struct key_match_data *match_data)
>>> */
>>> static void asymmetric_key_match_free(struct key_match_data *match_data)
>>> {
>>> + kfree(match_data->preparsed);
>>> }
>>>
>>> /*
>>> @@ -209,8 +171,10 @@ static void asymmetric_key_match_free(struct key_match_data *match_data)
>>> static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
>>> {
>>> const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>>> - const char *kid = asymmetric_key_id(key);
>>> - size_t n;
>>> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>>> + const struct asymmetric_key_id *kid;
>>> + const unsigned char *p;
>>> + int n;
>>>
>>> seq_puts(m, key->description);
>>>
>>> @@ -218,13 +182,16 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
>>> seq_puts(m, ": ");
>>> subtype->describe(key, m);
>>>
>>> - if (kid) {
>>> + if (kids && kids->id[0]) {
>>> + kid = kids->id[0];
>>> seq_putc(m, ' ');
>>> - n = strlen(kid);
>>> - if (n <= 8)
>>> - seq_puts(m, kid);
>>> - else
>>> - seq_puts(m, kid + n - 8);
>>> + n = kid->len;
>>> + p = kid->data;
>>> + if (n > 8) {
>>> + p += n - 8;
>>> + n = 8;
>>> + }
>>> + seq_printf(m, "%*phN", n, p);
>>> }
>>>
>>> seq_puts(m, " [");
>>> @@ -275,6 +242,7 @@ static int asymmetric_key_preparse(struct key_preparsed_payload *prep)
>>> static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>>> {
>>> struct asymmetric_key_subtype *subtype = prep->type_data[0];
>>> + struct asymmetric_key_ids *kids = prep->type_data[1];
>>>
>>> pr_devel("==>%s()\n", __func__);
>>>
>>> @@ -282,7 +250,11 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>>> subtype->destroy(prep->payload[0]);
>>> module_put(subtype->owner);
>>> }
>>> - kfree(prep->type_data[1]);
>>> + if (kids) {
>>> + kfree(kids->id[0]);
>>> + kfree(kids->id[1]);
>>> + kfree(kids);
>>> + }
>>> kfree(prep->description);
>>> }
>>>
>>> @@ -292,13 +264,20 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
>>> static void asymmetric_key_destroy(struct key *key)
>>> {
>>> struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>>> + struct asymmetric_key_ids *kids = key->type_data.p[1];
>>> +
>>> if (subtype) {
>>> subtype->destroy(key->payload.data);
>>> module_put(subtype->owner);
>>> key->type_data.p[0] = NULL;
>>> }
>>> - kfree(key->type_data.p[1]);
>>> - key->type_data.p[1] = NULL;
>>> +
>>> + if (kids) {
>>> + kfree(kids->id[0]);
>>> + kfree(kids->id[1]);
>>> + kfree(kids);
>>> + key->type_data.p[1] = NULL;
>>> + }
>>> }
>>>
>>> struct key_type key_type_asymmetric = {
>>> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
>>> index 459d2077c61b..ad6ae9d7c884 100644
>>> --- a/crypto/asymmetric_keys/pkcs7_parser.c
>>> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
>>> @@ -29,6 +29,10 @@ struct pkcs7_parse_context {
>>> enum OID last_oid; /* Last OID encountered */
>>> unsigned x509_index;
>>> unsigned sinfo_index;
>>> + const void *raw_serial;
>>> + unsigned raw_serial_size;
>>> + unsigned raw_issuer_size;
>>> + const void *raw_issuer;
>>> };
>>>
>>> /*
>>> @@ -39,6 +43,7 @@ static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
>>> if (sinfo) {
>>> mpi_free(sinfo->sig.mpi[0]);
>>> kfree(sinfo->sig.digest);
>>> + kfree(sinfo->signing_cert_id);
>>> kfree(sinfo);
>>> }
>>> }
>>> @@ -256,10 +261,10 @@ int pkcs7_extract_cert(void *context, size_t hdrlen,
>>> if (IS_ERR(x509))
>>> return PTR_ERR(x509);
>>>
>>> - pr_debug("Got cert for %s\n", x509->subject);
>>> - pr_debug("- fingerprint %s\n", x509->fingerprint);
>>> -
>>> x509->index = ++ctx->x509_index;
>>> + pr_debug("Got cert %u for %s\n", x509->index, x509->subject);
>>> + pr_debug("- fingerprint %*phN\n", x509->id->len, x509->id->data);
>>> +
>>> *ctx->ppcerts = x509;
>>> ctx->ppcerts = &x509->next;
>>> return 0;
>>> @@ -348,8 +353,8 @@ int pkcs7_sig_note_serial(void *context, size_t hdrlen,
>>> const void *value, size_t vlen)
>>> {
>>> struct pkcs7_parse_context *ctx = context;
>>> - ctx->sinfo->raw_serial = value;
>>> - ctx->sinfo->raw_serial_size = vlen;
>>> + ctx->raw_serial = value;
>>> + ctx->raw_serial_size = vlen;
>>> return 0;
>>> }
>>>
>>> @@ -361,8 +366,8 @@ int pkcs7_sig_note_issuer(void *context, size_t hdrlen,
>>> const void *value, size_t vlen)
>>> {
>>> struct pkcs7_parse_context *ctx = context;
>>> - ctx->sinfo->raw_issuer = value;
>>> - ctx->sinfo->raw_issuer_size = vlen;
>>> + ctx->raw_issuer = value;
>>> + ctx->raw_issuer_size = vlen;
>>> return 0;
>>> }
>>>
>>> @@ -395,10 +400,21 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
>>> const void *value, size_t vlen)
>>> {
>>> struct pkcs7_parse_context *ctx = context;
>>> -
>>> - ctx->sinfo->index = ++ctx->sinfo_index;
>>> - *ctx->ppsinfo = ctx->sinfo;
>>> - ctx->ppsinfo = &ctx->sinfo->next;
>>> + struct pkcs7_signed_info *sinfo = ctx->sinfo;
>>> + struct asymmetric_key_id *kid;
>>> +
>>> + /* Generate cert issuer + serial number key ID */
>>> + kid = asymmetric_key_generate_id(ctx->raw_serial,
>>> + ctx->raw_serial_size,
>>> + ctx->raw_issuer,
>>> + ctx->raw_issuer_size);
>>> + if (IS_ERR(kid))
>>> + return PTR_ERR(kid);
>>> +
>>> + sinfo->signing_cert_id = kid;
>>> + sinfo->index = ++ctx->sinfo_index;
>>> + *ctx->ppsinfo = sinfo;
>>> + ctx->ppsinfo = &sinfo->next;
>>> ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
>>> if (!ctx->sinfo)
>>> return -ENOMEM;
>>> diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
>>> index d25f4d15370f..91949f92bc72 100644
>>> --- a/crypto/asymmetric_keys/pkcs7_parser.h
>>> +++ b/crypto/asymmetric_keys/pkcs7_parser.h
>>> @@ -33,10 +33,7 @@ struct pkcs7_signed_info {
>>> const void *authattrs;
>>>
>>> /* Issuing cert serial number and issuer's name */
>>> - const void *raw_serial;
>>> - unsigned raw_serial_size;
>>> - unsigned raw_issuer_size;
>>> - const void *raw_issuer;
>>> + struct asymmetric_key_id *signing_cert_id;
>>>
>>> /* Message signature.
>>> *
>>> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
>>> index e666eb011a85..4e8dd7214753 100644
>>> --- a/crypto/asymmetric_keys/pkcs7_trust.c
>>> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
>>> @@ -49,8 +49,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>>> /* Look to see if this certificate is present in the trusted
>>> * keys.
>>> */
>>> - key = x509_request_asymmetric_key(trust_keyring, x509->subject,
>>> - x509->fingerprint);
>>> + key = x509_request_asymmetric_key(trust_keyring, x509->id);
>>> if (!IS_ERR(key))
>>> /* One of the X.509 certificates in the PKCS#7 message
>>> * is apparently the same as one we already trust.
>>> @@ -82,8 +81,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>>> return -ENOKEY;
>>> }
>>>
>>> - key = x509_request_asymmetric_key(trust_keyring, last->issuer,
>>> - last->authority);
>>> + key = x509_request_asymmetric_key(trust_keyring, last->authority);
>>> if (IS_ERR(key))
>>> return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
>>> x509 = last;
>>> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
>>> index c62cf8006e1f..57e90fa17f2b 100644
>>> --- a/crypto/asymmetric_keys/pkcs7_verify.c
>>> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
>>> @@ -131,8 +131,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>>> struct x509_certificate *x509;
>>> unsigned certix = 1;
>>>
>>> - kenter("%u,%u,%u",
>>> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_issuer_size);
>>> + kenter("%u", sinfo->index);
>>>
>>> for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
>>> /* I'm _assuming_ that the generator of the PKCS#7 message will
>>> @@ -140,21 +139,11 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>>> * PKCS#7 message - but I can't be 100% sure of that. It's
>>> * possible this will need element-by-element comparison.
>>> */
>>> - if (x509->raw_serial_size != sinfo->raw_serial_size ||
>>> - memcmp(x509->raw_serial, sinfo->raw_serial,
>>> - sinfo->raw_serial_size) != 0)
>>> + if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
>>> continue;
>>> pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
>>> sinfo->index, certix);
>>>
>>> - if (x509->raw_issuer_size != sinfo->raw_issuer_size ||
>>> - memcmp(x509->raw_issuer, sinfo->raw_issuer,
>>> - sinfo->raw_issuer_size) != 0) {
>>> - pr_warn("Sig %u: X.509 subject and PKCS#7 issuer don't match\n",
>>> - sinfo->index);
>>> - continue;
>>> - }
>>> -
>>> if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
>>> pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
>>> sinfo->index);
>>> @@ -164,8 +153,10 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>>> sinfo->signer = x509;
>>> return 0;
>>> }
>>> +
>>> pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
>>> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_serial);
>>> + sinfo->index,
>>> + sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
>>> return -ENOKEY;
>>> }
>>>
>>> @@ -184,7 +175,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>>> p->seen = false;
>>>
>>> for (;;) {
>>> - pr_debug("verify %s: %s\n", x509->subject, x509->fingerprint);
>>> + pr_debug("verify %s: %*phN\n",
>>> + x509->subject,
>>> + x509->raw_serial_size, x509->raw_serial);
>>> x509->seen = true;
>>> ret = x509_get_sig_params(x509);
>>> if (ret < 0)
>>> @@ -192,7 +185,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>>>
>>> pr_debug("- issuer %s\n", x509->issuer);
>>> if (x509->authority)
>>> - pr_debug("- authkeyid %s\n", x509->authority);
>>> + pr_debug("- authkeyid %*phN\n",
>>> + x509->authority->len, x509->authority->data);
>>>
>>> if (!x509->authority ||
>>> strcmp(x509->subject, x509->issuer) == 0) {
>>> @@ -218,13 +212,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>>> /* Look through the X.509 certificates in the PKCS#7 message's
>>> * list to see if the next one is there.
>>> */
>>> - pr_debug("- want %s\n", x509->authority);
>>> + pr_debug("- want %*phN\n",
>>> + x509->authority->len, x509->authority->data);
>>> for (p = pkcs7->certs; p; p = p->next) {
>>> - pr_debug("- cmp [%u] %s\n", p->index, p->fingerprint);
>>> - if (p->raw_subject_size == x509->raw_issuer_size &&
>>> - strcmp(p->fingerprint, x509->authority) == 0 &&
>>> - memcmp(p->raw_subject, x509->raw_issuer,
>>> - x509->raw_issuer_size) == 0)
>>> + if (!p->skid)
>>> + continue;
>>> + pr_debug("- cmp [%u] %*phN\n",
>>> + p->index, p->skid->len, p->skid->data);
>>> + if (asymmetric_key_id_same(p->skid, x509->authority))
>>> goto found_issuer;
>>> }
>>>
>>> @@ -233,7 +228,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>>> return 0;
>>>
>>> found_issuer:
>>> - pr_debug("- issuer %s\n", p->subject);
>>> + pr_debug("- subject %s\n", p->subject);
>>> if (p->seen) {
>>> pr_warn("Sig %u: X.509 chain contains loop\n",
>>> sinfo->index);
>>> @@ -304,7 +299,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
>>> ret = x509_get_sig_params(x509);
>>> if (ret < 0)
>>> return ret;
>>> - pr_debug("X.509[%u] %s\n", n, x509->authority);
>>> + pr_debug("X.509[%u] %*phN\n",
>>> + n, x509->authority->len, x509->authority->data);
>>> }
>>>
>>> for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
>>> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
>>> index ac72348c186a..96151b2b91a2 100644
>>> --- a/crypto/asymmetric_keys/x509_cert_parser.c
>>> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
>>> @@ -46,7 +46,8 @@ void x509_free_certificate(struct x509_certificate *cert)
>>> public_key_destroy(cert->pub);
>>> kfree(cert->issuer);
>>> kfree(cert->subject);
>>> - kfree(cert->fingerprint);
>>> + kfree(cert->id);
>>> + kfree(cert->skid);
>>> kfree(cert->authority);
>>> kfree(cert->sig.digest);
>>> mpi_free(cert->sig.rsa.s);
>>> @@ -62,6 +63,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>>> {
>>> struct x509_certificate *cert;
>>> struct x509_parse_context *ctx;
>>> + struct asymmetric_key_id *kid;
>>> long ret;
>>>
>>> ret = -ENOMEM;
>>> @@ -89,6 +91,17 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>>> if (ret < 0)
>>> goto error_decode;
>>>
>>> + /* Generate cert issuer + serial number key ID */
>>> + kid = asymmetric_key_generate_id(cert->raw_serial,
>>> + cert->raw_serial_size,
>>> + cert->raw_issuer,
>>> + cert->raw_issuer_size);
>>> + if (IS_ERR(kid)) {
>>> + ret = PTR_ERR(kid);
>>> + goto error_decode;
>>> + }
>>> + cert->id = kid;
>>> +
>>> kfree(ctx);
>>> return cert;
>>>
>>> @@ -407,36 +420,34 @@ int x509_process_extension(void *context, size_t hdrlen,
>>> const void *value, size_t vlen)
>>> {
>>> struct x509_parse_context *ctx = context;
>>> + struct asymmetric_key_id *kid;
>>> const unsigned char *v = value;
>>> - char *f;
>>> int i;
>>>
>>> pr_debug("Extension: %u\n", ctx->last_oid);
>>>
>>> if (ctx->last_oid == OID_subjectKeyIdentifier) {
>>> /* Get hold of the key fingerprint */
>>> - if (vlen < 3)
>>> + if (ctx->cert->skid || vlen < 3)
>>> return -EBADMSG;
>>> if (v[0] != ASN1_OTS || v[1] != vlen - 2)
>>> return -EBADMSG;
>>> v += 2;
>>> vlen -= 2;
>>>
>>> - f = kmalloc(vlen * 2 + 1, GFP_KERNEL);
>>> - if (!f)
>>> - return -ENOMEM;
>>> - for (i = 0; i < vlen; i++)
>>> - sprintf(f + i * 2, "%02x", v[i]);
>>> - pr_debug("fingerprint %s\n", f);
>>> - ctx->cert->fingerprint = f;
>>> + kid = asymmetric_key_generate_id(v, vlen,
>>> + ctx->cert->raw_subject,
>>> + ctx->cert->raw_subject_size);
>>> + if (IS_ERR(kid))
>>> + return PTR_ERR(kid);
>>> + ctx->cert->skid = kid;
>>> + pr_debug("subjkeyid %*phN\n", kid->len, kid->data);
>>> return 0;
>>> }
>>>
>>> if (ctx->last_oid == OID_authorityKeyIdentifier) {
>>> - size_t key_len;
>>> -
>>> /* Get hold of the CA key fingerprint */
>>> - if (vlen < 5)
>>> + if (ctx->cert->authority || vlen < 5)
>>> return -EBADMSG;
>>>
>>> /* Authority Key Identifier must be a Constructed SEQUENCE */
>>> @@ -454,7 +465,7 @@ int x509_process_extension(void *context, size_t hdrlen,
>>> v[3] > vlen - 4)
>>> return -EBADMSG;
>>>
>>> - key_len = v[3];
>>> + vlen = v[3];
>>> v += 4;
>>> } else {
>>> /* Long Form length */
>>> @@ -476,17 +487,17 @@ int x509_process_extension(void *context, size_t hdrlen,
>>> v[sub + 1] > vlen - 4 - sub)
>>> return -EBADMSG;
>>>
>>> - key_len = v[sub + 1];
>>> + vlen = v[sub + 1];
>>> v += (sub + 2);
>>> }
>>>
>>> - f = kmalloc(key_len * 2 + 1, GFP_KERNEL);
>>> - if (!f)
>>> - return -ENOMEM;
>>> - for (i = 0; i < key_len; i++)
>>> - sprintf(f + i * 2, "%02x", v[i]);
>>> - pr_debug("authority %s\n", f);
>>> - ctx->cert->authority = f;
>>> + kid = asymmetric_key_generate_id(v, vlen,
>>> + ctx->cert->raw_issuer,
>>> + ctx->cert->raw_issuer_size);
>>> + if (IS_ERR(kid))
>>> + return PTR_ERR(kid);
>>> + pr_debug("authkeyid %*phN\n", kid->len, kid->data);
>>> + ctx->cert->authority = kid;
>>> return 0;
>>> }
>>>
>>> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
>>> index 1b76f207c1f3..0e8d59b010fb 100644
>>> --- a/crypto/asymmetric_keys/x509_parser.h
>>> +++ b/crypto/asymmetric_keys/x509_parser.h
>>> @@ -19,8 +19,9 @@ struct x509_certificate {
>>> struct public_key_signature sig; /* Signature parameters */
>>> char *issuer; /* Name of certificate issuer */
>>> char *subject; /* Name of certificate subject */
>>> - char *fingerprint; /* Key fingerprint as hex */
>>> - char *authority; /* Authority key fingerprint as hex */
>>> + struct asymmetric_key_id *id; /* Issuer + serial number */
>>> + struct asymmetric_key_id *skid; /* Subject key identifier */
>>> + struct asymmetric_key_id *authority; /* Authority key identifier */
>>> struct tm valid_from;
>>> struct tm valid_to;
>>> const void *tbs; /* Signed data */
>>> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
>>> index f3d62307e6ee..c60905c3f4d2 100644
>>> --- a/crypto/asymmetric_keys/x509_public_key.c
>>> +++ b/crypto/asymmetric_keys/x509_public_key.c
>>> @@ -25,7 +25,7 @@
>>> #include "x509_parser.h"
>>>
>>> static bool use_builtin_keys;
>>> -static char *ca_keyid;
>>> +static struct asymmetric_key_id *ca_keyid;
>>>
>>> #ifndef MODULE
>>> static int __init ca_keys_setup(char *str)
>>> @@ -33,10 +33,16 @@ static int __init ca_keys_setup(char *str)
>>> if (!str) /* default system keyring */
>>> return 1;
>>>
>>> - if (strncmp(str, "id:", 3) == 0)
>>> - ca_keyid = str; /* owner key 'id:xxxxxx' */
>>> - else if (strcmp(str, "builtin") == 0)
>>> + if (strncmp(str, "id:", 3) == 0) {
>>> + struct asymmetric_key_id *p;
>>> + p = asymmetric_key_hex_to_key_id(str);
>>> + if (p == ERR_PTR(-EINVAL))
>>> + pr_err("Unparsable hex string in ca_keys\n");
>>> + else if (!IS_ERR(p))
>>> + ca_keyid = p; /* owner key 'id:xxxxxx' */
>>> + } else if (strcmp(str, "builtin") == 0) {
>>> use_builtin_keys = true;
>>> + }
>>>
>>> return 1;
>>> }
>>> @@ -46,31 +52,28 @@ __setup("ca_keys=", ca_keys_setup);
>>> /**
>>> * x509_request_asymmetric_key - Request a key by X.509 certificate params.
>>> * @keyring: The keys to search.
>>> - * @subject: The name of the subject to whom the key belongs.
>>> - * @key_id: The subject key ID as a hex string.
>>> + * @kid: The key ID.
>>> *
>>> * Find a key in the given keyring by subject name and key ID. These might,
>>> * for instance, be the issuer name and the authority key ID of an X.509
>>> * certificate that needs to be verified.
>>> */
>>> struct key *x509_request_asymmetric_key(struct key *keyring,
>>> - const char *subject,
>>> - const char *key_id)
>>> + const struct asymmetric_key_id *kid)
>>> {
>>> key_ref_t key;
>>> - size_t subject_len = strlen(subject), key_id_len = strlen(key_id);
>>> - char *id;
>>> + char *id, *p;
>>>
>>> - /* Construct an identifier "<subjname>:<keyid>". */
>>> - id = kmalloc(subject_len + 2 + key_id_len + 1, GFP_KERNEL);
>>> + /* Construct an identifier "id:<keyid>". */
>>> + p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
>>> if (!id)
>>> return ERR_PTR(-ENOMEM);
>>>
>>> - memcpy(id, subject, subject_len);
>>> - id[subject_len + 0] = ':';
>>> - id[subject_len + 1] = ' ';
>>> - memcpy(id + subject_len + 2, key_id, key_id_len);
>>> - id[subject_len + 2 + key_id_len] = 0;
>>> + *p++ = 'i';
>>> + *p++ = 'd';
>>> + *p++ = ':';
>>> + p = bin2hex(p, kid->data, kid->len);
>>> + *p = 0;
>>>
>>> pr_debug("Look up: \"%s\"\n", id);
>>>
>>> @@ -195,11 +198,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
>>> if (!trust_keyring)
>>> return -EOPNOTSUPP;
>>>
>>> - if (ca_keyid && !asymmetric_keyid_match(cert->authority, ca_keyid))
>>> + if (ca_keyid && !asymmetric_key_id_same(cert->authority, ca_keyid))
>>> return -EPERM;
>>>
>>> - key = x509_request_asymmetric_key(trust_keyring,
>>> - cert->issuer, cert->authority);
>>> + key = x509_request_asymmetric_key(trust_keyring, cert->authority);
>>> if (!IS_ERR(key)) {
>>> if (!use_builtin_keys
>>> || test_bit(KEY_FLAG_BUILTIN, &key->flags))
>>> @@ -214,9 +216,11 @@ static int x509_validate_trust(struct x509_certificate *cert,
>>> */
>>> static int x509_key_preparse(struct key_preparsed_payload *prep)
>>> {
>>> + struct asymmetric_key_ids *kids;
>>> struct x509_certificate *cert;
>>> + const char *q;
>>> size_t srlen, sulen;
>>> - char *desc = NULL;
>>> + char *desc = NULL, *p;
>>> int ret;
>>>
>>> cert = x509_cert_parse(prep->data, prep->datalen);
>>> @@ -249,19 +253,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>>> pkey_algo_name[cert->sig.pkey_algo],
>>> hash_algo_name[cert->sig.pkey_hash_algo]);
>>>
>>> - if (!cert->fingerprint) {
>>> - pr_warn("Cert for '%s' must have a SubjKeyId extension\n",
>>> - cert->subject);
>>> - ret = -EKEYREJECTED;
>>> - goto error_free_cert;
>>> - }
>>> -
>>> cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
>>> cert->pub->id_type = PKEY_ID_X509;
>>>
>>> /* Check the signature on the key if it appears to be self-signed */
>>> if (!cert->authority ||
>>> - strcmp(cert->fingerprint, cert->authority) == 0) {
>>> + asymmetric_key_id_same(cert->skid, cert->authority)) {
>>> ret = x509_check_signature(cert->pub, cert); /* self-signed */
>>> if (ret < 0)
>>> goto error_free_cert;
>>> @@ -273,31 +270,47 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>>>
>>> /* Propose a description */
>>> sulen = strlen(cert->subject);
>>> - srlen = strlen(cert->fingerprint);
>>> + srlen = cert->raw_serial_size;
>>> + q = cert->raw_serial;
>>> + if (srlen > 1 && *q == 0) {
>>> + srlen--;
>>> + q++;
>>> + }
>>> +
>>> ret = -ENOMEM;
>>> - desc = kmalloc(sulen + 2 + srlen + 1, GFP_KERNEL);
>>> + desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
>>> if (!desc)
>>> goto error_free_cert;
>>> - memcpy(desc, cert->subject, sulen);
>>> - desc[sulen] = ':';
>>> - desc[sulen + 1] = ' ';
>>> - memcpy(desc + sulen + 2, cert->fingerprint, srlen);
>>> - desc[sulen + 2 + srlen] = 0;
>>> + p = memcpy(desc, cert->subject, sulen);
>>> + p += sulen;
>>> + *p++ = ':';
>>> + *p++ = ' ';
>>> + p = bin2hex(p, q, srlen);
>>> + *p = 0;
>>> +
>>> + kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
>>> + if (!kids)
>>> + goto error_free_desc;
>>> + kids->id[0] = cert->id;
>>> + kids->id[1] = cert->skid;
>>>
>>> /* We're pinning the module by being linked against it */
>>> __module_get(public_key_subtype.owner);
>>> prep->type_data[0] = &public_key_subtype;
>>> - prep->type_data[1] = cert->fingerprint;
>>> + prep->type_data[1] = kids;
>>> prep->payload[0] = cert->pub;
>>> prep->description = desc;
>>> prep->quotalen = 100;
>>>
>>> /* We've finished with the certificate */
>>> cert->pub = NULL;
>>> - cert->fingerprint = NULL;
>>> + cert->id = NULL;
>>> + cert->skid = NULL;
>>> desc = NULL;
>>> ret = 0;
>>>
>>> +error_free_desc:
>>> + kfree(desc);
>>> error_free_cert:
>>> x509_free_certificate(cert);
>>> return ret;
>>> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
>>> index 0d164c6af539..fa73a6fd536c 100644
>>> --- a/include/crypto/public_key.h
>>> +++ b/include/crypto/public_key.h
>>> @@ -15,6 +15,7 @@
>>> #define _LINUX_PUBLIC_KEY_H
>>>
>>> #include <linux/mpi.h>
>>> +#include <keys/asymmetric-type.h>
>>> #include <crypto/hash_info.h>
>>>
>>> enum pkey_algo {
>>> @@ -98,8 +99,8 @@ struct key;
>>> extern int verify_signature(const struct key *key,
>>> const struct public_key_signature *sig);
>>>
>>> +struct asymmetric_key_id;
>>> extern struct key *x509_request_asymmetric_key(struct key *keyring,
>>> - const char *issuer,
>>> - const char *key_id);
>>> + const struct asymmetric_key_id *kid);
>>>
>>> #endif /* _LINUX_PUBLIC_KEY_H */
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Thanks,
Dmitry

2014-10-02 18:39:04

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

On Thu, 2014-10-02 at 19:04 +0300, Dmitry Kasatkin wrote:
> On 02/10/14 18:49, Dmitry Kasatkin wrote:
> > Hi David,
> >
> > I just took latest #next branch from James's security tree which
> > includes latest KEYs patches and noticed following:
> >
> > [ 9.812332] Request for unknown module key 'Magrathea: Glacier
> > signing key: 926305d6dda66f47139eb4e3cb25a6adef527f08' err -11
> >
> > Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
> > also has changed in respect of certificate ids..
> >
> > Those ids does not look any close to my kernel X509 X509v3 Subject Key
> > Identifier, which is:
> > 92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08
> >
> > proc/keys shows
> >
> > symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
> > 6865612e68326732 []
> >
> > Very different ids..
> >
> > How could I match certificate now?
> > Module verification code cannot find needed key..
> >
> > - Dmitry
>
>
> Hehe. Also now I get kernel Oops in asymmetric_key_id_same...

Confirmed

Mimi

> -------------------------
> [ 132.816522] BUG: unable to handle kernel paging request at
> ffffffffffffffea
> [ 132.819902] IP: [<ffffffff812bfc20>] asymmetric_key_id_same+0x14/0x36
> [ 132.820302] PGD 1a12067 PUD 1a14067 PMD 0
> [ 132.820302] Oops: 0000 [#1] SMP
> [ 132.820302] Modules linked in: bridge(E) stp(E) llc(E) evdev(E)
> serio_raw(E) i2c_piix4(E) button(E) fuse(E)
> [ 132.820302] CPU: 0 PID: 2993 Comm: cat Tainted: G E
> 3.16.0-kds+ #2847
> [ 132.820302] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 132.820302] task: ffff88004249a430 ti: ffff880056640000 task.ti:
> ffff880056640000
> [ 132.820302] RIP: 0010:[<ffffffff812bfc20>] [<ffffffff812bfc20>]
> asymmetric_key_id_same+0x14/0x36
> [ 132.820302] RSP: 0018:ffff880056643930 EFLAGS: 00010246
> [ 132.820302] RAX: 0000000000000000 RBX: ffffffffffffffea RCX:
> ffff880056643ae0
> [ 132.820302] RDX: 000000000000005e RSI: ffffffffffffffea RDI:
> ffff88005bac9300
> [ 132.820302] RBP: ffff880056643948 R08: 0000000000000003 R09:
> 00000007504aa01a
> [ 132.820302] R10: 0000000000000000 R11: 0000000000000000 R12:
> ffff88005d68ca40
> [ 132.820302] R13: 0000000000000101 R14: 0000000000000000 R15:
> ffff88005bac5280
> [ 132.820302] FS: 00007f67a153c740(0000) GS:ffff88005da00000(0000)
> knlGS:0000000000000000
> [ 132.820302] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 132.820302] CR2: ffffffffffffffea CR3: 000000002e663000 CR4:
> 00000000000006f0
> [ 132.820302] Stack:
> [ 132.820302] ffffffff812bfc66 ffff880056643ae0 ffff88005bac5280
> ffff880056643958
> [ 132.820302] ffffffff812bfc9d ffff880056643980 ffffffff812971d9
> ffff88005ce930c1
> [ 132.820302] ffff88005ce930c0 0000000000000000 ffff8800566439c8
> ffffffff812fb753
> [ 132.820302] Call Trace:
> [ 132.820302] [<ffffffff812bfc66>] ? asymmetric_match_key_ids+0x24/0x42
> [ 132.820302] [<ffffffff812bfc9d>] asymmetric_key_cmp+0x19/0x1b
> [ 132.820302] [<ffffffff812971d9>] keyring_search_iterator+0x74/0xd7
> [ 132.820302] [<ffffffff812fb753>] assoc_array_subtree_iterate+0x67/0xd2
> [ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
> [ 132.820302] [<ffffffff812fbaa1>] assoc_array_iterate+0x19/0x1e
> [ 132.820302] [<ffffffff81297332>] search_nested_keyrings+0xf6/0x2b6
> [ 132.820302] [<ffffffff810728da>] ? sched_clock_cpu+0x91/0xa2
> [ 132.820302] [<ffffffff810860d2>] ? mark_held_locks+0x58/0x6e
> [ 132.820302] [<ffffffff810a137d>] ? current_kernel_time+0x77/0xb8
> [ 132.820302] [<ffffffff81297871>] keyring_search_aux+0xe1/0x14c
> [ 132.820302] [<ffffffff812977fc>] ? keyring_search_aux+0x6c/0x14c
> [ 132.820302] [<ffffffff8129796b>] keyring_search+0x8f/0xb6
> [ 132.820302] [<ffffffff812bfc84>] ? asymmetric_match_key_ids+0x42/0x42
> [ 132.820302] [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
> [ 132.820302] [<ffffffff812ab9e3>] asymmetric_verify+0xa4/0x214
> [ 132.820302] [<ffffffff812ab90e>] integrity_digsig_verify+0xb1/0xe2
> [ 132.820302] [<ffffffff812abe41>] ? evm_verifyxattr+0x6a/0x7a
> [ 132.820302] [<ffffffff812b0390>] ima_appraise_measurement+0x160/0x370
> [ 132.820302] [<ffffffff81161db2>] ? d_absolute_path+0x5b/0x7a
> [ 132.820302] [<ffffffff812ada30>] process_measurement+0x322/0x404
>
>
> > On 08/09/14 18:38, David Howells wrote:
> >> Make use of the new match string preparsing to overhaul key identification
> >> when searching for asymmetric keys. The following changes are made:
> >>
> >> (1) Use the previously created asymmetric_key_id struct to hold the following
> >> key IDs derived from the X.509 certificate or PKCS#7 message:
> >>
> >> id: serial number + issuer
> >> skid: subjKeyId + subject
> >> authority: authKeyId + issuer
> >>
> >> (2) Replace the hex fingerprint attached to key->type_data[1] with an
> >> asymmetric_key_ids struct containing the id and the skid (if present).
> >>
> >> (3) Make the asymmetric_type match data preparse select one of two searches:
> >>
> >> (a) An iterative search for the key ID given if prefixed with "id:". The
> >> prefix is expected to be followed by a hex string giving the ID to
> >> search for. The criterion key ID is checked against all key IDs
> >> recorded on the key.
> >>
> >> (b) A direct search if the key ID is not prefixed with "id:". This will
> >> look for an exact match on the key description.
> >>
> >> (4) Make x509_request_asymmetric_key() take a key ID. This is then converted
> >> into "id:<hex>" and passed into keyring_search() where match preparsing
> >> will turn it back into a binary ID.
> >>
> >> (5) X.509 certificate verification then takes the authority key ID and looks
> >> up a key that matches it to find the public key for the certificate
> >> signature.
> >>
> >> (6) PKCS#7 certificate verification then takes the id key ID and looks up a
> >> key that matches it to find the public key for the signed information
> >> block signature.
> >>
> >> Additional changes:
> >>
> >> (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
> >> cert to be rejected with -EBADMSG.
> >>
> >> (2) The 'fingerprint' ID is gone. This was primarily intended to convey PGP
> >> public key fingerprints. If PGP is supported in future, this should
> >> generate a key ID that carries the fingerprint.
> >>
> >> (3) Th ca_keyid= kernel command line option is now converted to a key ID and
> >> used to match the authority key ID. Possibly this should only match the
> >> actual authKeyId part and not the issuer as well.
> >>
> >> Signed-off-by: David Howells <[email protected]>
> >> ---
> >>
> >> crypto/asymmetric_keys/asymmetric_keys.h | 4 -
> >> crypto/asymmetric_keys/asymmetric_type.c | 133 ++++++++++++-----------------
> >> crypto/asymmetric_keys/pkcs7_parser.c | 38 ++++++--
> >> crypto/asymmetric_keys/pkcs7_parser.h | 5 -
> >> crypto/asymmetric_keys/pkcs7_trust.c | 6 -
> >> crypto/asymmetric_keys/pkcs7_verify.c | 44 ++++------
> >> crypto/asymmetric_keys/x509_cert_parser.c | 55 +++++++-----
> >> crypto/asymmetric_keys/x509_parser.h | 5 +
> >> crypto/asymmetric_keys/x509_public_key.c | 89 +++++++++++--------
> >> include/crypto/public_key.h | 5 +
> >> 10 files changed, 198 insertions(+), 186 deletions(-)
> >>
> >> diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
> >> index 917be6b985e7..fd21ac28e0a0 100644
> >> --- a/crypto/asymmetric_keys/asymmetric_keys.h
> >> +++ b/crypto/asymmetric_keys/asymmetric_keys.h
> >> @@ -9,13 +9,13 @@
> >> * 2 of the Licence, or (at your option) any later version.
> >> */
> >>
> >> -int asymmetric_keyid_match(const char *kid, const char *id);
> >> extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
> >> const struct asymmetric_key_id *match_id);
> >>
> >> extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
> >>
> >> -static inline const char *asymmetric_key_id(const struct key *key)
> >> +static inline
> >> +const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
> >> {
> >> return key->type_data.p[1];
> >> }
> >> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> >> index 3bc71b4e1eed..6f16f647d21b 100644
> >> --- a/crypto/asymmetric_keys/asymmetric_type.c
> >> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> >> @@ -105,76 +105,15 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
> >> }
> >>
> >> /*
> >> - * Match asymmetric key id with partial match
> >> - * @id: key id to match in a form "id:<id>"
> >> - */
> >> -int asymmetric_keyid_match(const char *kid, const char *id)
> >> -{
> >> - size_t idlen, kidlen;
> >> -
> >> - if (!kid || !id)
> >> - return 0;
> >> -
> >> - /* make it possible to use id as in the request: "id:<id>" */
> >> - if (strncmp(id, "id:", 3) == 0)
> >> - id += 3;
> >> -
> >> - /* Anything after here requires a partial match on the ID string */
> >> - idlen = strlen(id);
> >> - kidlen = strlen(kid);
> >> - if (idlen > kidlen)
> >> - return 0;
> >> -
> >> - kid += kidlen - idlen;
> >> - if (strcasecmp(id, kid) != 0)
> >> - return 0;
> >> -
> >> - return 1;
> >> -}
> >> -EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
> >> -
> >> -/*
> >> - * Match asymmetric keys on (part of) their name
> >> - * We have some shorthand methods for matching keys. We allow:
> >> - *
> >> - * "<desc>" - request a key by description
> >> - * "id:<id>" - request a key matching the ID
> >> - * "<subtype>:<id>" - request a key of a subtype
> >> + * Match asymmetric keys by ID.
> >> */
> >> static bool asymmetric_key_cmp(const struct key *key,
> >> const struct key_match_data *match_data)
> >> {
> >> - const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> >> - const char *description = match_data->raw_data;
> >> - const char *spec = description;
> >> - const char *id;
> >> - ptrdiff_t speclen;
> >> -
> >> - if (!subtype || !spec || !*spec)
> >> - return 0;
> >> -
> >> - /* See if the full key description matches as is */
> >> - if (key->description && strcmp(key->description, description) == 0)
> >> - return 1;
> >> -
> >> - /* All tests from here on break the criterion description into a
> >> - * specifier, a colon and then an identifier.
> >> - */
> >> - id = strchr(spec, ':');
> >> - if (!id)
> >> - return 0;
> >> -
> >> - speclen = id - spec;
> >> - id++;
> >> -
> >> - if (speclen == 2 && memcmp(spec, "id", 2) == 0)
> >> - return asymmetric_keyid_match(asymmetric_key_id(key), id);
> >> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> >> + const struct asymmetric_key_id *match_id = match_data->preparsed;
> >>
> >> - if (speclen == subtype->name_len &&
> >> - memcmp(spec, subtype->name, speclen) == 0)
> >> - return 1;
> >> -
> >> - return 0;
> >> + return asymmetric_match_key_ids(kids, match_id);
> >> }
> >>
> >> /*
> >> @@ -191,8 +130,30 @@ static bool asymmetric_key_cmp(const struct key *key,
> >> */
> >> static int asymmetric_key_match_preparse(struct key_match_data *match_data)
> >> {
> >> - match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
> >> + struct asymmetric_key_id *match_id;
> >> + const char *spec = match_data->raw_data;
> >> + const char *id;
> >> +
> >> + if (!spec || !*spec)
> >> + return -EINVAL;
> >> + if (spec[0] == 'i' &&
> >> + spec[1] == 'd' &&
> >> + spec[2] == ':') {
> >> + id = spec + 3;
> >> + } else {
> >> + goto default_match;
> >> + }
> >> +
> >> + match_id = asymmetric_key_hex_to_key_id(id);
> >> + if (!match_id)
> >> + return -ENOMEM;
> >> +
> >> + match_data->preparsed = match_id;
> >> match_data->cmp = asymmetric_key_cmp;
> >> + match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
> >> + return 0;
> >> +
> >> +default_match:
> >> return 0;
> >> }
> >>
> >> @@ -201,6 +162,7 @@ static int asymmetric_key_match_preparse(struct key_match_data *match_data)
> >> */
> >> static void asymmetric_key_match_free(struct key_match_data *match_data)
> >> {
> >> + kfree(match_data->preparsed);
> >> }
> >>
> >> /*
> >> @@ -209,8 +171,10 @@ static void asymmetric_key_match_free(struct key_match_data *match_data)
> >> static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
> >> {
> >> const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> >> - const char *kid = asymmetric_key_id(key);
> >> - size_t n;
> >> + const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
> >> + const struct asymmetric_key_id *kid;
> >> + const unsigned char *p;
> >> + int n;
> >>
> >> seq_puts(m, key->description);
> >>
> >> @@ -218,13 +182,16 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
> >> seq_puts(m, ": ");
> >> subtype->describe(key, m);
> >>
> >> - if (kid) {
> >> + if (kids && kids->id[0]) {
> >> + kid = kids->id[0];
> >> seq_putc(m, ' ');
> >> - n = strlen(kid);
> >> - if (n <= 8)
> >> - seq_puts(m, kid);
> >> - else
> >> - seq_puts(m, kid + n - 8);
> >> + n = kid->len;
> >> + p = kid->data;
> >> + if (n > 8) {
> >> + p += n - 8;
> >> + n = 8;
> >> + }
> >> + seq_printf(m, "%*phN", n, p);
> >> }
> >>
> >> seq_puts(m, " [");
> >> @@ -275,6 +242,7 @@ static int asymmetric_key_preparse(struct key_preparsed_payload *prep)
> >> static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> >> {
> >> struct asymmetric_key_subtype *subtype = prep->type_data[0];
> >> + struct asymmetric_key_ids *kids = prep->type_data[1];
> >>
> >> pr_devel("==>%s()\n", __func__);
> >>
> >> @@ -282,7 +250,11 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> >> subtype->destroy(prep->payload[0]);
> >> module_put(subtype->owner);
> >> }
> >> - kfree(prep->type_data[1]);
> >> + if (kids) {
> >> + kfree(kids->id[0]);
> >> + kfree(kids->id[1]);
> >> + kfree(kids);
> >> + }
> >> kfree(prep->description);
> >> }
> >>
> >> @@ -292,13 +264,20 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
> >> static void asymmetric_key_destroy(struct key *key)
> >> {
> >> struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
> >> + struct asymmetric_key_ids *kids = key->type_data.p[1];
> >> +
> >> if (subtype) {
> >> subtype->destroy(key->payload.data);
> >> module_put(subtype->owner);
> >> key->type_data.p[0] = NULL;
> >> }
> >> - kfree(key->type_data.p[1]);
> >> - key->type_data.p[1] = NULL;
> >> +
> >> + if (kids) {
> >> + kfree(kids->id[0]);
> >> + kfree(kids->id[1]);
> >> + kfree(kids);
> >> + key->type_data.p[1] = NULL;
> >> + }
> >> }
> >>
> >> struct key_type key_type_asymmetric = {
> >> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> >> index 459d2077c61b..ad6ae9d7c884 100644
> >> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> >> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> >> @@ -29,6 +29,10 @@ struct pkcs7_parse_context {
> >> enum OID last_oid; /* Last OID encountered */
> >> unsigned x509_index;
> >> unsigned sinfo_index;
> >> + const void *raw_serial;
> >> + unsigned raw_serial_size;
> >> + unsigned raw_issuer_size;
> >> + const void *raw_issuer;
> >> };
> >>
> >> /*
> >> @@ -39,6 +43,7 @@ static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
> >> if (sinfo) {
> >> mpi_free(sinfo->sig.mpi[0]);
> >> kfree(sinfo->sig.digest);
> >> + kfree(sinfo->signing_cert_id);
> >> kfree(sinfo);
> >> }
> >> }
> >> @@ -256,10 +261,10 @@ int pkcs7_extract_cert(void *context, size_t hdrlen,
> >> if (IS_ERR(x509))
> >> return PTR_ERR(x509);
> >>
> >> - pr_debug("Got cert for %s\n", x509->subject);
> >> - pr_debug("- fingerprint %s\n", x509->fingerprint);
> >> -
> >> x509->index = ++ctx->x509_index;
> >> + pr_debug("Got cert %u for %s\n", x509->index, x509->subject);
> >> + pr_debug("- fingerprint %*phN\n", x509->id->len, x509->id->data);
> >> +
> >> *ctx->ppcerts = x509;
> >> ctx->ppcerts = &x509->next;
> >> return 0;
> >> @@ -348,8 +353,8 @@ int pkcs7_sig_note_serial(void *context, size_t hdrlen,
> >> const void *value, size_t vlen)
> >> {
> >> struct pkcs7_parse_context *ctx = context;
> >> - ctx->sinfo->raw_serial = value;
> >> - ctx->sinfo->raw_serial_size = vlen;
> >> + ctx->raw_serial = value;
> >> + ctx->raw_serial_size = vlen;
> >> return 0;
> >> }
> >>
> >> @@ -361,8 +366,8 @@ int pkcs7_sig_note_issuer(void *context, size_t hdrlen,
> >> const void *value, size_t vlen)
> >> {
> >> struct pkcs7_parse_context *ctx = context;
> >> - ctx->sinfo->raw_issuer = value;
> >> - ctx->sinfo->raw_issuer_size = vlen;
> >> + ctx->raw_issuer = value;
> >> + ctx->raw_issuer_size = vlen;
> >> return 0;
> >> }
> >>
> >> @@ -395,10 +400,21 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
> >> const void *value, size_t vlen)
> >> {
> >> struct pkcs7_parse_context *ctx = context;
> >> -
> >> - ctx->sinfo->index = ++ctx->sinfo_index;
> >> - *ctx->ppsinfo = ctx->sinfo;
> >> - ctx->ppsinfo = &ctx->sinfo->next;
> >> + struct pkcs7_signed_info *sinfo = ctx->sinfo;
> >> + struct asymmetric_key_id *kid;
> >> +
> >> + /* Generate cert issuer + serial number key ID */
> >> + kid = asymmetric_key_generate_id(ctx->raw_serial,
> >> + ctx->raw_serial_size,
> >> + ctx->raw_issuer,
> >> + ctx->raw_issuer_size);
> >> + if (IS_ERR(kid))
> >> + return PTR_ERR(kid);
> >> +
> >> + sinfo->signing_cert_id = kid;
> >> + sinfo->index = ++ctx->sinfo_index;
> >> + *ctx->ppsinfo = sinfo;
> >> + ctx->ppsinfo = &sinfo->next;
> >> ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
> >> if (!ctx->sinfo)
> >> return -ENOMEM;
> >> diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
> >> index d25f4d15370f..91949f92bc72 100644
> >> --- a/crypto/asymmetric_keys/pkcs7_parser.h
> >> +++ b/crypto/asymmetric_keys/pkcs7_parser.h
> >> @@ -33,10 +33,7 @@ struct pkcs7_signed_info {
> >> const void *authattrs;
> >>
> >> /* Issuing cert serial number and issuer's name */
> >> - const void *raw_serial;
> >> - unsigned raw_serial_size;
> >> - unsigned raw_issuer_size;
> >> - const void *raw_issuer;
> >> + struct asymmetric_key_id *signing_cert_id;
> >>
> >> /* Message signature.
> >> *
> >> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> >> index e666eb011a85..4e8dd7214753 100644
> >> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> >> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> >> @@ -49,8 +49,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> >> /* Look to see if this certificate is present in the trusted
> >> * keys.
> >> */
> >> - key = x509_request_asymmetric_key(trust_keyring, x509->subject,
> >> - x509->fingerprint);
> >> + key = x509_request_asymmetric_key(trust_keyring, x509->id);
> >> if (!IS_ERR(key))
> >> /* One of the X.509 certificates in the PKCS#7 message
> >> * is apparently the same as one we already trust.
> >> @@ -82,8 +81,7 @@ int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> >> return -ENOKEY;
> >> }
> >>
> >> - key = x509_request_asymmetric_key(trust_keyring, last->issuer,
> >> - last->authority);
> >> + key = x509_request_asymmetric_key(trust_keyring, last->authority);
> >> if (IS_ERR(key))
> >> return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
> >> x509 = last;
> >> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> >> index c62cf8006e1f..57e90fa17f2b 100644
> >> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> >> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> >> @@ -131,8 +131,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> >> struct x509_certificate *x509;
> >> unsigned certix = 1;
> >>
> >> - kenter("%u,%u,%u",
> >> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_issuer_size);
> >> + kenter("%u", sinfo->index);
> >>
> >> for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
> >> /* I'm _assuming_ that the generator of the PKCS#7 message will
> >> @@ -140,21 +139,11 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> >> * PKCS#7 message - but I can't be 100% sure of that. It's
> >> * possible this will need element-by-element comparison.
> >> */
> >> - if (x509->raw_serial_size != sinfo->raw_serial_size ||
> >> - memcmp(x509->raw_serial, sinfo->raw_serial,
> >> - sinfo->raw_serial_size) != 0)
> >> + if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
> >> continue;
> >> pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
> >> sinfo->index, certix);
> >>
> >> - if (x509->raw_issuer_size != sinfo->raw_issuer_size ||
> >> - memcmp(x509->raw_issuer, sinfo->raw_issuer,
> >> - sinfo->raw_issuer_size) != 0) {
> >> - pr_warn("Sig %u: X.509 subject and PKCS#7 issuer don't match\n",
> >> - sinfo->index);
> >> - continue;
> >> - }
> >> -
> >> if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
> >> pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
> >> sinfo->index);
> >> @@ -164,8 +153,10 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> >> sinfo->signer = x509;
> >> return 0;
> >> }
> >> +
> >> pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
> >> - sinfo->index, sinfo->raw_serial_size, sinfo->raw_serial);
> >> + sinfo->index,
> >> + sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
> >> return -ENOKEY;
> >> }
> >>
> >> @@ -184,7 +175,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >> p->seen = false;
> >>
> >> for (;;) {
> >> - pr_debug("verify %s: %s\n", x509->subject, x509->fingerprint);
> >> + pr_debug("verify %s: %*phN\n",
> >> + x509->subject,
> >> + x509->raw_serial_size, x509->raw_serial);
> >> x509->seen = true;
> >> ret = x509_get_sig_params(x509);
> >> if (ret < 0)
> >> @@ -192,7 +185,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >>
> >> pr_debug("- issuer %s\n", x509->issuer);
> >> if (x509->authority)
> >> - pr_debug("- authkeyid %s\n", x509->authority);
> >> + pr_debug("- authkeyid %*phN\n",
> >> + x509->authority->len, x509->authority->data);
> >>
> >> if (!x509->authority ||
> >> strcmp(x509->subject, x509->issuer) == 0) {
> >> @@ -218,13 +212,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >> /* Look through the X.509 certificates in the PKCS#7 message's
> >> * list to see if the next one is there.
> >> */
> >> - pr_debug("- want %s\n", x509->authority);
> >> + pr_debug("- want %*phN\n",
> >> + x509->authority->len, x509->authority->data);
> >> for (p = pkcs7->certs; p; p = p->next) {
> >> - pr_debug("- cmp [%u] %s\n", p->index, p->fingerprint);
> >> - if (p->raw_subject_size == x509->raw_issuer_size &&
> >> - strcmp(p->fingerprint, x509->authority) == 0 &&
> >> - memcmp(p->raw_subject, x509->raw_issuer,
> >> - x509->raw_issuer_size) == 0)
> >> + if (!p->skid)
> >> + continue;
> >> + pr_debug("- cmp [%u] %*phN\n",
> >> + p->index, p->skid->len, p->skid->data);
> >> + if (asymmetric_key_id_same(p->skid, x509->authority))
> >> goto found_issuer;
> >> }
> >>
> >> @@ -233,7 +228,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> >> return 0;
> >>
> >> found_issuer:
> >> - pr_debug("- issuer %s\n", p->subject);
> >> + pr_debug("- subject %s\n", p->subject);
> >> if (p->seen) {
> >> pr_warn("Sig %u: X.509 chain contains loop\n",
> >> sinfo->index);
> >> @@ -304,7 +299,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
> >> ret = x509_get_sig_params(x509);
> >> if (ret < 0)
> >> return ret;
> >> - pr_debug("X.509[%u] %s\n", n, x509->authority);
> >> + pr_debug("X.509[%u] %*phN\n",
> >> + n, x509->authority->len, x509->authority->data);
> >> }
> >>
> >> for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> >> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> >> index ac72348c186a..96151b2b91a2 100644
> >> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> >> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> >> @@ -46,7 +46,8 @@ void x509_free_certificate(struct x509_certificate *cert)
> >> public_key_destroy(cert->pub);
> >> kfree(cert->issuer);
> >> kfree(cert->subject);
> >> - kfree(cert->fingerprint);
> >> + kfree(cert->id);
> >> + kfree(cert->skid);
> >> kfree(cert->authority);
> >> kfree(cert->sig.digest);
> >> mpi_free(cert->sig.rsa.s);
> >> @@ -62,6 +63,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> >> {
> >> struct x509_certificate *cert;
> >> struct x509_parse_context *ctx;
> >> + struct asymmetric_key_id *kid;
> >> long ret;
> >>
> >> ret = -ENOMEM;
> >> @@ -89,6 +91,17 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> >> if (ret < 0)
> >> goto error_decode;
> >>
> >> + /* Generate cert issuer + serial number key ID */
> >> + kid = asymmetric_key_generate_id(cert->raw_serial,
> >> + cert->raw_serial_size,
> >> + cert->raw_issuer,
> >> + cert->raw_issuer_size);
> >> + if (IS_ERR(kid)) {
> >> + ret = PTR_ERR(kid);
> >> + goto error_decode;
> >> + }
> >> + cert->id = kid;
> >> +
> >> kfree(ctx);
> >> return cert;
> >>
> >> @@ -407,36 +420,34 @@ int x509_process_extension(void *context, size_t hdrlen,
> >> const void *value, size_t vlen)
> >> {
> >> struct x509_parse_context *ctx = context;
> >> + struct asymmetric_key_id *kid;
> >> const unsigned char *v = value;
> >> - char *f;
> >> int i;
> >>
> >> pr_debug("Extension: %u\n", ctx->last_oid);
> >>
> >> if (ctx->last_oid == OID_subjectKeyIdentifier) {
> >> /* Get hold of the key fingerprint */
> >> - if (vlen < 3)
> >> + if (ctx->cert->skid || vlen < 3)
> >> return -EBADMSG;
> >> if (v[0] != ASN1_OTS || v[1] != vlen - 2)
> >> return -EBADMSG;
> >> v += 2;
> >> vlen -= 2;
> >>
> >> - f = kmalloc(vlen * 2 + 1, GFP_KERNEL);
> >> - if (!f)
> >> - return -ENOMEM;
> >> - for (i = 0; i < vlen; i++)
> >> - sprintf(f + i * 2, "%02x", v[i]);
> >> - pr_debug("fingerprint %s\n", f);
> >> - ctx->cert->fingerprint = f;
> >> + kid = asymmetric_key_generate_id(v, vlen,
> >> + ctx->cert->raw_subject,
> >> + ctx->cert->raw_subject_size);
> >> + if (IS_ERR(kid))
> >> + return PTR_ERR(kid);
> >> + ctx->cert->skid = kid;
> >> + pr_debug("subjkeyid %*phN\n", kid->len, kid->data);
> >> return 0;
> >> }
> >>
> >> if (ctx->last_oid == OID_authorityKeyIdentifier) {
> >> - size_t key_len;
> >> -
> >> /* Get hold of the CA key fingerprint */
> >> - if (vlen < 5)
> >> + if (ctx->cert->authority || vlen < 5)
> >> return -EBADMSG;
> >>
> >> /* Authority Key Identifier must be a Constructed SEQUENCE */
> >> @@ -454,7 +465,7 @@ int x509_process_extension(void *context, size_t hdrlen,
> >> v[3] > vlen - 4)
> >> return -EBADMSG;
> >>
> >> - key_len = v[3];
> >> + vlen = v[3];
> >> v += 4;
> >> } else {
> >> /* Long Form length */
> >> @@ -476,17 +487,17 @@ int x509_process_extension(void *context, size_t hdrlen,
> >> v[sub + 1] > vlen - 4 - sub)
> >> return -EBADMSG;
> >>
> >> - key_len = v[sub + 1];
> >> + vlen = v[sub + 1];
> >> v += (sub + 2);
> >> }
> >>
> >> - f = kmalloc(key_len * 2 + 1, GFP_KERNEL);
> >> - if (!f)
> >> - return -ENOMEM;
> >> - for (i = 0; i < key_len; i++)
> >> - sprintf(f + i * 2, "%02x", v[i]);
> >> - pr_debug("authority %s\n", f);
> >> - ctx->cert->authority = f;
> >> + kid = asymmetric_key_generate_id(v, vlen,
> >> + ctx->cert->raw_issuer,
> >> + ctx->cert->raw_issuer_size);
> >> + if (IS_ERR(kid))
> >> + return PTR_ERR(kid);
> >> + pr_debug("authkeyid %*phN\n", kid->len, kid->data);
> >> + ctx->cert->authority = kid;
> >> return 0;
> >> }
> >>
> >> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
> >> index 1b76f207c1f3..0e8d59b010fb 100644
> >> --- a/crypto/asymmetric_keys/x509_parser.h
> >> +++ b/crypto/asymmetric_keys/x509_parser.h
> >> @@ -19,8 +19,9 @@ struct x509_certificate {
> >> struct public_key_signature sig; /* Signature parameters */
> >> char *issuer; /* Name of certificate issuer */
> >> char *subject; /* Name of certificate subject */
> >> - char *fingerprint; /* Key fingerprint as hex */
> >> - char *authority; /* Authority key fingerprint as hex */
> >> + struct asymmetric_key_id *id; /* Issuer + serial number */
> >> + struct asymmetric_key_id *skid; /* Subject key identifier */
> >> + struct asymmetric_key_id *authority; /* Authority key identifier */
> >> struct tm valid_from;
> >> struct tm valid_to;
> >> const void *tbs; /* Signed data */
> >> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> >> index f3d62307e6ee..c60905c3f4d2 100644
> >> --- a/crypto/asymmetric_keys/x509_public_key.c
> >> +++ b/crypto/asymmetric_keys/x509_public_key.c
> >> @@ -25,7 +25,7 @@
> >> #include "x509_parser.h"
> >>
> >> static bool use_builtin_keys;
> >> -static char *ca_keyid;
> >> +static struct asymmetric_key_id *ca_keyid;
> >>
> >> #ifndef MODULE
> >> static int __init ca_keys_setup(char *str)
> >> @@ -33,10 +33,16 @@ static int __init ca_keys_setup(char *str)
> >> if (!str) /* default system keyring */
> >> return 1;
> >>
> >> - if (strncmp(str, "id:", 3) == 0)
> >> - ca_keyid = str; /* owner key 'id:xxxxxx' */
> >> - else if (strcmp(str, "builtin") == 0)
> >> + if (strncmp(str, "id:", 3) == 0) {
> >> + struct asymmetric_key_id *p;
> >> + p = asymmetric_key_hex_to_key_id(str);
> >> + if (p == ERR_PTR(-EINVAL))
> >> + pr_err("Unparsable hex string in ca_keys\n");
> >> + else if (!IS_ERR(p))
> >> + ca_keyid = p; /* owner key 'id:xxxxxx' */
> >> + } else if (strcmp(str, "builtin") == 0) {
> >> use_builtin_keys = true;
> >> + }
> >>
> >> return 1;
> >> }
> >> @@ -46,31 +52,28 @@ __setup("ca_keys=", ca_keys_setup);
> >> /**
> >> * x509_request_asymmetric_key - Request a key by X.509 certificate params.
> >> * @keyring: The keys to search.
> >> - * @subject: The name of the subject to whom the key belongs.
> >> - * @key_id: The subject key ID as a hex string.
> >> + * @kid: The key ID.
> >> *
> >> * Find a key in the given keyring by subject name and key ID. These might,
> >> * for instance, be the issuer name and the authority key ID of an X.509
> >> * certificate that needs to be verified.
> >> */
> >> struct key *x509_request_asymmetric_key(struct key *keyring,
> >> - const char *subject,
> >> - const char *key_id)
> >> + const struct asymmetric_key_id *kid)
> >> {
> >> key_ref_t key;
> >> - size_t subject_len = strlen(subject), key_id_len = strlen(key_id);
> >> - char *id;
> >> + char *id, *p;
> >>
> >> - /* Construct an identifier "<subjname>:<keyid>". */
> >> - id = kmalloc(subject_len + 2 + key_id_len + 1, GFP_KERNEL);
> >> + /* Construct an identifier "id:<keyid>". */
> >> + p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
> >> if (!id)
> >> return ERR_PTR(-ENOMEM);
> >>
> >> - memcpy(id, subject, subject_len);
> >> - id[subject_len + 0] = ':';
> >> - id[subject_len + 1] = ' ';
> >> - memcpy(id + subject_len + 2, key_id, key_id_len);
> >> - id[subject_len + 2 + key_id_len] = 0;
> >> + *p++ = 'i';
> >> + *p++ = 'd';
> >> + *p++ = ':';
> >> + p = bin2hex(p, kid->data, kid->len);
> >> + *p = 0;
> >>
> >> pr_debug("Look up: \"%s\"\n", id);
> >>
> >> @@ -195,11 +198,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
> >> if (!trust_keyring)
> >> return -EOPNOTSUPP;
> >>
> >> - if (ca_keyid && !asymmetric_keyid_match(cert->authority, ca_keyid))
> >> + if (ca_keyid && !asymmetric_key_id_same(cert->authority, ca_keyid))
> >> return -EPERM;
> >>
> >> - key = x509_request_asymmetric_key(trust_keyring,
> >> - cert->issuer, cert->authority);
> >> + key = x509_request_asymmetric_key(trust_keyring, cert->authority);
> >> if (!IS_ERR(key)) {
> >> if (!use_builtin_keys
> >> || test_bit(KEY_FLAG_BUILTIN, &key->flags))
> >> @@ -214,9 +216,11 @@ static int x509_validate_trust(struct x509_certificate *cert,
> >> */
> >> static int x509_key_preparse(struct key_preparsed_payload *prep)
> >> {
> >> + struct asymmetric_key_ids *kids;
> >> struct x509_certificate *cert;
> >> + const char *q;
> >> size_t srlen, sulen;
> >> - char *desc = NULL;
> >> + char *desc = NULL, *p;
> >> int ret;
> >>
> >> cert = x509_cert_parse(prep->data, prep->datalen);
> >> @@ -249,19 +253,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> >> pkey_algo_name[cert->sig.pkey_algo],
> >> hash_algo_name[cert->sig.pkey_hash_algo]);
> >>
> >> - if (!cert->fingerprint) {
> >> - pr_warn("Cert for '%s' must have a SubjKeyId extension\n",
> >> - cert->subject);
> >> - ret = -EKEYREJECTED;
> >> - goto error_free_cert;
> >> - }
> >> -
> >> cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
> >> cert->pub->id_type = PKEY_ID_X509;
> >>
> >> /* Check the signature on the key if it appears to be self-signed */
> >> if (!cert->authority ||
> >> - strcmp(cert->fingerprint, cert->authority) == 0) {
> >> + asymmetric_key_id_same(cert->skid, cert->authority)) {
> >> ret = x509_check_signature(cert->pub, cert); /* self-signed */
> >> if (ret < 0)
> >> goto error_free_cert;
> >> @@ -273,31 +270,47 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> >>
> >> /* Propose a description */
> >> sulen = strlen(cert->subject);
> >> - srlen = strlen(cert->fingerprint);
> >> + srlen = cert->raw_serial_size;
> >> + q = cert->raw_serial;
> >> + if (srlen > 1 && *q == 0) {
> >> + srlen--;
> >> + q++;
> >> + }
> >> +
> >> ret = -ENOMEM;
> >> - desc = kmalloc(sulen + 2 + srlen + 1, GFP_KERNEL);
> >> + desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
> >> if (!desc)
> >> goto error_free_cert;
> >> - memcpy(desc, cert->subject, sulen);
> >> - desc[sulen] = ':';
> >> - desc[sulen + 1] = ' ';
> >> - memcpy(desc + sulen + 2, cert->fingerprint, srlen);
> >> - desc[sulen + 2 + srlen] = 0;
> >> + p = memcpy(desc, cert->subject, sulen);
> >> + p += sulen;
> >> + *p++ = ':';
> >> + *p++ = ' ';
> >> + p = bin2hex(p, q, srlen);
> >> + *p = 0;
> >> +
> >> + kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
> >> + if (!kids)
> >> + goto error_free_desc;
> >> + kids->id[0] = cert->id;
> >> + kids->id[1] = cert->skid;
> >>
> >> /* We're pinning the module by being linked against it */
> >> __module_get(public_key_subtype.owner);
> >> prep->type_data[0] = &public_key_subtype;
> >> - prep->type_data[1] = cert->fingerprint;
> >> + prep->type_data[1] = kids;
> >> prep->payload[0] = cert->pub;
> >> prep->description = desc;
> >> prep->quotalen = 100;
> >>
> >> /* We've finished with the certificate */
> >> cert->pub = NULL;
> >> - cert->fingerprint = NULL;
> >> + cert->id = NULL;
> >> + cert->skid = NULL;
> >> desc = NULL;
> >> ret = 0;
> >>
> >> +error_free_desc:
> >> + kfree(desc);
> >> error_free_cert:
> >> x509_free_certificate(cert);
> >> return ret;
> >> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> >> index 0d164c6af539..fa73a6fd536c 100644
> >> --- a/include/crypto/public_key.h
> >> +++ b/include/crypto/public_key.h
> >> @@ -15,6 +15,7 @@
> >> #define _LINUX_PUBLIC_KEY_H
> >>
> >> #include <linux/mpi.h>
> >> +#include <keys/asymmetric-type.h>
> >> #include <crypto/hash_info.h>
> >>
> >> enum pkey_algo {
> >> @@ -98,8 +99,8 @@ struct key;
> >> extern int verify_signature(const struct key *key,
> >> const struct public_key_signature *sig);
> >>
> >> +struct asymmetric_key_id;
> >> extern struct key *x509_request_asymmetric_key(struct key *keyring,
> >> - const char *issuer,
> >> - const char *key_id);
> >> + const struct asymmetric_key_id *kid);
> >>
> >> #endif /* _LINUX_PUBLIC_KEY_H */
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-10-03 12:13:09

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

Dmitry Kasatkin <[email protected]> wrote:

> Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
> also has changed in respect of certificate ids..
>
> Those ids does not look any close to my kernel X509 X509v3 Subject Key
> Identifier, which is:
> 92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08
>
> proc/keys shows
>
> symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
> 6865612e68326732 []
>
> Very different ids..
>
> How could I match certificate now?

There are two IDs available:

id: serial number + issuer
skid: subjKeyId + subject

You can use either of them and their content is somewhat negotiable. Note
that they are both compound IDs at this point.

We have to move away from using subjKeyId for module signatures because we
have to be able to deal with keys that don't have one. Blech, but the PKCS
specs suck somewhat.

This is why I want to move to using detached-data PKCS#7 certs as the
signature. We have the PKCS#7 handling in the kernel now for doing kexec.

David

2014-10-03 12:13:33

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

Dmitry Kasatkin <[email protected]> wrote:

> [ 132.820302] CR2: ffffffffffffffea

Looks like I leaked an error number somewhere.

David

2014-10-03 12:21:12

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

Dmitry Kasatkin <[email protected]> wrote:

> What is this 6865612e68326732?

It's the tail of the first ID attached to the key (there may be more than
one). As this is derived from an X.509 cert, that will be the serial number
plus the ASN.1 encoded issuer name.

To quote from the description in the patch:

(1) Use the previously created asymmetric_key_id struct to hold the following
key IDs derived from the X.509 certificate or PKCS#7 message:

id: serial number + issuer
skid: subjKeyId + subject
authority: authKeyId + issuer

(2) Replace the hex fingerprint attached to key->type_data[1] with an
asymmetric_key_ids struct containing the id and the skid (if present).

If you turn the hex into chars, you will see "hea.h2g2"

I'm open to suggestions about the best way to represent the auxiliary IDs in
/proc/keys - but don't forget there can be more than one.

David

2014-10-03 12:23:08

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

Dmitry Kasatkin <[email protected]> wrote:

> Does it still support searching for the key by partially matching
> Subject Key Identifier?

No, not at this time. Adding partial matching support wouldn't be hard, and
given that the subjKeyId is the first part of the second key ID, that would
then work.

David

2014-10-03 12:23:44

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

On 03/10/14 15:12, David Howells wrote:
> Dmitry Kasatkin <[email protected]> wrote:
>
>> Also I noticed that output of 'keyctl show' and 'cat /proc/keys' output
>> also has changed in respect of certificate ids..
>>
>> Those ids does not look any close to my kernel X509 X509v3 Subject Key
>> Identifier, which is:
>> 92:63:05:D6:DD:A6:6F:47:13:9E:B4:E3:CB:25:A6:AD:EF:52:7F:08
>>
>> proc/keys shows
>>
>> symmetri Magrathea: Glacier signing key: d9e2e4c6951f1e83: X509.RSA
>> 6865612e68326732 []
>>
>> Very different ids..
>>
>> How could I match certificate now?
> There are two IDs available:
>
> id: serial number + issuer
> skid: subjKeyId + subject
>
> You can use either of them and their content is somewhat negotiable. Note
> that they are both compound IDs at this point.
>
> We have to move away from using subjKeyId for module signatures because we
> have to be able to deal with keys that don't have one. Blech, but the PKCS
> specs suck somewhat.
>
> This is why I want to move to using detached-data PKCS#7 certs as the
> signature. We have the PKCS#7 handling in the kernel now for doing kexec.

I looked to the code and understood...

See my patches please.

- Dmitry

> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-10-03 12:25:37

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 08/13] KEYS: Overhaul key identification when searching for asymmetric keys

On 03/10/14 15:13, David Howells wrote:
> Dmitry Kasatkin <[email protected]> wrote:
>
>> [ 132.820302] CR2: ffffffffffffffea
> Looks like I leaked an error number somewhere.
>
> David
>

Yes... I found..
Check patches I posted.

Also here
http://git.kernel.org/cgit/linux/kernel/git/kasatkin/linux-digsig.git/log/?h=keys-fixes

- Dmitry