summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Morris <james.l.morris@oracle.com>2017-12-09 14:39:48 +1100
committerJames Morris <james.l.morris@oracle.com>2017-12-09 14:39:48 +1100
commit4ded3bec65a07343258ed8fd9d46483f032d866f (patch)
treeecb134c1ac71a1c4ca59b50d4d097de7c3939cbf
parentf335195adf043168ee69d78ea72ac3e30f0c57ce (diff)
parent54c1fb39fe0495f846539ab765925b008f86801c (diff)
Merge tag 'keys-fixes-20171208' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs into keys-for-linus
Assorted fixes for keyrings, ASN.1, X.509 and PKCS#7.
-rw-r--r--crypto/asymmetric_keys/pkcs7_parser.c4
-rw-r--r--crypto/asymmetric_keys/pkcs7_trust.c2
-rw-r--r--crypto/asymmetric_keys/pkcs7_verify.c9
-rw-r--r--crypto/asymmetric_keys/public_key.c7
-rw-r--r--crypto/asymmetric_keys/x509_cert_parser.c2
-rw-r--r--crypto/asymmetric_keys/x509_public_key.c8
-rw-r--r--lib/asn1_decoder.c49
-rw-r--r--lib/oid_registry.c16
-rw-r--r--security/keys/key.c1
-rw-r--r--security/keys/keyctl.c24
-rw-r--r--security/keys/request_key.c48
11 files changed, 101 insertions, 69 deletions
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index c1ca1e86f5c4..a6dcaa659aa8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -148,8 +148,10 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
}
ret = pkcs7_check_authattrs(ctx->msg);
- if (ret < 0)
+ if (ret < 0) {
+ msg = ERR_PTR(ret);
goto out;
+ }
msg = ctx->msg;
ctx->msg = NULL;
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index f6a009d88a33..1f4e25f10049 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -69,7 +69,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
/* Self-signed certificates form roots of their own, and if we
* don't know them, then we can't accept them.
*/
- if (x509->next == x509) {
+ if (x509->signer == x509) {
kleave(" = -ENOKEY [unknown self-signed]");
return -ENOKEY;
}
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 2d93d9eccb4d..39e6de0c2761 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -59,11 +59,8 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
/* Digest the message [RFC2315 9.3] */
- ret = crypto_shash_init(desc);
- if (ret < 0)
- goto error;
- ret = crypto_shash_finup(desc, pkcs7->data, pkcs7->data_len,
- sig->digest);
+ ret = crypto_shash_digest(desc, pkcs7->data, pkcs7->data_len,
+ sig->digest);
if (ret < 0)
goto error;
pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
@@ -150,7 +147,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
sinfo->index, certix);
- if (x509->pub->pkey_algo != sinfo->sig->pkey_algo) {
+ if (strcmp(x509->pub->pkey_algo, sinfo->sig->pkey_algo) != 0) {
pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
sinfo->index);
continue;
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index bc3035ef27a2..de996586762a 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -73,7 +73,7 @@ int public_key_verify_signature(const struct public_key *pkey,
char alg_name_buf[CRYPTO_MAX_ALG_NAME];
void *output;
unsigned int outlen;
- int ret = -ENOMEM;
+ int ret;
pr_devel("==>%s()\n", __func__);
@@ -99,6 +99,7 @@ int public_key_verify_signature(const struct public_key *pkey,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
+ ret = -ENOMEM;
req = akcipher_request_alloc(tfm, GFP_KERNEL);
if (!req)
goto error_free_tfm;
@@ -127,7 +128,7 @@ int public_key_verify_signature(const struct public_key *pkey,
* signature and returns that to us.
*/
ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
- if (ret < 0)
+ if (ret)
goto out_free_output;
/* Do the actual verification step. */
@@ -142,6 +143,8 @@ error_free_req:
error_free_tfm:
crypto_free_akcipher(tfm);
pr_devel("<==%s() = %d\n", __func__, ret);
+ if (WARN_ON_ONCE(ret > 0))
+ ret = -EINVAL;
return ret;
}
EXPORT_SYMBOL_GPL(public_key_verify_signature);
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index dd03fead1ca3..ce2df8c9c583 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -409,6 +409,8 @@ int x509_extract_key_data(void *context, size_t hdrlen,
ctx->cert->pub->pkey_algo = "rsa";
/* Discard the BIT STRING metadata */
+ if (vlen < 1 || *(const u8 *)value != 0)
+ return -EBADMSG;
ctx->key = value + 1;
ctx->key_size = vlen - 1;
return 0;
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index c9013582c026..9338b4558cdc 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -79,11 +79,7 @@ int x509_get_sig_params(struct x509_certificate *cert)
desc->tfm = tfm;
desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
- ret = crypto_shash_init(desc);
- if (ret < 0)
- goto error_2;
- might_sleep();
- ret = crypto_shash_finup(desc, cert->tbs, cert->tbs_size, sig->digest);
+ ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size, sig->digest);
if (ret < 0)
goto error_2;
@@ -135,7 +131,7 @@ int x509_check_for_self_signed(struct x509_certificate *cert)
}
ret = -EKEYREJECTED;
- if (cert->pub->pkey_algo != cert->sig->pkey_algo)
+ if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo) != 0)
goto out;
ret = public_key_verify_signature(cert->pub, cert->sig);
diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 1ef0cec38d78..dc14beae2c9a 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -313,42 +313,47 @@ next_op:
/* Decide how to handle the operation */
switch (op) {
- case ASN1_OP_MATCH_ANY_ACT:
- case ASN1_OP_MATCH_ANY_ACT_OR_SKIP:
- case ASN1_OP_COND_MATCH_ANY_ACT:
- case ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP:
- ret = actions[machine[pc + 1]](context, hdr, tag, data + dp, len);
- if (ret < 0)
- return ret;
- goto skip_data;
-
- case ASN1_OP_MATCH_ACT:
- case ASN1_OP_MATCH_ACT_OR_SKIP:
- case ASN1_OP_COND_MATCH_ACT_OR_SKIP:
- ret = actions[machine[pc + 2]](context, hdr, tag, data + dp, len);
- if (ret < 0)
- return ret;
- goto skip_data;
-
case ASN1_OP_MATCH:
case ASN1_OP_MATCH_OR_SKIP:
+ case ASN1_OP_MATCH_ACT:
+ case ASN1_OP_MATCH_ACT_OR_SKIP:
case ASN1_OP_MATCH_ANY:
case ASN1_OP_MATCH_ANY_OR_SKIP:
+ case ASN1_OP_MATCH_ANY_ACT:
+ case ASN1_OP_MATCH_ANY_ACT_OR_SKIP:
case ASN1_OP_COND_MATCH_OR_SKIP:
+ case ASN1_OP_COND_MATCH_ACT_OR_SKIP:
case ASN1_OP_COND_MATCH_ANY:
case ASN1_OP_COND_MATCH_ANY_OR_SKIP:
- skip_data:
+ case ASN1_OP_COND_MATCH_ANY_ACT:
+ case ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP:
+
if (!(flags & FLAG_CONS)) {
if (flags & FLAG_INDEFINITE_LENGTH) {
+ size_t tmp = dp;
+
ret = asn1_find_indefinite_length(
- data, datalen, &dp, &len, &errmsg);
+ data, datalen, &tmp, &len, &errmsg);
if (ret < 0)
goto error;
- } else {
- dp += len;
}
pr_debug("- LEAF: %zu\n", len);
}
+
+ if (op & ASN1_OP_MATCH__ACT) {
+ unsigned char act;
+
+ if (op & ASN1_OP_MATCH__ANY)
+ act = machine[pc + 1];
+ else
+ act = machine[pc + 2];
+ ret = actions[act](context, hdr, tag, data + dp, len);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!(flags & FLAG_CONS))
+ dp += len;
pc += asn1_op_lengths[op];
goto next_op;
@@ -434,6 +439,8 @@ next_op:
else
act = machine[pc + 1];
ret = actions[act](context, hdr, 0, data + tdp, len);
+ if (ret < 0)
+ return ret;
}
pc += asn1_op_lengths[op];
goto next_op;
diff --git a/lib/oid_registry.c b/lib/oid_registry.c
index 41b9e50711a7..0bcac6ccb1b2 100644
--- a/lib/oid_registry.c
+++ b/lib/oid_registry.c
@@ -116,14 +116,14 @@ int sprint_oid(const void *data, size_t datasize, char *buffer, size_t bufsize)
int count;
if (v >= end)
- return -EBADMSG;
+ goto bad;
n = *v++;
ret = count = snprintf(buffer, bufsize, "%u.%u", n / 40, n % 40);
+ if (count >= bufsize)
+ return -ENOBUFS;
buffer += count;
bufsize -= count;
- if (bufsize == 0)
- return -ENOBUFS;
while (v < end) {
num = 0;
@@ -134,20 +134,24 @@ int sprint_oid(const void *data, size_t datasize, char *buffer, size_t bufsize)
num = n & 0x7f;
do {
if (v >= end)
- return -EBADMSG;
+ goto bad;
n = *v++;
num <<= 7;
num |= n & 0x7f;
} while (n & 0x80);
}
ret += count = snprintf(buffer, bufsize, ".%lu", num);
- buffer += count;
- if (bufsize <= count)
+ if (count >= bufsize)
return -ENOBUFS;
+ buffer += count;
bufsize -= count;
}
return ret;
+
+bad:
+ snprintf(buffer, bufsize, "(bad)");
+ return -EBADMSG;
}
EXPORT_SYMBOL_GPL(sprint_oid);
diff --git a/security/keys/key.c b/security/keys/key.c
index 66049183ad89..d97c9394b5dd 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -833,7 +833,6 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
key_check(keyring);
- key_ref = ERR_PTR(-EPERM);
if (!(flags & KEY_ALLOC_BYPASS_RESTRICTION))
restrict_link = keyring->restrict_link;
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 76d22f726ae4..1ffe60bb2845 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1588,9 +1588,8 @@ error_keyring:
* The caller must have Setattr permission to change keyring restrictions.
*
* The requested type name may be a NULL pointer to reject all attempts
- * to link to the keyring. If _type is non-NULL, _restriction can be
- * NULL or a pointer to a string describing the restriction. If _type is
- * NULL, _restriction must also be NULL.
+ * to link to the keyring. In this case, _restriction must also be NULL.
+ * Otherwise, both _type and _restriction must be non-NULL.
*
* Returns 0 if successful.
*/
@@ -1598,7 +1597,6 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type,
const char __user *_restriction)
{
key_ref_t key_ref;
- bool link_reject = !_type;
char type[32];
char *restriction = NULL;
long ret;
@@ -1607,31 +1605,29 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type,
if (IS_ERR(key_ref))
return PTR_ERR(key_ref);
+ ret = -EINVAL;
if (_type) {
- ret = key_get_type_from_user(type, _type, sizeof(type));
- if (ret < 0)
+ if (!_restriction)
goto error;
- }
- if (_restriction) {
- if (!_type) {
- ret = -EINVAL;
+ ret = key_get_type_from_user(type, _type, sizeof(type));
+ if (ret < 0)
goto error;
- }
restriction = strndup_user(_restriction, PAGE_SIZE);
if (IS_ERR(restriction)) {
ret = PTR_ERR(restriction);
goto error;
}
+ } else {
+ if (_restriction)
+ goto error;
}
- ret = keyring_restrict(key_ref, link_reject ? NULL : type, restriction);
+ ret = keyring_restrict(key_ref, _type ? type : NULL, restriction);
kfree(restriction);
-
error:
key_ref_put(key_ref);
-
return ret;
}
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index e8036cd0ad54..114f7408feee 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -251,11 +251,12 @@ static int construct_key(struct key *key, const void *callout_info,
* The keyring selected is returned with an extra reference upon it which the
* caller must release.
*/
-static void construct_get_dest_keyring(struct key **_dest_keyring)
+static int construct_get_dest_keyring(struct key **_dest_keyring)
{
struct request_key_auth *rka;
const struct cred *cred = current_cred();
struct key *dest_keyring = *_dest_keyring, *authkey;
+ int ret;
kenter("%p", dest_keyring);
@@ -264,6 +265,8 @@ static void construct_get_dest_keyring(struct key **_dest_keyring)
/* the caller supplied one */
key_get(dest_keyring);
} else {
+ bool do_perm_check = true;
+
/* use a default keyring; falling through the cases until we
* find one that we actually have */
switch (cred->jit_keyring) {
@@ -278,8 +281,10 @@ static void construct_get_dest_keyring(struct key **_dest_keyring)
dest_keyring =
key_get(rka->dest_keyring);
up_read(&authkey->sem);
- if (dest_keyring)
+ if (dest_keyring) {
+ do_perm_check = false;
break;
+ }
}
case KEY_REQKEY_DEFL_THREAD_KEYRING:
@@ -314,11 +319,29 @@ static void construct_get_dest_keyring(struct key **_dest_keyring)
default:
BUG();
}
+
+ /*
+ * Require Write permission on the keyring. This is essential
+ * because the default keyring may be the session keyring, and
+ * joining a keyring only requires Search permission.
+ *
+ * However, this check is skipped for the "requestor keyring" so
+ * that /sbin/request-key can itself use request_key() to add
+ * keys to the original requestor's destination keyring.
+ */
+ if (dest_keyring && do_perm_check) {
+ ret = key_permission(make_key_ref(dest_keyring, 1),
+ KEY_NEED_WRITE);
+ if (ret) {
+ key_put(dest_keyring);
+ return ret;
+ }
+ }
}
*_dest_keyring = dest_keyring;
kleave(" [dk %d]", key_serial(dest_keyring));
- return;
+ return 0;
}
/*
@@ -444,11 +467,15 @@ static struct key *construct_key_and_link(struct keyring_search_context *ctx,
if (ctx->index_key.type == &key_type_keyring)
return ERR_PTR(-EPERM);
- user = key_user_lookup(current_fsuid());
- if (!user)
- return ERR_PTR(-ENOMEM);
+ ret = construct_get_dest_keyring(&dest_keyring);
+ if (ret)
+ goto error;
- construct_get_dest_keyring(&dest_keyring);
+ user = key_user_lookup(current_fsuid());
+ if (!user) {
+ ret = -ENOMEM;
+ goto error_put_dest_keyring;
+ }
ret = construct_alloc_key(ctx, dest_keyring, flags, user, &key);
key_user_put(user);
@@ -463,7 +490,7 @@ static struct key *construct_key_and_link(struct keyring_search_context *ctx,
} else if (ret == -EINPROGRESS) {
ret = 0;
} else {
- goto couldnt_alloc_key;
+ goto error_put_dest_keyring;
}
key_put(dest_keyring);
@@ -473,8 +500,9 @@ static struct key *construct_key_and_link(struct keyring_search_context *ctx,
construction_failed:
key_negate_and_link(key, key_negative_timeout, NULL, NULL);
key_put(key);
-couldnt_alloc_key:
+error_put_dest_keyring:
key_put(dest_keyring);
+error:
kleave(" = %d", ret);
return ERR_PTR(ret);
}
@@ -546,9 +574,7 @@ struct key *request_key_and_link(struct key_type *type,
if (!IS_ERR(key_ref)) {
key = key_ref_to_ptr(key_ref);
if (dest_keyring) {
- construct_get_dest_keyring(&dest_keyring);
ret = key_link(dest_keyring, key);
- key_put(dest_keyring);
if (ret < 0) {
key_put(key);
key = ERR_PTR(ret);