Bugzilla Bug 257693: set the correct error code in EC_ValidatePublicKey.

Documented the return values of ECPoint_validate.  Have the tests compare
the return value of ECPoint_validate with MP_NO for negative test cases.
r=doublas.stebila.
Modified Files:
	ec.c ecl/ecl.h ecl/tests/ec2_test.c ecl/tests/ecp_test.c

4 files changed, 35 insertions, 9 deletions

diff --git a/security/nss/lib/freebl/ec.c b/security/nss/lib/freebl/ec.c
index 54c13eb86..84e1a1bf6 100644
--- a/security/nss/lib/freebl/ec.c
+++ b/security/nss/lib/freebl/ec.c
@@ -408,9 +408,9 @@ EC_ValidatePublicKey(ECParams *ecParams, SECItem *publicValue)
 	PORT_SetError(SEC_ERROR_UNSUPPORTED_EC_POINT_FORM);
 	return SECFailure;
     } else if (publicValue->len != (2 * len + 1)) {
-	PORT_SetError(SEC_ERROR_INPUT_LEN);
+	PORT_SetError(SEC_ERROR_BAD_KEY);
 	return SECFailure;
-    };
+    }
 
     MP_DIGITS(&Px) = 0;
     MP_DIGITS(&Py) = 0;
@@ -423,11 +423,34 @@ EC_ValidatePublicKey(ECParams *ecParams, SECItem *publicValue)
 
     /* construct from named params */
     group = ECGroup_fromName(ecParams->name);
-    if (group == NULL)
+    if (group == NULL) {
+	/*
+	 * ECGroup_fromName fails if ecParams->name is not a valid
+	 * ECCurveName value, or if we run out of memory, or perhaps
+	 * for other reasons.  Unfortunately if ecParams->name is a
+	 * valid ECCurveName value, we don't know what the right error
+	 * code should be because ECGroup_fromName doesn't return an
+	 * error code to the caller.  Set err to MP_UNDEF because
+	 * that's what ECGroup_fromName uses internally.
+	 */
+	if ((ecParams->name <= ECCurve_noName) ||
+	    (ecParams->name >= ECCurve_pastLastCurve)) {
+	    err = MP_BADARG;
+	} else {
+	    err = MP_UNDEF;
+	}
 	goto cleanup;
+    }
 
     /* validate public point */
-    CHECK_MPI_OK( ECPoint_validate(group, &Px, &Py) );
+    if ((err = ECPoint_validate(group, &Px, &Py)) < MP_YES) {
+	if (err == MP_NO) {
+	    PORT_SetError(SEC_ERROR_BAD_KEY);
+	    rv = SECFailure;
+	    err = MP_OKAY;  /* don't change the error code */
+	}
+	goto cleanup;
+    }
 
     rv = SECSuccess;
 
diff --git a/security/nss/lib/freebl/ecl/ecl.h b/security/nss/lib/freebl/ecl/ecl.h
index db297281a..dbd86c253 100644
--- a/security/nss/lib/freebl/ecl/ecl.h
+++ b/security/nss/lib/freebl/ecl/ecl.h
@@ -81,7 +81,10 @@ mp_err ECPoints_mul(const ECGroup *group, const mp_int *k1,
 					const mp_int *k2, const mp_int *px, const mp_int *py,
 					mp_int *qx, mp_int *qy);
 
-/* Validates an EC public key as described in Section 5.2.2 of X9.62. */
+/* Validates an EC public key as described in Section 5.2.2 of X9.62.
+ * Returns MP_YES if the public key is valid, MP_NO if the public key
+ * is invalid, or an error code if the validation could not be
+ * performed. */
 mp_err ECPoint_validate(const ECGroup *group, const mp_int *px, const 
 					mp_int *py);
 
diff --git a/security/nss/lib/freebl/ecl/tests/ec2_test.c b/security/nss/lib/freebl/ecl/tests/ec2_test.c
index 3ce5878f9..e82b47da0 100644
--- a/security/nss/lib/freebl/ecl/tests/ec2_test.c
+++ b/security/nss/lib/freebl/ecl/tests/ec2_test.c
@@ -349,13 +349,13 @@ ectest_curve_GF2m(ECGroup *group, int ectestPrint, int ectestTime,
 	}
 
 	/* test validate_point function */
-	if (ECPoint_validate(group, &gx, &gy) != 0) {
+	if (ECPoint_validate(group, &gx, &gy) != MP_YES) {
 		printf("  Error: validate point on base point failed.\n");
 		res = MP_NO;
 		goto CLEANUP;
 	}
 	MP_CHECKOK(mp_add_d(&gy, 1, &ry));
-	if (ECPoint_validate(group, &gx, &ry) == 0) {
+	if (ECPoint_validate(group, &gx, &ry) != MP_NO) {
 		printf("  Error: validate point on invalid point passed.\n");
 		res = MP_NO;
 		goto CLEANUP;
diff --git a/security/nss/lib/freebl/ecl/tests/ecp_test.c b/security/nss/lib/freebl/ecl/tests/ecp_test.c
index 4e2047969..d7ce299ec 100644
--- a/security/nss/lib/freebl/ecl/tests/ecp_test.c
+++ b/security/nss/lib/freebl/ecl/tests/ecp_test.c
@@ -311,13 +311,13 @@ ectest_curve_GFp(ECGroup *group, int ectestPrint, int ectestTime,
 	}
 
 	/* test validate_point function */
-	if (ECPoint_validate(group, &gx, &gy) != 0) {
+	if (ECPoint_validate(group, &gx, &gy) != MP_YES) {
 		printf("  Error: validate point on base point failed.\n");
 		res = MP_NO;
 		goto CLEANUP;
 	}
 	MP_CHECKOK(mp_add_d(&gy, 1, &ry));
-	if (ECPoint_validate(group, &gx, &ry) == 0) {
+	if (ECPoint_validate(group, &gx, &ry) != MP_NO) {
 		printf("  Error: validate point on invalid point passed.\n");
 		res = MP_NO;
 		goto CLEANUP;