On Mon, 2019-05-13 at 08:44 -0400, Simo Sorce wrote:
> On Sat, 2019-05-11 at 10:00 +0200, Niels Möller wrote:
> > Simo Sorce <[email protected]> writes:
> > 
> > > While reviewing FIPS requirements for public key checks in Ephemeral
> > > Diffie-Hellman key exchanges it came out that FIPS requires checks that
> > > the public key point is not the (0, 0) coordinate and nettle is not
> > > doing it (only checks that neither point is negative.
> > 
> > ecc_point_set also checks that the point is on the curve, i.e.,
> > satisfies the curve equation. That should rule out (0, 0), except if we
> > have some curve with constant term b == 0, which I don't think makes
> > sense.
> 
> Ah you are right the later check would catch it.
> I was just following the checks FIPS explicitly requires in order and
> didn't think about that ...
> 
> > Not sure how FIPS requirements are formulated, but maybe it would be
> > better to add a test case to check that ecc_point_set rejects (0,0) ?
> 
> Yes, I will drop my patch and add a test case.

Attached find patch that adds points checks to the ECDH test case.
Let me know if that's ok or if you prefer a whole new test.

Simo.

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

From a720da816f238e2a51747ab3df22715d8f134453 Mon Sep 17 00:00:00 2001
From: Simo Sorce <[email protected]>
Date: Mon, 13 May 2019 15:24:56 -0400
Subject: [PATCH] Add tests that exercise public key checks for ECDH

When performing ECDH the peer provided public key needs to be checked
for validity. FIPS requires basic tests be performed to insure the
provided points are in fact on the selected curve. Those checks already
exists in the ecc_point_set() function.
Add an explicit test that checks the boundaries so that any regression
in checks will be caught.

Signed-off-by: Simo Sorce <[email protected]>
---
 testsuite/ecdh-test.c | 61 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/testsuite/ecdh-test.c b/testsuite/ecdh-test.c
index 2bfffd68..9a029c61 100644
--- a/testsuite/ecdh-test.c
+++ b/testsuite/ecdh-test.c
@@ -31,20 +31,30 @@
 
 #include "testutils.h"
 
-static void
-set_point (struct ecc_point *p,
-	   const char *x, const char *y)
+static int
+ret_set_point (struct ecc_point *p,
+               const char *x, const char *y)
 {
   mpz_t X, Y;
+  int ret;
+
   mpz_init_set_str (X, x, 0);
   mpz_init_set_str (Y, y, 0);
-  if (!ecc_point_set (p, X, Y))
-    die ("Test point not on curve!\n");
+  ret = ecc_point_set (p, X, Y);
 
   mpz_clear (X);
   mpz_clear (Y);
+  return ret;
+}
+
+static void
+set_point (struct ecc_point *p,
+	   const char *x, const char *y)
+{
+  if (!ret_set_point (p, x, y))
+    die ("Test point not on curve!\n");
 }
-  
+
 static void
 set_scalar (struct ecc_scalar *s,
 	    const char *x)
@@ -135,9 +145,48 @@ test_dh (const char *name, const struct ecc_curve *ecc,
   ecc_point_clear (&T);  
 }
 
+static void
+test_public_key (const char *label, const struct ecc_curve *ecc,
+                 const char *x, const char *y, int expect_success)
+{
+  struct ecc_point P;
+  int ret;
+
+  ecc_point_init (&P, ecc);
+  ret = ret_set_point (&P, x, y);
+
+  if (!ret && expect_success)
+    die ("Test point '%s' not on curve!\n", label);
+
+  if (ret && !expect_success)
+    die ("Expected failure to set point '%s'!", label);
+
+  ecc_point_clear (&P);
+}
+
 void
 test_main(void)
 {
+  test_public_key ("(0,0) with secp-192r1", &_nettle_secp_192r1, "0", "0", 0);
+  test_public_key (
+    "(P,0) with secp-192r1", &_nettle_secp_192r1,
+    "6277101735386680763835789423207666416083908700390324961279",
+    "0", 0);
+  test_public_key (
+    "(0,P) with secp-192r1", &_nettle_secp_192r1, "0",
+    "6277101735386680763835789423207666416083908700390324961279",
+    0);
+  test_public_key (
+    "(P,P) with secp-192r1", &_nettle_secp_192r1,
+    "6277101735386680763835789423207666416083908700390324961279",
+    "6277101735386680763835789423207666416083908700390324961279",
+    0);
+  test_public_key ("(1,2) with secp-192r1", &_nettle_secp_192r1, "1", "2", 0);
+  test_public_key ("(X,Y) with secp-192r1", &_nettle_secp_192r1,
+    "1050363442265225480786760666329560655512990381040021438562",
+    "5298249600854377235107392014200406283816103564916230704184",
+    1);
+
   test_dh ("secp-192r1", &_nettle_secp_192r1,
 	   "3406157206141798348095184987208239421004566462391397236532",
 	   "1050363442265225480786760666329560655512990381040021438562",
-- 
2.21.0

_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Reply via email to