Always call setCurveParameters() after clearing keys

Also before generating new ones, if we don't call it, then isInitialized() will be false. Fixes #36
arekinath · Aug 6, 2020 · 5221da2 · 5221da2
1 parent 339e473
commit 5221da2
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 15 deletions.
diff --git a/src/net/cooperi/pivapplet/PivApplet.java b/src/net/cooperi/pivapplet/PivApplet.java
@@ -1118,6 +1118,11 @@ else if (key == (byte)0x81)
 			return;
 		}
 
+//#if PIV_SUPPORT_EC
+		final ECPrivateKey ecPriv;
+		final ECPublicKey ecPub;
+//#endif
+
 		switch (alg) {
 //#if PIV_SUPPORT_RSA
 		case PIV_ALG_RSA1024:
@@ -1141,9 +1146,8 @@ else if (key == (byte)0x81)
 				ISOException.throwIt(ISO7816.SW_WRONG_DATA);
 				return;
 			}
+
 			if (slot.asym == null || slot.asymAlg != alg) {
-				final ECPrivateKey ecPriv;
-				final ECPublicKey ecPub;
 				ecPriv = (ECPrivateKey)KeyBuilder.buildKey(
 				    KeyBuilder.TYPE_EC_FP_PRIVATE,
 				    (short)256, false);
@@ -1152,9 +1156,14 @@ else if (key == (byte)0x81)
 				    (short)256, false);
 				slot.asym = new KeyPair(
 				    (PublicKey)ecPub, (PrivateKey)ecPriv);
-				ECParams.setCurveParametersP256(ecPriv);
-				ECParams.setCurveParametersP256(ecPub);
+			} else {
+				ecPriv = (ECPrivateKey)slot.asym.getPrivate();
+				ecPub = (ECPublicKey)slot.asym.getPublic();
+				ecPriv.clearKey();
+				ecPub.clearKey();
 			}
+			ECParams.setCurveParametersP256(ecPriv);
+			ECParams.setCurveParametersP256(ecPub);
 			slot.asymAlg = alg;
 			break;
 		case PIV_ALG_ECCP384:
@@ -1163,8 +1172,6 @@ else if (key == (byte)0x81)
 				return;
 			}
 			if (slot.asym == null || slot.asymAlg != alg) {
-				final ECPrivateKey ecPriv;
-				final ECPublicKey ecPub;
 				ecPriv = (ECPrivateKey)KeyBuilder.buildKey(
 				    KeyBuilder.TYPE_EC_FP_PRIVATE,
 				    (short)384, false);
@@ -1173,9 +1180,14 @@ else if (key == (byte)0x81)
 				    (short)384, false);
 				slot.asym = new KeyPair(
 				    (PublicKey)ecPub, (PrivateKey)ecPriv);
-				ECParams.setCurveParametersP384(ecPriv);
-				ECParams.setCurveParametersP384(ecPub);
+			} else {
+				ecPriv = (ECPrivateKey)slot.asym.getPrivate();
+				ecPub = (ECPublicKey)slot.asym.getPublic();
+				ecPriv.clearKey();
+				ecPub.clearKey();
 			}
+			ECParams.setCurveParametersP384(ecPriv);
+			ECParams.setCurveParametersP384(ecPub);
 			slot.asymAlg = alg;
 			break;
 //#endif
@@ -1335,8 +1347,6 @@ else if (key == (byte)0x81)
 				    (short)256, false);
 				slot.asym = new KeyPair(
 				    (PublicKey)ecPub, (PrivateKey)ecPriv);
-				ECParams.setCurveParametersP256(ecPriv);
-				ECParams.setCurveParametersP256(ecPub);
 			}
 			slot.asymAlg = alg;
 			break;
@@ -1357,8 +1367,6 @@ else if (key == (byte)0x81)
 				    (short)384, false);
 				slot.asym = new KeyPair(
 				    (PublicKey)ecPub, (PrivateKey)ecPriv);
-				ECParams.setCurveParametersP384(ecPriv);
-				ECParams.setCurveParametersP384(ecPub);
 			}
 			slot.asymAlg = alg;
 			break;
@@ -1488,6 +1496,14 @@ else if (key == (byte)0x81)
 			epubk.clearKey();
 			eprivk.clearKey();
 
+			if (alg == PIV_ALG_ECCP256) {
+				ECParams.setCurveParametersP256(eprivk);
+				ECParams.setCurveParametersP256(epubk);
+			} else if (alg == PIV_ALG_ECCP384) {
+				ECParams.setCurveParametersP384(eprivk);
+				ECParams.setCurveParametersP384(epubk);
+			}
+
 			while (!tlv.atEnd()) {
 				tag = tlv.readTag();
 				final short tlen = tlv.tagLength();
@@ -1499,7 +1515,7 @@ else if (key == (byte)0x81)
 					tlv.end();
 					break;
 				case (byte)0xaa:
-					if (tlv.tagLength() != 1) {
+					if (tlen != 1) {
 						tlv.abort();
 						ISOException.throwIt(
 						    ISO7816.SW_WRONG_DATA);
@@ -1534,7 +1550,7 @@ else if (key == (byte)0x81)
 					}
 					break;
 				case (byte)0xab:
-					if (tlv.tagLength() != 1) {
+					if (tlen != 1) {
 						tlv.abort();
 						ISOException.throwIt(
 						    ISO7816.SW_WRONG_DATA);

diff --git a/test/simulator-tests b/test/simulator-tests
@@ -365,7 +365,22 @@ class TestPivyBasic < Minitest::Test
 		assert(@cert.verify(@key.public_key))
 	end
 
-	def test_06_reset_admin
+	def test_06_import_ec
+		@key = OpenSSL::PKey::EC.generate('prime256v1')
+		f = File.new("#{TestDir}/testkey2.pem", 'w')
+		f.write(@key.to_pem)
+		f.close()
+
+		out = `pivy-tool -g #{$guid} -P #{DefaultPin} import 82 < #{TestDir}/testkey2.pem`
+		assert_equal(0, $?)
+
+		out = `pivy-tool -g #{$guid} cert 82`
+		assert_equal(0, $?)
+		@cert = OpenSSL::X509::Certificate.new(out)
+		assert(@cert.verify(@key))
+	end
+
+	def test_07_reset_admin
 		# pivy-tool set-admin doesn't yet get the pinfo admin key in
 		# 0.5.0 :( change this after next ver?
 		out = ykpivtool(:verify_pin, :read_object, '--id 0x5FC109')