add_key("asymmetric", ...) preparses the supplied blob and then instantiates the key with generic_key_instantiate(), which charges prep->quotalen to the owning user's key quota through key_payload_reserve(). pkcs8_parse() duplicates the attacker-controlled PrivateKey OCTET STRING with kmemdup() into pub->key. pkcs8_key_preparse() stores that object in the asymmetric-key payload but hard-codes prep->quotalen to 100, so the quota records 100 bytes regardless of the retained private key size. The ASN.1 decoder accepts blobs up to 65535 bytes, so one key can pin about 64 KiB while consuming 100 quota bytes. With the default non-root limits (200 keys / 20000 bytes) a user can retain roughly 12 MiB instead of the 20 KiB the limit is meant to allow. x509_key_preparse() keeps the parsed public key, its parameters, the signature and the authority key IDs, but also charges a fixed 100 bytes. PKCS#8 later repeated the same fixed-100 accounting. The same add_key() call reaches the X.509 parser, so fixing only PKCS#8 would leave the bypass reachable. Charge the size of the retained payload instead. An oversized PKCS#8 key now fails with -EDQUOT instead of being accepted for 100 bytes, and the same applies to an X.509 certificate that retains an oversized public key or set of authority key IDs. A charge which does not fit in unsigned short is rejected with -EINVAL before the payload is installed, because key_payload_reserve() keeps the full delta in the user counter and refunds only the truncated key->quotalen. Measured on v7.3-rc1 (10396a2d6d41), 2 vCPU / 2 GiB QEMU guest, as uid 1000, adding PKCS#8 keys with a 65000-byte PrivateKey field. Before the patch, 176 keys were retained (about 11.4 MiB) while /proc/key-users charged only 19712 bytes. After the patch the oversized key is rejected immediately, and a 200-byte PrivateKey still loads and is charged 264 bytes. A small X.509 certificate whose only large object is a 26000-byte authorityKeyIdentifier is likewise rejected, while the unpatched kernel accepted it and charged 109 bytes. Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates") Cc: stable@vger.kernel.org Reported-by: Vega Assisted-by: LLM Signed-off-by: Yuqi Xu Reviewed-by: Ren Wei --- Changes in v3: - Sashiko (High): "keys: integer truncation of key->quotalen causes permanent quota leak" on pkcs8_key_preparse() and x509_key_preparse(). A charge above USHRT_MAX is now rejected with -EINVAL before the payload is installed. key_payload_reserve() keeps the full delta in the user quota and refunds only the truncated key->quotalen, so that charge would leak quota. - Reposted so Sashiko records a review against this Message-ID, as requested by Herbert Xu. v2 was posted and Ignat Korchagin replied. - v2 Link: https://lore.kernel.org/all/4a0b2e00f5cc62c173a01fec2f56eab1b04d70bb.1790568982.git.xuyuqiabc@gmail.com/ Changes in v2: - Move the bug details and the measured quota numbers into this commit message, as requested by Ignat Korchagin. - Sashiko (High): "keys: integer truncation in key->quotalen causes permanent quota leak" on pkcs8_key_preparse() and x509_key_preparse(). Not changed in v2. The same finding is addressed in v3. - v1 Link: https://lore.kernel.org/all/cover.1789801335.git.xuyuqiabc@gmail.com/ diff --git a/crypto/asymmetric_keys/pkcs8_parser.c b/crypto/asymmetric_keys/pkcs8_parser.c index 9dd1c181789a..e1bb37b6949a 100644 --- a/crypto/asymmetric_keys/pkcs8_parser.c +++ b/crypto/asymmetric_keys/pkcs8_parser.c @@ -136,6 +136,7 @@ static struct public_key *pkcs8_parse(const void *data, size_t datalen) static int pkcs8_key_preparse(struct key_preparsed_payload *prep) { struct public_key *pub; + size_t quotalen; pub = pkcs8_parse(prep->data, prep->datalen); if (IS_ERR(pub)) @@ -144,13 +145,20 @@ static int pkcs8_key_preparse(struct key_preparsed_payload *prep) pr_devel("Cert Key Algo: %s\n", pub->pkey_algo); pub->id_type = "PKCS8"; + quotalen = sizeof(*pub) + pub->keylen; + /* A charge past USHRT_MAX would truncate and leak quota */ + if (quotalen > USHRT_MAX) { + public_key_free(pub); + return -EINVAL; + } + /* We're pinning the module by being linked against it */ __module_get(public_key_subtype.owner); prep->payload.data[asym_subtype] = &public_key_subtype; prep->payload.data[asym_key_ids] = NULL; prep->payload.data[asym_crypto] = pub; prep->payload.data[asym_auth] = NULL; - prep->quotalen = 100; + prep->quotalen = quotalen; return 0; } diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index 25cf8ac7f257..ecceeb291833 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -163,9 +163,11 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) { struct x509_certificate *cert __free(x509_free_certificate) = NULL; struct asymmetric_key_ids *kids __free(kfree) = NULL; + struct asymmetric_key_id *id; char *p, *desc __free(kfree) = NULL; const char *q; - size_t srlen, sulen; + size_t srlen, sulen, quotalen; + int i; cert = x509_cert_parse(prep->data, prep->datalen); if (IS_ERR(cert)) @@ -221,6 +223,31 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) if (IS_ERR(kids->id[2])) return PTR_ERR(kids->id[2]); + /* Charge the memory retained in the payload, not a fixed estimate */ + quotalen = sizeof(*cert->pub) + cert->pub->keylen + + cert->pub->paramlen; + if (cert->sig) { + quotalen += sizeof(*cert->sig) + cert->sig->s_size; + if (cert->sig->m_free) + quotalen += cert->sig->m_size; + for (i = 0; i < ARRAY_SIZE(cert->sig->auth_ids); i++) { + id = cert->sig->auth_ids[i]; + if (id) + quotalen += sizeof(*id) + id->len; + } + } + quotalen += sizeof(*kids); + for (i = 0; i < ARRAY_SIZE(kids->id); i++) { + id = kids->id[i]; + if (id) + quotalen += sizeof(*id) + id->len; + } + /* A charge past USHRT_MAX would truncate and leak quota */ + if (quotalen > USHRT_MAX) { + kfree(kids->id[2]); + return -EINVAL; + } + /* We're pinning the module by being linked against it */ __module_get(public_key_subtype.owner); prep->payload.data[asym_subtype] = &public_key_subtype; @@ -228,7 +255,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) prep->payload.data[asym_crypto] = cert->pub; prep->payload.data[asym_auth] = cert->sig; prep->description = desc; - prep->quotalen = 100; + prep->quotalen = quotalen; /* We've finished with the certificate */ cert->pub = NULL; -- 2.56.0