Skip to content

Commit 6c2dc5a

Browse files
dhowellsdhowells
committed
X.509: Extract signature digest and make self-signed cert checks earlier
Extract the signature digest for an X.509 certificate earlier, at the end of x509_cert_parse() rather than leaving it to the callers thereof since it has to be called anyway. Further, immediately after that, check the signature on self-signed certificates, also rather in the callers of x509_cert_parse(). We note in the x509_certificate struct the following bits of information: (1) Whether the signature is self-signed (even if we can't check the signature due to missing crypto). (2) Whether the key held in the certificate needs unsupported crypto to be used. We may get a PKCS#7 message with X.509 certs that we can't make use of - we just ignore them and give ENOPKG at the end it we couldn't verify anything if at least one of these unusable certs are in the chain of trust. (3) Whether the signature held in the certificate needs unsupported crypto to be checked. We can still use the key held in this certificate, even if we can't check the signature on it - if it is held in the system trusted keyring, for instance. We just can't add it to a ring of trusted keys or follow it further up the chain of trust. Making these checks earlier allows x509_check_signature() to be removed and replaced with direct calls to public_key_verify_signature(). Signed-off-by: David Howells <dhowells@redhat.com>
1 parent 566a117 commit 6c2dc5a

File tree

4 files changed

+110
-71
lines changed

4 files changed

+110
-71
lines changed

‎crypto/asymmetric_keys/pkcs7_verify.c

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
190190
x509->subject,
191191
x509->raw_serial_size, x509->raw_serial);
192192
x509->seen = true;
193-
ret = x509_get_sig_params(x509);
194-
if (ret < 0)
195-
goto maybe_missing_crypto_in_x509;
193+
if (x509->unsupported_key)
194+
goto unsupported_crypto_in_x509;
196195

197196
pr_debug("- issuer %s\n", x509->issuer);
198197
sig = x509->sig;
@@ -203,22 +202,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
203202
pr_debug("- authkeyid.skid %*phN\n",
204203
sig->auth_ids[1]->len, sig->auth_ids[1]->data);
205204

206-
if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
207-
strcmp(x509->subject, x509->issuer) == 0) {
205+
if (x509->self_signed) {
208206
/* If there's no authority certificate specified, then
209207
* the certificate must be self-signed and is the root
210208
* of the chain. Likewise if the cert is its own
211209
* authority.
212210
*/
213-
pr_debug("- no auth?\n");
214-
if (x509->raw_subject_size != x509->raw_issuer_size ||
215-
memcmp(x509->raw_subject, x509->raw_issuer,
216-
x509->raw_issuer_size) != 0)
217-
return 0;
218-
219-
ret = x509_check_signature(x509->pub, x509);
220-
if (ret < 0)
221-
goto maybe_missing_crypto_in_x509;
211+
if (x509->unsupported_sig)
212+
goto unsupported_crypto_in_x509;
222213
x509->signer = x509;
223214
pr_debug("- self-signed\n");
224215
return 0;
@@ -270,7 +261,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
270261
sinfo->index);
271262
return 0;
272263
}
273-
ret = x509_check_signature(p->pub, x509);
264+
ret = public_key_verify_signature(p->pub, p->sig);
274265
if (ret < 0)
275266
return ret;
276267
x509->signer = p;
@@ -282,16 +273,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
282273
might_sleep();
283274
}
284275

285-
maybe_missing_crypto_in_x509:
276+
unsupported_crypto_in_x509:
286277
/* Just prune the certificate chain at this point if we lack some
287278
* crypto module to go further. Note, however, we don't want to set
288-
* sinfo->missing_crypto as the signed info block may still be
279+
* sinfo->unsupported_crypto as the signed info block may still be
289280
* validatable against an X.509 cert lower in the chain that we have a
290281
* trusted copy of.
291282
*/
292-
if (ret == -ENOPKG)
293-
return 0;
294-
return ret;
283+
return 0;
295284
}
296285

297286
/*
@@ -378,9 +367,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
378367
enum key_being_used_for usage)
379368
{
380369
struct pkcs7_signed_info *sinfo;
381-
struct x509_certificate *x509;
382370
int enopkg = -ENOPKG;
383-
int ret, n;
371+
int ret;
384372

385373
kenter("");
386374

@@ -422,12 +410,6 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
422410
return -EINVAL;
423411
}
424412

425-
for (n = 0, x509 = pkcs7->certs; x509; x509 = x509->next, n++) {
426-
ret = x509_get_sig_params(x509);
427-
if (ret < 0)
428-
return ret;
429-
}
430-
431413
for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
432414
ret = pkcs7_verify_one(pkcs7, sinfo);
433415
if (ret < 0) {

‎crypto/asymmetric_keys/x509_cert_parser.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
108108

109109
cert->pub->keylen = ctx->key_size;
110110

111+
/* Grab the signature bits */
112+
ret = x509_get_sig_params(cert);
113+
if (ret < 0)
114+
goto error_decode;
115+
111116
/* Generate cert issuer + serial number key ID */
112117
kid = asymmetric_key_generate_id(cert->raw_serial,
113118
cert->raw_serial_size,
@@ -119,6 +124,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
119124
}
120125
cert->id = kid;
121126

127+
/* Detect self-signed certificates */
128+
ret = x509_check_for_self_signed(cert);
129+
if (ret < 0)
130+
goto error_decode;
131+
122132
kfree(ctx);
123133
return cert;
124134

‎crypto/asymmetric_keys/x509_parser.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ struct x509_certificate {
4040
bool seen; /* Infinite recursion prevention */
4141
bool verified;
4242
bool trusted;
43-
bool unsupported_crypto; /* T if can't be verified due to missing crypto */
43+
bool self_signed; /* T if self-signed (check unsupported_sig too) */
44+
bool unsupported_key; /* T if key uses unsupported crypto */
45+
bool unsupported_sig; /* T if signature uses unsupported crypto */
4446
};
4547

4648
/*
@@ -56,5 +58,4 @@ extern int x509_decode_time(time64_t *_t, size_t hdrlen,
5658
* x509_public_key.c
5759
*/
5860
extern int x509_get_sig_params(struct x509_certificate *cert);
59-
extern int x509_check_signature(const struct public_key *pub,
60-
struct x509_certificate *cert);
61+
extern int x509_check_for_self_signed(struct x509_certificate *cert);

‎crypto/asymmetric_keys/x509_public_key.c

Lines changed: 86 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,17 @@ int x509_get_sig_params(struct x509_certificate *cert)
161161

162162
pr_devel("==>%s()\n", __func__);
163163

164-
if (cert->unsupported_crypto)
165-
return -ENOPKG;
166-
if (sig->s)
164+
if (!cert->pub->pkey_algo)
165+
cert->unsupported_key = true;
166+
167+
if (!sig->pkey_algo)
168+
cert->unsupported_sig = true;
169+
170+
/* We check the hash if we can - even if we can't then verify it */
171+
if (!sig->hash_algo) {
172+
cert->unsupported_sig = true;
167173
return 0;
174+
}
168175

169176
sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
170177
if (!sig->s)
@@ -178,8 +185,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
178185
tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
179186
if (IS_ERR(tfm)) {
180187
if (PTR_ERR(tfm) == -ENOENT) {
181-
cert->unsupported_crypto = true;
182-
return -ENOPKG;
188+
cert->unsupported_sig = true;
189+
return 0;
183190
}
184191
return PTR_ERR(tfm);
185192
}
@@ -212,29 +219,53 @@ int x509_get_sig_params(struct x509_certificate *cert)
212219
pr_devel("<==%s() = %d\n", __func__, ret);
213220
return ret;
214221
}
215-
EXPORT_SYMBOL_GPL(x509_get_sig_params);
216222

217223
/*
218-
* Check the signature on a certificate using the provided public key
224+
* Check for self-signedness in an X.509 cert and if found, check the signature
225+
* immediately if we can.
219226
*/
220-
int x509_check_signature(const struct public_key *pub,
221-
struct x509_certificate *cert)
227+
int x509_check_for_self_signed(struct x509_certificate *cert)
222228
{
223-
int ret;
229+
int ret = 0;
224230

225231
pr_devel("==>%s()\n", __func__);
226232

227-
ret = x509_get_sig_params(cert);
228-
if (ret < 0)
229-
return ret;
233+
if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
234+
/* If the AKID is present it may have one or two parts. If
235+
* both are supplied, both must match.
236+
*/
237+
bool a = asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]);
238+
bool b = asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0]);
239+
240+
if (!a && !b)
241+
goto not_self_signed;
242+
243+
ret = -EKEYREJECTED;
244+
if (((a && !b) || (b && !a)) &&
245+
cert->sig->auth_ids[0] && cert->sig->auth_ids[1])
246+
goto out;
247+
}
248+
249+
ret = public_key_verify_signature(cert->pub, cert->sig);
250+
if (ret < 0) {
251+
if (ret == -ENOPKG) {
252+
cert->unsupported_sig = true;
253+
ret = 0;
254+
}
255+
goto out;
256+
}
257+
258+
pr_devel("Cert Self-signature verified");
259+
cert->self_signed = true;
230260

231-
ret = public_key_verify_signature(pub, cert->sig);
232-
if (ret == -ENOPKG)
233-
cert->unsupported_crypto = true;
234-
pr_debug("Cert Verification: %d\n", ret);
261+
out:
262+
pr_devel("<==%s() = %d\n", __func__, ret);
235263
return ret;
264+
265+
not_self_signed:
266+
pr_devel("<==%s() = 0 [not]\n", __func__);
267+
return 0;
236268
}
237-
EXPORT_SYMBOL_GPL(x509_check_signature);
238269

239270
/*
240271
* Check the new certificate against the ones in the trust keyring. If one of
@@ -252,22 +283,30 @@ static int x509_validate_trust(struct x509_certificate *cert,
252283
struct key *key;
253284
int ret = 1;
254285

286+
if (!sig->auth_ids[0] && !sig->auth_ids[1])
287+
return 1;
288+
255289
if (!trust_keyring)
256290
return -EOPNOTSUPP;
257-
258291
if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
259292
return -EPERM;
293+
if (cert->unsupported_sig)
294+
return -ENOPKG;
260295

261296
key = x509_request_asymmetric_key(trust_keyring,
262297
sig->auth_ids[0], sig->auth_ids[1],
263298
false);
264-
if (!IS_ERR(key)) {
265-
if (!use_builtin_keys
266-
|| test_bit(KEY_FLAG_BUILTIN, &key->flags))
267-
ret = x509_check_signature(key->payload.data[asym_crypto],
268-
cert);
269-
key_put(key);
299+
if (IS_ERR(key))
300+
return PTR_ERR(key);
301+
302+
if (!use_builtin_keys ||
303+
test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
304+
ret = public_key_verify_signature(
305+
key->payload.data[asym_crypto], cert->sig);
306+
if (ret == -ENOPKG)
307+
cert->unsupported_sig = true;
270308
}
309+
key_put(key);
271310
return ret;
272311
}
273312

@@ -290,34 +329,41 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
290329
pr_devel("Cert Issuer: %s\n", cert->issuer);
291330
pr_devel("Cert Subject: %s\n", cert->subject);
292331

293-
if (!cert->pub->pkey_algo ||
294-
!cert->sig->pkey_algo ||
295-
!cert->sig->hash_algo) {
332+
if (cert->unsupported_key) {
296333
ret = -ENOPKG;
297334
goto error_free_cert;
298335
}
299336

300337
pr_devel("Cert Key Algo: %s\n", cert->pub->pkey_algo);
301338
pr_devel("Cert Valid period: %lld-%lld\n", cert->valid_from, cert->valid_to);
302-
pr_devel("Cert Signature: %s + %s\n",
303-
cert->sig->pkey_algo,
304-
cert->sig->hash_algo);
305339

306340
cert->pub->id_type = "X509";
307341

308-
/* Check the signature on the key if it appears to be self-signed */
309-
if ((!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1]) ||
310-
asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]) ||
311-
asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0])) {
312-
ret = x509_check_signature(cert->pub, cert); /* self-signed */
313-
if (ret < 0)
314-
goto error_free_cert;
315-
} else if (!prep->trusted) {
342+
/* See if we can derive the trustability of this certificate.
343+
*
344+
* When it comes to self-signed certificates, we cannot evaluate
345+
* trustedness except by the fact that we obtained it from a trusted
346+
* location. So we just rely on x509_validate_trust() failing in this
347+
* case.
348+
*
349+
* Note that there's a possibility of a self-signed cert matching a
350+
* cert that we have (most likely a duplicate that we already trust) -
351+
* in which case it will be marked trusted.
352+
*/
353+
if (cert->unsupported_sig || cert->self_signed) {
354+
public_key_signature_free(cert->sig);
355+
cert->sig = NULL;
356+
} else {
357+
pr_devel("Cert Signature: %s + %s\n",
358+
cert->sig->pkey_algo, cert->sig->hash_algo);
359+
316360
ret = x509_validate_trust(cert, get_system_trusted_keyring());
317361
if (ret)
318362
ret = x509_validate_trust(cert, get_ima_mok_keyring());
363+
if (ret == -EKEYREJECTED)
364+
goto error_free_cert;
319365
if (!ret)
320-
prep->trusted = 1;
366+
prep->trusted = true;
321367
}
322368

323369
/* Propose a description */

0 commit comments

Comments
 (0)