2018-06-26 16:12:44

by David Howells

[permalink] [raw]
Subject: [PATCH 0/3] KEYS: Miscellaneous fixes


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(-)



2018-06-26 16:12:08

by David Howells

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

From: Nick Desaulniers <[email protected]>

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

Signed-off-by: Nick Desaulniers <[email protected]>
Signed-off-by: David Howells <[email protected]>
---

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[];


2018-06-26 16:12:52

by David Howells

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

From: Maciej S. Szmigiero <[email protected]>

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 <[email protected]>
Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
Cc: [email protected]
Signed-off-by: David Howells <[email protected]>
---

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;


2018-06-26 16:13:04

by David Howells

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

From: Eric Biggers <[email protected]>

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: [email protected]
Reported-by: [email protected]
Reported-by: [email protected]
Reported-by: [email protected]
Fixes: 383203eff718 ("dh key: get rid of stack allocated array")
Signed-off-by: Eric Biggers <[email protected]>
Acked-by: Kees Cook <[email protected]>
Acked-by: Tycho Andersen <[email protected]>
---

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) {


2018-06-26 16:40:39

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 0/3] KEYS: Miscellaneous fixes

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
<[email protected]>