<div dir="ltr"><div><div>Hi all,</div><div><br></div><div>I've been tasked with making
some modifications to OpenSSL 1.1.1 in order to bring it into compliance
with FIPS 140-2. One of the items on the to-do list was to implement
the required key agreement scheme assurances specified in NIST
SP.800-56Ar3 Section 9. This involves performing some validation on the
public key provided via the EVP_PKEY_derive() call.<br><br></div><div>To that end, I backported this patch which purports to implement the required validation in EC_KEY_check_key():</div><div><br>commit 5173cdde7d758824e6a07f2a6c6808b254602e11<br>Author: Shane Lontis <<a href="mailto:shane.lontis@oracle.com" target="_blank">shane.lontis@oracle.com</a>><br>Date: Sat Mar 23 13:12:08 2019 +1000<br><br> ec key validation checks updated<br> <br> Reviewed-by: Nicola Tuveri <<a href="mailto:nic.tuv@gmail.com" target="_blank">nic.tuv@gmail.com</a>><br> Reviewed-by: Matt Caswell <<a href="mailto:matt@openssl.org" target="_blank">matt@openssl.org</a>><br> (Merged from <a href="https://github.com/openssl/openssl/pull/8564" target="_blank">https://github.com/openssl/openssl/pull/8564</a>)</div><div><br></div>I then added a call to EC_KEY_check_key in pkey_ec_derive() to validate the public key, like so:<br><br><div style="margin-left:40px">diff --git a/crypto/ec/ec_pmeth.c b/crypto/ec/ec_pmeth.c<br>index 5bee031b92..84d8eb5d95 100644<br>--- a/crypto/ec/ec_pmeth.c<br>+++ b/crypto/ec/ec_pmeth.c<br>@@ -163,6 +163,14 @@ static int pkey_ec_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen)<br> <br> eckey = dctx->co_key ? dctx->co_key : ctx->pkey-><a href="http://pkey.ec">pkey.ec</a>;<br> <br>+ /*<br>+ * Check the validity of the received public key as required by NIST<br>+ * SP.800-56Ar3 Section 9<br>+ */<br>+ ret = EC_KEY_check_key(ctx->peerkey-><a href="http://pkey.ec">pkey.ec</a>);<br>+ if (ret <= 0)<br>+ return ret;<br>+<br> if (!key) {<br> const EC_GROUP *group;<br> group = EC_KEY_get0_group(eckey);</div><div style="margin-left:40px"><br></div>Adding
this check causes several unexpected unit test failures, which I was
hoping someone could help me with. The first category of failure seems
to be with TLS 1.3 tests that exercise the HelloRetryRequest (HRR)
functionality. Unfortunately, I'm not terribly familiar with the TLS
protocol, so my understanding here is limited. It seems like the unit
tests induce this HRR condition by calling SSL_set1_groups_list("P-256")
while providing an RSA private key. I'm not exactly sure of the effect
of this change, but here is an example test failure:<br><br><div style="margin-left:40px">ok 22 - test_early_data_skip<br> # Subtest: test_early_data_skip_hrr<br> 1..3<br> # ERROR: (bool) 'SSL_write_ex(clientssl, MSG2, strlen(MSG2), &written) == true' failed @ test/sslapitest.c:2620<br> # false<br> # 139755660280960:error:1010207B:elliptic curve routines:ec_key_simple_check_key:invalid private key:crypto/ec/ec_key.c:406:<br> # 139755660280960:error:1424E044:SSL routines:ssl_derive:internal error:ssl/s3_lib.c:4781:<br> not ok 1 - iteration 1<br></div></div><div dir="ltr"><div dir="ltr"><br><div>There are a couple of other tests that use SSL_set1_groups_list to do a similar thing and fail in a similar way.<br><br></div><div>Additionally,
there is another test in test_evp whose failure I don't quite
understand. The test involves calling EVP_PKEY_derive() with the
ALICE_zero_secp112r2 and BOB_zero_secp112r2_PUB keys from
test/recipes/30-test_evp_data/evppkey_ecc.txt. It appears to have been added by commit 5d92b853f6b875ba8d1a1b51b305f14df5adb8aa as a regression test for a change to the GFp ladder algorithm.<br><br></div>The test failure looks like this:<br><br></div><div dir="ltr"><div style="margin-left:40px"># Starting "zero x-coord regression tests" tests at line 4536<br># INFO: @ test/evp_test.c:2320<br># ../../test/recipes/30-test_evp_data/evppkey_ecc.txt:4670: Source of above error; unexpected error DERIVE_ERROR <br># 140441081306112:error:10102082:elliptic curve routines:ec_key_simple_check_key:wrong order:crypto/ec/ec_key.c:382:</div><div><br></div><div>My
question is: is there something invalid about adding this call to
EC_KEY_check_key() to pkey_ec_derive() or are these failures benign and
indications that the tests need to be changed? I'm particularly
concerned about the TLS 1.3 HRR tests as I want to make sure I haven't
somehow broken the TLS protocol.<br><br></div><div>FWIW, I see a similar check to the one I added in the DH shared secret derivation codepath.<br><br></div><div>Thank you for any insight you can bring to bear!<font color="#888888"><font color="#888888"><br></font></font></div></div></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div style="color:rgb(136,136,136);font-size:12.8000001907349px;word-wrap:break-word"><span style="border-collapse:separate;border-spacing:0px"><div style="word-wrap:break-word"><b style="font-size:14px;text-align:-webkit-auto"><font color="#0c59ab">Patrick Jakubowski</font></b><br></div></span></div><p style="margin:0px"><font color="#555555"><b>Member of Technical Staff</b></font><br style="font-family:arial,sans-serif;font-size:12.8000001907349px;color:rgb(136,136,136)"><b style="font-family:arial,sans-serif;font-size:12.8000001907349px;text-align:-webkit-auto;color:rgb(85,85,85)"><span style="font-size:14px">___________________________________</span></b><font style="font-family:arial,sans-serif;font-size:12.8000001907349px" color="#0c59ab"><span style="font-size:14px"><b><br></b></span></font><b style="font-family:arial,sans-serif;font-size:small;text-align:-webkit-auto;color:rgb(136,136,136)"><span style="font-size:14px"><font color="#0c59ab">Qumulo, Inc.</font></span></b><br style="font-family:arial,sans-serif;font-size:12.8000001907349px;color:rgb(136,136,136)"><i style="font-family:arial,sans-serif;font-size:11px;text-align:-webkit-auto;color:rgb(136,136,136);line-height:14px"><font color="#0c59ab"><b>World's First Data-Aware Scale-Out NAS</b></font></i></p><p style="margin:0px;font-size:12px;font-family:Helvetica"><span style="color:rgb(136,136,136);font-family:arial,sans-serif;text-align:-webkit-auto;font-size:small"><span style="text-align:-webkit-auto"><span style="font-size:11px;line-height:14px;text-align:-webkit-auto"><font color="#0061ff"><a href="https://twitter.com/qumulo?lang=en" style="color:rgb(17,85,204)" target="_blank">Twitter</a></font><font color="#2d71ff"> </font></span></span></span><span style="color:rgb(85,85,85);font-size:11px;line-height:14px;font-family:arial,sans-serif">/</span><span style="color:rgb(136,136,136);font-family:arial,sans-serif;text-align:-webkit-auto;font-size:small"><span style="text-align:-webkit-auto"><span style="font-size:11px;line-height:14px;text-align:-webkit-auto"><font color="#555555">//</font><font color="#606060"> </font><font color="#0061ff"><a href="https://www.linkedin.com/company/2533593?trk=tyah&trkInfo=idx%3A3-1-6%2CtarId%3A1426264780078%2Ctas%3Aqumul" style="color:rgb(17,85,204)" target="_blank">LinkedIn</a></font></span><font style="font-size:11px;line-height:14px" color="#2d71ff"> </font><font style="font-size:11px;line-height:14px" color="#555555">///</font><font style="font-size:11px;line-height:14px" color="#606060"> </font><span style="color:rgb(0,97,255);font-size:11px;line-height:14px"><a href="https://www.facebook.com/Qumulo" style="color:rgb(17,85,204)" target="_blank">Facebook</a></span></span></span><span style="font-family:arial,sans-serif;text-align:-webkit-auto;font-size:11px;line-height:14px;color:rgb(96,96,96)"> </span><font style="font-family:arial,sans-serif;text-align:-webkit-auto;font-size:11px;line-height:14px" color="#555555">///</font><font style="font-family:arial,sans-serif;text-align:-webkit-auto;font-size:11px;line-height:14px" color="#606060"> </font><font style="font-family:arial,sans-serif;text-align:-webkit-auto;font-size:11px;line-height:14px" color="#0061ff"><a href="https://www.youtube.com/channel/UCe6kSaQmqlCzpNd-tWIP9pQ/feed" style="color:rgb(17,85,204)" target="_blank">YouTube</a></font></p></div></div></div>