Conversation
slontis
force-pushed
the
selftest_drbg
branch
from
9e2c811 to
414ee09
Compare
slontis
added
approval: review pending
|
Modified to use the new DRBG_RAND_(set/get)_callback_data() call. |
slontis
force-pushed
the
selftest_drbg
branch
from
414ee09 to
59bfdfc
Compare
|
travis error is a timeout.. |
|
ping |
|
@slontis I am having problems setting up the DRBG selftests in a way that I can run them in a debugger. Maybe you can help me? Here's what I tried: I commented out the Then I added a I see that the selftest gets invoked in command 3 However, when I subsequently try to run command 3 in the debugger like this the self tests does not get executed. What am I missing? Also, I have two more related questions:
fipshack.diffdiff --git a/Configure b/Configure
index 2dd0520d3d..be78d70169 100755
--- a/Configure
+++ b/Configure
@@ -833,7 +833,7 @@ while (@argvcopy)
{ $config{processor}=386; }
elsif (/^fips$/)
{
- die "FIPS mode not supported\n";
+# die "FIPS mode not supported\n";
}
elsif (/^rsaref$/)
{
diff --git a/providers/fips/self_test_kats.c b/providers/fips/self_test_kats.c
index 81c237954e..69a798a0d6 100644
--- a/providers/fips/self_test_kats.c
+++ b/providers/fips/self_test_kats.c
@@ -235,6 +235,9 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
OSSL_PARAM_END, OSSL_PARAM_END, OSSL_PARAM_END
};
+ fprintf(stderr, "SELF_TEST_DRBG\n");
+
+
SELF_TEST_EVENT_onbegin(event, OSSL_SELF_TEST_TYPE_DRBG, t->desc);
if (strcmp(t->desc, OSSL_SELF_TEST_DESC_DRBG_HMAC) == 0)
diff --git a/util/opensslwrap.sh b/util/opensslwrap.sh
index 7a38830270..3be8802f51 100755
--- a/util/opensslwrap.sh
+++ b/util/opensslwrap.sh
@@ -10,6 +10,10 @@ if [ -d "${HERE}../providers" -a "x$OPENSSL_MODULES" = "x" ]; then
OPENSSL_MODULES="${HERE}../providers"; export OPENSSL_MODULES
fi
+echo OPENSSL_MODULES=$OPENSSL_MODULES
+echo OPENSSL_ENGINES=$OPENSSL_ENGINES
+
+
if [ -x "${OPENSSL}.exe" ]; then
# The original reason for this script existence is to work around
# certain caveats in run-time linker behaviour. On Windows platformsfipstest.log |
|
I just run it like this when I am testing..
:> break SELF_TEST_kats |
|
The self test always runs during the init().. I am working on being able to call it via the providers get_params().. But this only runs after the init() has been called - so that all the setup is stored and can be reused for running the kats.. |
You don't need to specify "fips" on the config line at all. That's a legacy option from the 2.0 module. |
Ok, that's what I remembered from the last time I asked you this question. But the fact that the test refused to run without the fips option confused me: Also, the Error message is very confusing: You are telling the users that you are not supporting FIPS mode. IMHO the error should only be a warning and reformulated, e.g., as follows: |
Right...that just looks wrong.
Oh...that needs updating. Its inherited from 1.1.1 where that is true. Its not been changed for 3.0. |
|
ping |
|
Could you please provide a reference (link) to the location where you got the test vectors from? |
|
@slontis @paulidale I haven't forgotten about the TODO you gave me, but I'm still pondering about the best way to do it. My first reflex was to hook the uninstantiate callback of the RAND_DRBG_METHOD but it feels a bit hacky and I would like to avoid hooking. So I looked for alternative ways to do it. The problem starts in This was added by me at some time and was necessary to make DRBG error recovery work. Because otherwise the DRBG would completely forget its type and reinstantiation would be impossible. However, no secret state information is restored, so I wondered whether it would be allowed to just separate the public 'administrative' information from the secret 'working state' and prove only that the latter is zeroed properly. Indeed, NIST.SP800.90Ar1 sees the secret DRBG state as part of the Working State, which is a subset of the Internal State. NIST.IR.8019 makes it a little bit more precise: Unfortunately, it seems that nevertheless FIPS insists that the entire Internal State is erased, including the administrative information, right? Or has the interpretation of the FIPS standard been relaxed in the mean time? (Side Note: If the entire internal state of the DRBG must be zeroed then it is actually questionable why the border around the internal state can be drawn at the Maybe there is a better alternative to ensure that at least What's your opinion, which path should I take?
|
|
I think a good argument could be made for only zeroizing the secret state. We'd need to run this past the lab for confirmation. Nonetheless, postponing the init call until later would be the best (i.e. safest) path forward I think. This all might need revisiting in light of fetchable rand which will impact the RNG architecture :( |
|
See #11111. |
|
Some of the self tests here use From 012dc8380b90e061cc6cf9f5b6e69b3a3691b29c Mon Sep 17 00:00:00 2001
From: "Dr. Matthias St. Pierre" <[email protected]>
Date: Mon, 17 Feb 2020 23:28:53 +0100
Subject: [PATCH] fixup! Add DRBG self tests
---
providers/fips/self_test_kats.c | 53 +++++++++++++++++----------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/providers/fips/self_test_kats.c b/providers/fips/self_test_kats.c
index 81c237954e..54e6fcfeb2 100644
--- a/providers/fips/self_test_kats.c
+++ b/providers/fips/self_test_kats.c
@@ -99,43 +99,43 @@ static int self_test_cipher(const ST_KAT_CIPHER *t, OSSL_ST_EVENT *event,
ctx = EVP_CIPHER_CTX_new();
if (ctx == NULL)
- goto end;
+ goto err;
cipher = EVP_CIPHER_fetch(libctx, t->base.algorithm, "");
if (cipher == NULL)
- goto end;
+ goto err;
/* Encrypt plain text message */
if (!cipher_init(ctx, cipher, t, encrypt)
|| !EVP_CipherUpdate(ctx, ct_buf, &len, t->base.pt, t->base.pt_len)
|| !EVP_CipherFinal_ex(ctx, ct_buf + len, &ct_len))
- goto end;
+ goto err;
SELF_TEST_EVENT_oncorrupt_byte(event, ct_buf);
ct_len += len;
if (ct_len != (int)t->base.expected_len
|| memcmp(t->base.expected, ct_buf, ct_len) != 0)
- goto end;
+ goto err;
if (t->tag != NULL) {
unsigned char tag[16] = { 0 };
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, t->tag_len, tag)
|| memcmp(tag, t->tag, t->tag_len) != 0)
- goto end;
+ goto err;
}
if (!(cipher_init(ctx, cipher, t, !encrypt)
&& EVP_CipherUpdate(ctx, pt_buf, &len, ct_buf, ct_len)
&& EVP_CipherFinal_ex(ctx, pt_buf + len, &pt_len)))
- goto end;
+ goto err;
pt_len += len;
if (pt_len != (int)t->base.pt_len
|| memcmp(pt_buf, t->base.pt, pt_len) != 0)
- goto end;
+ goto err;
ret = 1;
-end:
+err:
EVP_CIPHER_free(cipher);
EVP_CIPHER_CTX_free(ctx);
SELF_TEST_EVENT_onend(event, ret);
@@ -163,33 +163,33 @@ static int self_test_kdf(const ST_KAT_KDF *t, OSSL_ST_EVENT *event,
kdf = EVP_KDF_fetch(libctx, t->algorithm, "");
ctx = EVP_KDF_CTX_new(kdf);
if (ctx == NULL)
- goto end;
+ goto err;
settables = EVP_KDF_settable_ctx_params(kdf);
for (i = 0; t->ctrls[i].name != NULL; ++i) {
if (!ossl_assert(i < (numparams - 1)))
- goto end;
+ goto err;
if (!OSSL_PARAM_allocate_from_text(¶ms[i], settables,
t->ctrls[i].name,
t->ctrls[i].value,
strlen(t->ctrls[i].value)))
- goto end;
+ goto err;
}
if (!EVP_KDF_CTX_set_params(ctx, params))
- goto end;
+ goto err;
if (t->expected_len > sizeof(out))
- goto end;
+ goto err;
if (EVP_KDF_derive(ctx, out, t->expected_len) <= 0)
- goto end;
+ goto err;
SELF_TEST_EVENT_oncorrupt_byte(event, out);
if (memcmp(out, t->expected, t->expected_len) != 0)
- goto end;
+ goto err;
ret = 1;
-end:
+err:
for (i = 0; params[i].key != NULL; ++i)
OPENSSL_free(params[i].data);
EVP_KDF_free(kdf);
@@ -242,14 +242,14 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
drbg = RAND_DRBG_new_ex(libctx, t->nid, flags, NULL);
if (drbg == NULL)
- goto end;
+ goto err;
if (!RAND_DRBG_set_callback_data(drbg, drbg_params))
- goto end;
+ goto err;
if (!RAND_DRBG_set_callbacks(drbg, drbg_kat_entropy_cb, NULL,
drbg_kat_nonce_cb, NULL))
- goto end;
+ goto err;
drbg_params[0] =
OSSL_PARAM_construct_octet_string(DRBG_PARAM_ENTROPY,
@@ -259,7 +259,7 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
(void *)t->nonce, t->noncelen);
if (!RAND_DRBG_instantiate(drbg, t->persstr, t->persstrlen))
- goto end;
+ goto err;
drbg_params[0] =
OSSL_PARAM_construct_octet_string(DRBG_PARAM_ENTROPY,
@@ -268,7 +268,7 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
if (!RAND_DRBG_generate(drbg, out, t->expectedlen, prediction_resistance,
t->entropyaddin1, t->entropyaddin1len))
- goto end;
+ goto err;
drbg_params[0] =
OSSL_PARAM_construct_octet_string(DRBG_PARAM_ENTROPY,
@@ -277,15 +277,16 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
/* This calls RAND_DRBG_reseed() internally when prediction_resistance = 1 */
if (!RAND_DRBG_generate(drbg, out, t->expectedlen, prediction_resistance,
t->entropyaddin2, t->entropyaddin2len))
- goto end;
+ goto err;
SELF_TEST_EVENT_oncorrupt_byte(event, out);
if (memcmp(out, t->expected, t->expectedlen) != 0)
- goto end;
+ goto err;
if (!RAND_DRBG_uninstantiate(drbg))
- goto end;
+ goto err;
+
/*
* TODO(3.0) : Check that the DRBG data has been zeroed after
* RAND_DRBG_uninstantiate. Its a bit hard currently to do this when
@@ -298,11 +299,11 @@ static int self_test_drbg(const ST_KAT_DRBG *t, OSSL_ST_EVENT *event,
for (i = 0; i < sz; ++i)
if (*p++ != 0)
- goto end;
+ goto err;
}
#endif
ret = 1;
-end:
+err:
RAND_DRBG_free(drbg);
SELF_TEST_EVENT_onend(event, ret);
return ret;
--
2.24.1.1.gb6d4d82bd5.dirty |
|
I hope I find to review this some time today. |
mspncp
left a comment
mspncp
left a comment
There was a problem hiding this comment.
I did a sample check of the test vectors and added some comments which makes it easier to find the original vectors.
Since the tests succeed, all seems to be fine. But do you think that one check per DRBG type suffices?
Anyway, approved with the comments added.
mspncp
added
approval: done
openssl-machine
added
the
hold: cla required
slontis
removed
the
hold: cla required
slontis
force-pushed
the
selftest_drbg
branch
from
12dc615 to
8b30d23
Compare
|
Well that went bad.. I have partially merged your changes manually instead of clicking on the button here and getting CLA problems... |
Yes this conforms to what is minimally required for the self tests. |
Could you reconfirm approval .. |
Just in case your interested to know the reason: Your CLA problem is very likely caused by the fact that you configured your GitHub account to hide your primary email address. That's why GitHub committed the suggestions you accepted using the stealth address [email protected]. |
|
Travis error is the well known timeout. |
mspncp
added
approval: ready to merge
Reviewed-by: Matthias St. Pierre <[email protected]> (Merged from #11010)
5 participants
Checklist