[PATCH 0/3] KEYS: Miscellaneous fixes

STALE2995d

Revision v1 of 7 in this series.

5 messages, 2 authors, 2018-06-26 · open the first message on its own page

[PATCH 0/3] KEYS: Miscellaneous fixes

From: David Howells <dhowells@redhat.com>
Date: 2018-06-26 15:59:33

Hi James,

Here's a bunch of miscellaneous fixes:

 (1) Fix the handling of the X.509 signature BIT STRING.

 (2) Fix a section declaration.

 (3) Fix rounding in KDF.

Could you pass these on to Linus please?

The patches can be found here tagged thusly:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	keys-fixes-20180626

and also on the following branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes

David
---
Eric Biggers (1):
      dh key: fix rounding up KDF output length

Maciej S. Szmigiero (1):
      X.509: unpack RSA signatureValue field from BIT STRING

Nick Desaulniers (1):
      certs/blacklist: fix const confusion


 certs/blacklist.h                         |    2 +-
 crypto/asymmetric_keys/x509_cert_parser.c |    9 +++++++++
 security/keys/dh.c                        |    6 ++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

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

[PATCH 1/3] X.509: unpack RSA signatureValue field from BIT STRING

From: David Howells <dhowells@redhat.com>
Date: 2018-06-26 15:59:40

From: Maciej S. Szmigiero <redacted>

The signatureValue field of a X.509 certificate is encoded as a BIT STRING.
For RSA signatures this BIT STRING is of so-called primitive subtype, which
contains a u8 prefix indicating a count of unused bits in the encoding.

We have to strip this prefix from signature data, just as we already do for
key data in x509_extract_key_data() function.

This wasn't noticed earlier because this prefix byte is zero for RSA key
sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero
prefixes has no bearing on its value.

The signature length, however was incorrect, which is a problem for RSA
implementations that need it to be exactly correct (like AMD CCP).

Signed-off-by: Maciej S. Szmigiero <redacted>
Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
Cc: stable at vger.kernel.org
Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/x509_cert_parser.c |    9 +++++++++
 1 file changed, 9 insertions(+)
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 7d81e6bb461a..b6cabac4b62b 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -249,6 +249,15 @@ int x509_note_signature(void *context, size_t hdrlen,
 		return -EINVAL;
 	}
 
+	if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0) {
+		/* Discard the BIT STRING metadata */
+		if (vlen < 1 || *(const u8 *)value != 0)
+			return -EBADMSG;
+
+		value++;
+		vlen--;
+	}
+
 	ctx->cert->raw_sig = value;
 	ctx->cert->raw_sig_size = vlen;
 	return 0;

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

[PATCH 2/3] certs/blacklist: fix const confusion

From: David Howells <dhowells@redhat.com>
Date: 2018-06-26 15:59:46

From: Nick Desaulniers <redacted>

Fixes commit 2be04df5668d ("certs/blacklist_nohashes.c: fix const confusion
in certs blacklist")

Signed-off-by: Nick Desaulniers <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 certs/blacklist.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/certs/blacklist.h b/certs/blacklist.h
index 150d82da8e99..1efd6fa0dc60 100644
--- a/certs/blacklist.h
+++ b/certs/blacklist.h
@@ -1,3 +1,3 @@
 #include <linux/kernel.h>
 
-extern const char __initdata *const blacklist_hashes[];
+extern const char __initconst *const blacklist_hashes[];

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

[PATCH 3/3] dh key: fix rounding up KDF output length

From: David Howells <dhowells@redhat.com>
Date: 2018-06-26 15:59:54

From: Eric Biggers <redacted>

Commit 383203eff718 ("dh key: get rid of stack allocated array") changed
kdf_ctr() to assume that the length of key material to derive is a
multiple of the digest size.  The length was supposed to be rounded up
accordingly.  However, the round_up() macro was used which only gives
the correct result on power-of-2 arguments, whereas not all hash
algorithms have power-of-2 digest sizes.  In some cases this resulted in
a write past the end of the 'outbuf' buffer.

Fix it by switching to roundup(), which works for non-power-of-2 inputs.

Reported-by: syzbot+486f97f892efeb2075a3 at syzkaller.appspotmail.com
Reported-by: syzbot+29d17b7898b41ee120a5 at syzkaller.appspotmail.com
Reported-by: syzbot+8a608baf8751184ec727 at syzkaller.appspotmail.com
Reported-by: syzbot+d04e58bd384f1fe0b112 at syzkaller.appspotmail.com
Fixes: 383203eff718 ("dh key: get rid of stack allocated array")
Signed-off-by: Eric Biggers <redacted>
Acked-by: Kees Cook <redacted>
Acked-by: Tycho Andersen <redacted>
---

 security/keys/dh.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index f7403821db7f..b203f7758f97 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -142,6 +142,8 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc)
  * The src pointer is defined as Z || other info where Z is the shared secret
  * from DH and other info is an arbitrary string (see SP800-56A section
  * 5.8.1.2).
+ *
+ * 'dlen' must be a multiple of the digest size.
  */
 static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
 		   u8 *dst, unsigned int dlen, unsigned int zlen)
@@ -205,8 +207,8 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
 {
 	uint8_t *outbuf = NULL;
 	int ret;
-	size_t outbuf_len = round_up(buflen,
-			             crypto_shash_digestsize(sdesc->shash.tfm));
+	size_t outbuf_len = roundup(buflen,
+				    crypto_shash_digestsize(sdesc->shash.tfm));
 
 	outbuf = kmalloc(outbuf_len, GFP_KERNEL);
 	if (!outbuf) {

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

Re: [PATCH 0/3] KEYS: Miscellaneous fixes

From: James Morris <jmorris@namei.org>
Date: 2018-06-26 16:39:48

On Tue, 26 Jun 2018, David Howells wrote:
Hi James,

Here's a bunch of miscellaneous fixes:

 (1) Fix the handling of the X.509 signature BIT STRING.

 (2) Fix a section declaration.

 (3) Fix rounding in KDF.

Could you pass these on to Linus please?
Sure, the RSA one is already merged.
The patches can be found here tagged thusly:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	keys-fixes-20180626

and also on the following branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes

David
---
Eric Biggers (1):
      dh key: fix rounding up KDF output length

Maciej S. Szmigiero (1):
      X.509: unpack RSA signatureValue field from BIT STRING

Nick Desaulniers (1):
      certs/blacklist: fix const confusion


 certs/blacklist.h                         |    2 +-
 crypto/asymmetric_keys/x509_cert_parser.c |    9 +++++++++
 security/keys/dh.c                        |    6 ++++--
 3 files changed, 14 insertions(+), 3 deletions(-)
-- 
James Morris
[off-list ref]

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help