linux-kernel - [RFC][PATCH] KEYS: Sort out big

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10390.1469625815@warthog.procyon.org.uk>
Date:	Wed, 27 Jul 2016 14:23:35 +0100
From:	David Howells <dhowells@...hat.com>
To:	Peter Hlavaty <zer0mem@...oo.com>,
	Kirill Marinushkin <k.marinushkin@...il.com>
Cc:	dhowells@...hat.com, Greg KH <gregkh@...uxfoundation.org>,
	James Morris <james.l.morris@...cle.com>,
	"Serge E. Hallyn" <serge@...lyn.com>, keyrings@...r.kernel.org,
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC][PATCH] KEYS: Sort out big_key initialisation

The attached patch *might* fix the problem that's being seen.  It certainly
fixes the init problem.

David
---
commit 57b7807d7f7ac69a818edb2ba4b1ec17b10cee4b
Author: David Howells <dhowells@...hat.com>
Date:   Wed Jul 27 13:45:02 2016 +0100

    KEYS: Sort out big_key initialisation
    
    big_key has two separate initialisation functions, one that registers the
    key type and one that registers the crypto.  If the key type fails to
    register, there's no problem if the crypto registers successfully because
    there's no way to reach the crypto except through the key type.
    
    However, if the key type registers successfully but the crypto does not,
    big_key_rng and big_key_blkcipher may end up set to NULL - but the code
    neither checks for this nor unregisters the big key key type.
    
    Furthermore, since the key type is registered before the crypto, it is
    theoretically possible for the kernel to try adding a big_key before the
    crypto is set up, leading to the same effect.
    
    Fix this by merging big_key_crypto_init() and big_key_init() and calling
    the resulting function late.  If they're going to be encrypted, we
    shouldn't be creating big_keys before we have the facilities to do the
    encryption available.  The key type registration is also moved after the
    crypto initialisation.
    
    The fix also includes message printing on failure.
    
    If the big_key type isn't correctly set up, simply doing:
    
    	dd if=/dev/zero bs=4096 count=1 | keyctl padd big_key a @s
    
    ought to cause an oops.
    
    Signed-off-by: David Howells <dhowells@...hat.com>
    cc: Peter Hlavaty <zer0mem@...oo.com>
    cc: Kirill Marinushkin <k.marinushkin@...il.com>
    cc: stable@...r.kernel.org

diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 9e443fccad4c..e9cc02e7aba8 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -9,6 +9,7 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) "big_key: "fmt
 #include <linux/init.h>
 #include <linux/seq_file.h>
 #include <linux/file.h>
@@ -336,43 +337,47 @@ error:
  */
 static int __init big_key_init(void)
 {
-	return register_key_type(&key_type_big_key);
-}
-
-/*
- * Initialize big_key crypto and RNG algorithms
- */
-static int __init big_key_crypto_init(void)
-{
-	int ret = -EINVAL;
+	struct crypto_blkcipher *cipher;
+	struct crypto_rng *rng;
+	int ret;
 
-	/* init RNG */
-	big_key_rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
-	if (IS_ERR(big_key_rng)) {
-		big_key_rng = NULL;
-		return -EFAULT;
+	rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
+	if (IS_ERR(rng)) {
+		pr_err("Can't alloc rng: %ld\n", PTR_ERR(rng));
+		return PTR_ERR(rng);
 	}
 
+	big_key_rng = rng;
+
 	/* seed RNG */
-	ret = crypto_rng_reset(big_key_rng, NULL, crypto_rng_seedsize(big_key_rng));
-	if (ret)
-		goto error;
+	ret = crypto_rng_reset(rng, NULL, crypto_rng_seedsize(rng));
+	if (ret) {
+		pr_err("Can't reset rng: %d\n", ret);
+		goto error_rng;
+	}
 
-	/* init block cipher */
-	big_key_blkcipher = crypto_alloc_blkcipher(big_key_alg_name, 0, 0);
-	if (IS_ERR(big_key_blkcipher)) {
-		big_key_blkcipher = NULL;
-		ret = -EFAULT;
-		goto error;
+	cipher = crypto_alloc_blkcipher(big_key_alg_name, 0, 0);
+	if (IS_ERR(cipher)) {
+		ret = PTR_ERR(cipher);
+		pr_err("Can't alloc crypto: %d\n", ret);
+		goto error_rng;
+	}
+
+	big_key_blkcipher = cipher;
+	
+	ret = register_key_type(&key_type_big_key);
+	if (ret < 0) {
+		pr_err("Can't register type: %d\n", ret);
+		goto error_cipher;
 	}
 
 	return 0;
 
-error:
+error_cipher:
+	crypto_free_blkcipher(big_key_blkcipher);
+error_rng:
 	crypto_free_rng(big_key_rng);
-	big_key_rng = NULL;
 	return ret;
 }
 
-device_initcall(big_key_init);
-late_initcall(big_key_crypto_init);
+late_initcall(big_key_init);