Skip to contentSkip to conversation

DRBG: Rebasing unapplied patches

PATCH In review Stephan Mueller 2 participants 8 messages Jul 6, 2014 – Jul 8, 2014

Subsystems (2)
Linux-next tree Crypto API
lore
Share & tools
lore

Stephan Mueller

author
#0 lore raw

cover DRBG: Rebasing unapplied patches

Hi,

This patchset superseeds the patch sets submitted with [1] and [2]. It rebases all non-applied patches to the current Herbert Xu's cryptodev-2.6 tree.

[1] https://lkml.org/lkml/2014/6/28/497
[2] https://lkml.org/lkml/2014/7/1/332

Stephan Mueller (6):
DRBG: cleanup of preprocessor macros
DRBG: Fix format string for debugging statements
DRBG: Call CTR DRBG DF function only once
DRBG: Select correct DRBG core for stdrng
DRBG: Mix a time stamp into DRBG state
DRBG: HMAC-SHA1 DRBG has crypto strength of 128 bits

 crypto/drbg.c         | 134 +++++++++++++++++++++++++++++---------------------
 include/crypto/drbg.h |   2 +-
 2 files changed, 78 insertions(+), 58 deletions(-)
Signature · 1 line
1.9.3
author
#1 lore raw

PATCH 1/6 DRBG: cleanup of preprocessor macros

The structure used to construct the module description line was marked problematic by the sparse code analysis tool. The module line description now does not contain any ifdefs to prevent error reports from sparse.

Reported-by kbuild test robot <[email protected]>
Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)
crypto/drbg.c +17 -11
index acc7523..cce915b 100644
@@ -356,6 +356,7 @@ static inline void drbg_add_buf(unsigned char *dst, size_t dstlen,
  ******************************************************************/
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
+#define CRYPTO_DRBG_CTR_STRING "CTR "
 static int drbg_kcapi_sym(struct drbg_state *drbg, const unsigned char *key,
 			  unsigned char *outval, const struct drbg_string *in);
 static int drbg_init_sym_kernel(struct drbg_state *drbg);

@@ -717,6 +718,7 @@ static int drbg_fini_hash_kernel(struct drbg_state *drbg);
 #endif /* (CONFIG_CRYPTO_DRBG_HASH || CONFIG_CRYPTO_DRBG_HMAC) */
 
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
+#define CRYPTO_DRBG_HMAC_STRING "HMAC "
 /* update function of HMAC DRBG as defined in 10.1.2.2 */
 static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
 			    int reseed)

@@ -836,6 +838,7 @@ static struct drbg_state_ops drbg_hmac_ops = {
  ******************************************************************/
 
 #ifdef CONFIG_CRYPTO_DRBG_HASH
+#define CRYPTO_DRBG_HASH_STRING "HASH "
 /*
  * scratchpad usage: as drbg_hash_update and drbg_hash_df are used
  * interlinked, the scratchpad is used as follows:

@@ -1867,7 +1870,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
 	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
-#elif CONFIG_CRYPTO_DRBG_HASH
+#elif defined CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
 #else
 	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);

@@ -2009,16 +2012,19 @@ void __exit drbg_exit(void)
 
 module_init(drbg_init);
 module_exit(drbg_exit);
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Stephan Mueller <[email protected]>");
-MODULE_DESCRIPTION("NIST SP800-90A Deterministic Random Bit Generator (DRBG) using following cores:"
-#ifdef CONFIG_CRYPTO_DRBG_HMAC
-"HMAC "
+#ifndef CRYPTO_DRBG_HASH_STRING
+#define CRYPTO_DRBG_HASH_STRING ""
 #endif
-#ifdef CONFIG_CRYPTO_DRBG_HASH
-"Hash "
+#ifndef CRYPTO_DRBG_HMAC_STRING
+#define CRYPTO_DRBG_HMAC_STRING ""
 #endif
-#ifdef CONFIG_CRYPTO_DRBG_CTR
-"CTR"
+#ifndef CRYPTO_DRBG_CTR_STRING
+#define CRYPTO_DRBG_CTR_STRING ""
 #endif
-);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <[email protected]>");
+MODULE_DESCRIPTION("NIST SP800-90A Deterministic Random Bit Generator (DRBG) "
+		   "using following cores: "
+		   CRYPTO_DRBG_HASH_STRING
+		   CRYPTO_DRBG_HMAC_STRING
+		   CRYPTO_DRBG_CTR_STRING);
Signature · 1 line
1.9.3
author
#2 lore raw

PATCH 2/6 DRBG: Fix format string for debugging statements

In reply to Stephan Mueller

The initial format strings caused warnings on several architectures. The updated format strings now match the variable types.

Reported-by kbuild test robot <[email protected]>
Reported-by Randy Dunlap <[email protected]>
CC Joe Perches <[email protected]>
Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
crypto/drbg.c +2 -2
index cce915b..c9b4c49 100644
@@ -1106,7 +1106,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 
 	/* 9.1 / 9.2 / 9.3.1 step 3 */
 	if (pers && pers->len > (drbg_max_addtl(drbg))) {
-		pr_devel("DRBG: personalization string too long %lu\n",
+		pr_devel("DRBG: personalization string too long %zu\n",
 			 pers->len);
 		return -EINVAL;
 	}

@@ -1984,7 +1984,7 @@ static int __init drbg_init(void)
 
 	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
 		pr_info("DRBG: Cannot register all DRBG types"
-			"(slots needed: %lu, slots available: %lu)\n",
+			"(slots needed: %zu, slots available: %zu)\n",
 			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
 		return ret;
 	}
Signature · 1 line
1.9.3
author
#3 lore raw

PATCH 3/6 DRBG: Call CTR DRBG DF function only once

In reply to Stephan Mueller

The CTR DRBG requires the update function to be called twice when generating a random number. In both cases, update function must process the additional information string by using the DF function. As the DF produces the same result in both cases, we can save one invocation of the DF function when the first DF function result is reused.

The result of the DF function is stored in the scratchpad storage. The patch ensures that the scratchpad is not cleared when we want to reuse the DF result. For achieving this, the CTR DRBG update function must know by whom and in which scenario it is called. This information is provided with the reseed parameter to the update function.

Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)
crypto/drbg.c +22 -19
index c9b4c49..dba5ed2 100644
@@ -562,7 +562,21 @@ out:
 	return ret;
 }
 
-/* update function of CTR DRBG as defined in 10.2.1.2 */
+/*
+ * update function of CTR DRBG as defined in 10.2.1.2
+ *
+ * The reseed variable has an enhanced meaning compared to the update
+ * functions of the other DRBGs as follows:
+ * 0 => initial seed from initialization
+ * 1 => reseed via drbg_seed
+ * 2 => first invocation from drbg_ctr_update when addtl is present. In
+ *      this case, the df_data scratchpad is not deleted so that it is
+ *      available for another calls to prevent calling the DF function
+ *      again.
+ * 3 => second invocation from drbg_ctr_update. When the update function
+ *      was called with addtl, the df_data memory already contains the
+ *      DFed addtl information and we do not need to call DF again.
+ */
 static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 			   int reseed)
 {

@@ -577,7 +591,8 @@ static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 	unsigned char prefix = DRBG_PREFIX1;
 
 	memset(temp, 0, drbg_statelen(drbg) + drbg_blocklen(drbg));
-	memset(df_data, 0, drbg_statelen(drbg));
+	if (3 > reseed)
+		memset(df_data, 0, drbg_statelen(drbg));
 
 	/* 10.2.1.3.2 step 2 and 10.2.1.4.2 step 2 */
 	if (seed) {

@@ -619,7 +634,8 @@ static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 
 out:
 	memset(temp, 0, drbg_statelen(drbg) + drbg_blocklen(drbg));
-	memset(df_data, 0, drbg_statelen(drbg));
+	if (2 != reseed)
+		memset(df_data, 0, drbg_statelen(drbg));
 	return ret;
 }
 

@@ -644,7 +660,7 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 		LIST_HEAD(addtllist);
 
 		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_ctr_update(drbg, &addtllist, 1);
+		ret = drbg_ctr_update(drbg, &addtllist, 2);
 		if (ret)
 			return 0;
 	}

@@ -675,21 +691,8 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 			drbg_add_buf(drbg->V, drbg_blocklen(drbg), &prefix, 1);
 	}
 
-	/*
-	 * 10.2.1.5.2 step 6
-	 * The following call invokes the DF function again which could be
-	 * optimized. In step 2, the "additional_input" after step 2 is the
-	 * output of the DF function. If this result would be saved, the DF
-	 * function would not need to be invoked again at this point.
-	 */
-	if (addtl && 0 < addtl->len) {
-		LIST_HEAD(addtllist);
-
-		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_ctr_update(drbg, &addtllist, 1);
-	} else {
-		ret = drbg_ctr_update(drbg, NULL, 1);
-	}
+	/* 10.2.1.5.2 step 6 */
+	ret = drbg_ctr_update(drbg, NULL, 3);
 	if (ret)
 		len = ret;
Signature · 1 line
1.9.3
author
#4 lore raw

PATCH 4/6 DRBG: Select correct DRBG core for stdrng

In reply to Stephan Mueller

When the DRBG is initialized, the core is looked up using the DRBG name. The name that can be used for the lookup is registered in cra_driver_name. The cra_name value contains stdrng.

Thus, the lookup code must use crypto_tfm_alg_driver_name to obtain the precise DRBG name and select the correct DRBG.

Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
crypto/drbg.c +1 -1
index dba5ed2..2a7860f 100644
@@ -1761,7 +1761,7 @@ static int drbg_kcapi_init(struct crypto_tfm *tfm)
 	bool pr = false;
 	int coreref = 0;
 
-	drbg_convert_tfm_core(crypto_tfm_alg_name(tfm), &coreref, &pr);
+	drbg_convert_tfm_core(crypto_tfm_alg_driver_name(tfm), &coreref, &pr);
 	/*
 	 * when personalization string is needed, the caller must call reset
 	 * and provide the personalization string as seed information
Signature · 1 line
1.9.3
author
#5 lore raw

PATCH 5/6 DRBG: Mix a time stamp into DRBG state

In reply to Stephan Mueller

The current locking approach of the DRBG tries to keep the protected code paths very minimal. It is therefore possible that two threads query one DRBG instance at the same time. When thread A requests random numbers, a shadow copy of the DRBG state is created upon which the request for A is processed. After finishing the state for A's request is merged back into the DRBG state. If now thread B requests random numbers from the same DRBG after the request for thread A is received, but before A's shadow state is merged back, the random numbers for B will be identical to the ones for A. Please note that the time window is very small for this scenario.

To prevent that there is even a theoretical chance for thread A and B having the same DRBG state, the current time stamp is provided as additional information string for each new request.

The addition of the time stamp as additional information string implies that now all generate functions must be capable to process a linked list with additional information strings instead of a scalar.

CC Rafael Aquini <[email protected]>
Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c         | 59 ++++++++++++++++++++++++++++++---------------------
 include/crypto/drbg.h |  2 +-
 2 files changed, 36 insertions(+), 25 deletions(-)
crypto/drbg.c +35 -24
index 2a7860f..a76b3cb 100644
@@ -646,7 +646,7 @@ out:
 /* Generate function of CTR DRBG as defined in 10.2.1.5.2 */
 static int drbg_ctr_generate(struct drbg_state *drbg,
 			     unsigned char *buf, unsigned int buflen,
-			     struct drbg_string *addtl)
+			     struct list_head *addtl)
 {
 	int len = 0;
 	int ret = 0;

@@ -656,11 +656,8 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 	memset(drbg->scratchpad, 0, drbg_blocklen(drbg));
 
 	/* 10.2.1.5.2 step 2 */
-	if (addtl && 0 < addtl->len) {
-		LIST_HEAD(addtllist);
-
-		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_ctr_update(drbg, &addtllist, 2);
+	if (addtl && !list_empty(addtl)) {
+		ret = drbg_ctr_update(drbg, addtl, 2);
 		if (ret)
 			return 0;
 	}

@@ -777,7 +774,7 @@ static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
 static int drbg_hmac_generate(struct drbg_state *drbg,
 			      unsigned char *buf,
 			      unsigned int buflen,
-			      struct drbg_string *addtl)
+			      struct list_head *addtl)
 {
 	int len = 0;
 	int ret = 0;

@@ -785,11 +782,8 @@ static int drbg_hmac_generate(struct drbg_state *drbg,
 	LIST_HEAD(datalist);
 
 	/* 10.1.2.5 step 2 */
-	if (addtl && 0 < addtl->len) {
-		LIST_HEAD(addtllist);
-
-		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_hmac_update(drbg, &addtllist, 1);
+	if (addtl && !list_empty(addtl)) {
+		ret = drbg_hmac_update(drbg, addtl, 1);
 		if (ret)
 			return ret;
 	}

@@ -813,14 +807,10 @@ static int drbg_hmac_generate(struct drbg_state *drbg,
 	}
 
 	/* 10.1.2.5 step 6 */
-	if (addtl && 0 < addtl->len) {
-		LIST_HEAD(addtllist);
-
-		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_hmac_update(drbg, &addtllist, 1);
-	} else {
+	if (addtl && !list_empty(addtl))
+		ret = drbg_hmac_update(drbg, addtl, 1);
+	else
 		ret = drbg_hmac_update(drbg, NULL, 1);
-	}
 	if (ret)
 		return ret;
 

@@ -944,7 +934,7 @@ out:
 
 /* processing of additional information string for Hash DRBG */
 static int drbg_hash_process_addtl(struct drbg_state *drbg,
-				   struct drbg_string *addtl)
+				   struct list_head *addtl)
 {
 	int ret = 0;
 	struct drbg_string data1, data2;

@@ -955,7 +945,7 @@ static int drbg_hash_process_addtl(struct drbg_state *drbg,
 	memset(drbg->scratchpad, 0, drbg_blocklen(drbg));
 
 	/* 10.1.1.4 step 2 */
-	if (!addtl || 0 == addtl->len)
+	if (!addtl || list_empty(addtl))
 		return 0;
 
 	/* 10.1.1.4 step 2a */

@@ -963,7 +953,7 @@ static int drbg_hash_process_addtl(struct drbg_state *drbg,
 	drbg_string_fill(&data2, drbg->V, drbg_statelen(drbg));
 	list_add_tail(&data1.list, &datalist);
 	list_add_tail(&data2.list, &datalist);
-	list_add_tail(&addtl->list, &datalist);
+	list_splice_tail(addtl, &datalist);
 	ret = drbg_kcapi_hash(drbg, NULL, drbg->scratchpad, &datalist);
 	if (ret)
 		goto out;

@@ -1029,7 +1019,7 @@ out:
 /* generate function for Hash DRBG as defined in  10.1.1.4 */
 static int drbg_hash_generate(struct drbg_state *drbg,
 			      unsigned char *buf, unsigned int buflen,
-			      struct drbg_string *addtl)
+			      struct list_head *addtl)
 {
 	int len = 0;
 	int ret = 0;

@@ -1347,6 +1337,12 @@ static int drbg_generate(struct drbg_state *drbg,
 {
 	int len = 0;
 	struct drbg_state *shadow = NULL;
+	LIST_HEAD(addtllist);
+	struct drbg_string timestamp;
+	union {
+		cycles_t cycles;
+		unsigned char char_cycles[sizeof(cycles_t)];
+	} now;
 
 	if (0 == buflen || !buf) {
 		pr_devel("DRBG: no output buffer provided\n");

@@ -1407,8 +1403,23 @@ static int drbg_generate(struct drbg_state *drbg,
 		/* 9.3.1 step 7.4 */
 		addtl = NULL;
 	}
+
+	/*
+	 * Mix the time stamp into the DRBG state if the DRBG is not in
+	 * test mode. If there are two callers invoking the DRBG at the same
+	 * time, i.e. before the first caller merges its shadow state back,
+	 * both callers would obtain the same random number stream without
+	 * changing the state here.
+	 */
+	if (!drbg->test_data) {
+		now.cycles = random_get_entropy();
+		drbg_string_fill(&timestamp, now.char_cycles, sizeof(cycles_t));
+		list_add_tail(&timestamp.list, &addtllist);
+	}
+	if (addtl && 0 < addtl->len)
+		list_add_tail(&addtl->list, &addtllist);
 	/* 9.3.1 step 8 and 10 */
-	len = shadow->d_ops->generate(shadow, buf, buflen, addtl);
+	len = shadow->d_ops->generate(shadow, buf, buflen, &addtllist);
 
 	/* 10.1.1.4 step 6, 10.1.2.5 step 7, 10.2.1.5.2 step 7 */
 	shadow->reseed_ctr++;
include/crypto/drbg.h +1 -1
index 4065dfc..831d786 100644
@@ -102,7 +102,7 @@ struct drbg_state_ops {
 		      int reseed);
 	int (*generate)(struct drbg_state *drbg,
 			unsigned char *buf, unsigned int buflen,
-			struct drbg_string *addtl);
+			struct list_head *addtl);
 	int (*crypto_init)(struct drbg_state *drbg);
 	int (*crypto_fini)(struct drbg_state *drbg);
Signature · 1 line
1.9.3
author
#6 lore raw

PATCH 6/6 DRBG: HMAC-SHA1 DRBG has crypto strength of 128 bits

In reply to Stephan Mueller

The patch corrects the security strength of the HMAC-SHA1 DRBG to 128 bits. This strength defines the size of the seed required for the DRBG. Thus, the patch lowers the seeding requirement from 256 bits to 128 bits for HMAC-SHA1.

Signed-off-by Stephan Mueller <[email protected]>
---
 crypto/drbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
crypto/drbg.c +1 -1
index a76b3cb..84478cb 100644
@@ -184,7 +184,7 @@ static const struct drbg_core drbg_cores[] = {
 #endif /* CONFIG_CRYPTO_DRBG_HASH */
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
 	{
-		.flags = DRBG_HMAC | DRBG_STRENGTH256,
+		.flags = DRBG_HMAC | DRBG_STRENGTH128,
 		.statelen = 20, /* block length of cipher */
 		.max_addtllen = 35,
 		.max_bits = 19,
Signature · 1 line
1.9.3
maintainer
#7 lore raw
In reply to Stephan Mueller
Quoted 9 lines · Stephan Mueller

Hi,

This patchset superseeds the patch sets submitted with [1] and [2]. It rebases all non-applied patches to the current Herbert Xu's cryptodev-2.6 tree.

[1] https://lkml.org/lkml/2014/6/28/497
[2] https://lkml.org/lkml/2014/7/1/332

Quoted 7 lines

Stephan Mueller (6):
DRBG: cleanup of preprocessor macros
DRBG: Fix format string for debugging statements
DRBG: Call CTR DRBG DF function only once
DRBG: Select correct DRBG core for stdrng
DRBG: Mix a time stamp into DRBG state
DRBG: HMAC-SHA1 DRBG has crypto strength of 128 bits

All applied. Thanks Stephan!

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