Skip to contentSkip to conversation

KEYS: Account for asymmetric key payload data in the quota

PATCH Superseded Yuqi Xu 3 participants 3 messages Sep 28 – Oct 5

Subsystems (4)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Yuqi Xu

author
#0 lore raw

PATCH v2 1/1 KEYS: Account for asymmetric key payload data in the quota

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.

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")
Assisted-by LLM

---
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. The truncation is real only past USHRT_MAX, the unprivileged maxbytes is 20000 so add_key() fails with -EDQUOT first, and this patch does not widen key->quotalen. Replied publicly: Message-ID [email protected]
- v1 Link: https://lore.kernel.org/all/[email protected]/

 crypto/asymmetric_keys/pkcs8_parser.c    |  2 +-
 crypto/asymmetric_keys/x509_public_key.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)
crypto/asymmetric_keys/pkcs8_parser.c +1 -1
index 9dd1c181789a..fa8b142a8e14 100644
@@ -150,7 +150,7 @@ static int pkcs8_key_preparse(struct key_preparsed_payload *prep)
 	prep->payload.data[asym_key_ids] = NULL;
 	prep->payload.data[asym_crypto] = pub;
 	prep->payload.data[asym_auth] = NULL;
-	prep->quotalen = 100;
+	prep->quotalen = sizeof(*pub) + pub->keylen;
 	return 0;
 }
 
crypto/asymmetric_keys/x509_public_key.c +22 -1
index 25cf8ac7f257..f2bdd0244262 100644
@@ -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;
+	int i;
 
 	cert = x509_cert_parse(prep->data, prep->datalen);
 	if (IS_ERR(cert))
@@ -228,7 +230,26 @@ 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;
+
+	/* Charge the memory retained in the payload, not a fixed estimate */
+	prep->quotalen = sizeof(*cert->pub) + cert->pub->keylen +
+			 cert->pub->paramlen;
+	if (cert->sig) {
+		prep->quotalen += sizeof(*cert->sig) + cert->sig->s_size;
+		if (cert->sig->m_free)
+			prep->quotalen += cert->sig->m_size;
+		for (i = 0; i < ARRAY_SIZE(cert->sig->auth_ids); i++) {
+			id = cert->sig->auth_ids[i];
+			if (id)
+				prep->quotalen += sizeof(*id) + id->len;
+		}
+	}
+	prep->quotalen += sizeof(*kids);
+	for (i = 0; i < ARRAY_SIZE(kids->id); i++) {
+		id = kids->id[i];
+		if (id)
+			prep->quotalen += sizeof(*id) + id->len;
+	}
 
 	/* We've finished with the certificate */
 	cert->pub = NULL;
Signature · 1 line
2.55.0
maintainer Reviewed-by on v2 1/1
#1 lore raw
Quoted 36 lines · Yuqi Xu

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.

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.

Quoted 6 lines
Fixes c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
Assisted-by LLM
Quoted 74 lines

---
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. The truncation is real only past USHRT_MAX, the unprivileged maxbytes is 20000 so add_key() fails with -EDQUOT first, and this patch does not widen key->quotalen. Replied publicly: Message-ID [email protected]
- v1 Link: https://lore.kernel.org/all/[email protected]/

 crypto/asymmetric_keys/pkcs8_parser.c    |  2 +-
 crypto/asymmetric_keys/x509_public_key.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)
crypto/asymmetric_keys/pkcs8_parser.c +1 -1
index 9dd1c181789a..fa8b142a8e14 100644
@@ -150,7 +150,7 @@ static int pkcs8_key_preparse(struct key_preparsed_payload *prep)
        prep->payload.data[asym_key_ids] = NULL;
        prep->payload.data[asym_crypto] = pub;
        prep->payload.data[asym_auth] = NULL;
-       prep->quotalen = 100;
+       prep->quotalen = sizeof(*pub) + pub->keylen;
        return 0;
 }
 
crypto/asymmetric_keys/x509_public_key.c +22 -2
index 25cf8ac7f257..f2bdd0244262 100644
@@ -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;
+       int i;
 
        cert = x509_cert_parse(prep->data, prep->datalen);
        if (IS_ERR(cert))
@@ -228,7 +230,26 @@ 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;
+
+       /* Charge the memory retained in the payload, not a fixed estimate */
+       prep->quotalen = sizeof(*cert->pub) + cert->pub->keylen +
+                        cert->pub->paramlen;
+       if (cert->sig) {
+               prep->quotalen += sizeof(*cert->sig) + cert->sig->s_size;
+               if (cert->sig->m_free)
+                       prep->quotalen += cert->sig->m_size;
+               for (i = 0; i < ARRAY_SIZE(cert->sig->auth_ids); i++) {
+                       id = cert->sig->auth_ids[i];
+                       if (id)
+                               prep->quotalen += sizeof(*id) + id->len;
+               }
+       }
+       prep->quotalen += sizeof(*kids);
+       for (i = 0; i < ARRAY_SIZE(kids->id); i++) {
+               id = kids->id[i];
+               if (id)
+                       prep->quotalen += sizeof(*id) + id->len;
+       }
 
        /* We've finished with the certificate */
        cert->pub = NULL;
--
Quoted 3 lines

2.55.0

Thanks

maintainer
#2 lore raw
In reply to Yuqi Xu
Quoted 53 lines · Yuqi Xu

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.

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")
Assisted-by LLM

---
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. The truncation is real only past USHRT_MAX, the unprivileged maxbytes is 20000 so add_key() fails with -EDQUOT first, and this patch does not widen key->quotalen. Replied publicly: Message-ID [email protected]
- v1 Link: https://lore.kernel.org/all/[email protected]/

Quoted 3 lines
 crypto/asymmetric_keys/pkcs8_parser.c    |  2 +-
 crypto/asymmetric_keys/x509_public_key.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

This patch wasn't reviewed by Sashiko. Please repost.

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt