Skip to content

Commit 9a09985

Browse files
samuel-lee-msftsamuel-lee-msft
committed
Merged PR 6732059: Fix SymCryptShortWeierstrassAddSideChannelUnsafe point equality check
+ Add test case reported by Intel + Tweak ECDSA KAT tests to enable tests with no hashing + Fix ECDSA KAT tests to fail when an expected successful verification fails (previously only failed on incorrect success) Related work items: #37272290
1 parent cc344e4 commit 9a09985

File tree

3 files changed

+97
-24
lines changed

3 files changed

+97
-24
lines changed

‎lib/ec_short_weierstrass.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,12 @@ SymCryptShortWeierstrassAddSideChannelUnsafe(
643643
SymCryptModMul( FMod, peX2, peT[0], peT[3], pbScratch, cbScratch ); /* T3 := X2 * Z1Z1 = U2 */
644644
SymCryptModSub( FMod, peT[3], peT[2], peT[5], pbScratch, cbScratch ); /* T5 := T3 - T2 = U2 - U1 = H */
645645

646-
if (SymCryptModElementIsZero( FMod, peT[5] ))
646+
SymCryptModMul( FMod, peY2, peT[1], peT[7], pbScratch, cbScratch ); /* T7 := Y2 * T1 = Y2*Z1*Z1Z1 = S2 */
647+
SymCryptModMul( FMod, peZ2, peT[6], peT[1], pbScratch, cbScratch ); /* T1 := Z2 * T6 = Z2*Z2Z2 */
648+
SymCryptModMul( FMod, peY1, peT[1], peT[1], pbScratch, cbScratch ); /* T1 := Y1 * T1 = Y1*Z2*Z2Z2 = S1 */
649+
SymCryptModSub( FMod, peT[7], peT[1], peT[7], pbScratch, cbScratch ); /* T7 := T7 - T1 = S2-S1 */
650+
651+
if (SymCryptModElementIsZero( FMod, peT[5] ) & SymCryptModElementIsZero( FMod, peT[7] ))
647652
{
648653
// Points are equal - run double on poSrc1
649654

@@ -698,10 +703,6 @@ SymCryptShortWeierstrassAddSideChannelUnsafe(
698703
SymCryptModSub( FMod, peT[4], peT[6], peT[4], pbScratch, cbScratch ); /* T4 := T4 - T6 = (Z1 + Z2)^2 - Z1Z1 - Z2Z2 */
699704
SymCryptModMul( FMod, peT[4], peT[5], peT[4], pbScratch, cbScratch ); /* T4 := T4 * T5 = ((Z1 + Z2)^2 - Z1Z1 - Z2Z2)*H = Z3 */
700705

701-
SymCryptModMul( FMod, peZ2, peT[6], peT[6], pbScratch, cbScratch ); /* T6 := Z2 * T6 = Z2*Z2Z2 */
702-
SymCryptModMul( FMod, peY1, peT[6], peT[6], pbScratch, cbScratch ); /* T6 := Y1 * T6 = Y1*Z2*Z2Z2 = S1 */
703-
SymCryptModMul( FMod, peY2, peT[1], peT[7], pbScratch, cbScratch ); /* T7 := Y2*Z1*Z1Z1 = S2 */
704-
SymCryptModSub( FMod, peT[7], peT[6], peT[7], pbScratch, cbScratch ); /* T7 := T7 - T6 = S2-S1 */
705706
SymCryptModAdd( FMod, peT[7], peT[7], peT[7], pbScratch, cbScratch ); /* T7 := T7 + T7 = 2*(S2-S1) = r */
706707

707708
SymCryptModAdd( FMod, peT[5], peT[5], peT[3], pbScratch, cbScratch ); /* T3 := T5 + T5 = 2*H */
@@ -716,7 +717,7 @@ SymCryptShortWeierstrassAddSideChannelUnsafe(
716717

717718
SymCryptModSub( FMod, peT[3], peT[2], peT[3], pbScratch, cbScratch ); /* T3 := T3 - T2 = V - X3 */
718719
SymCryptModMul( FMod, peT[3], peT[7], peT[3], pbScratch, cbScratch ); /* T3 := T3 * T7 = r*(V-X3) */
719-
SymCryptModMul( FMod, peT[6], peT[5], peT[6], pbScratch, cbScratch ); /* T6 := T6 * T5 = S1*J */
720+
SymCryptModMul( FMod, peT[1], peT[5], peT[6], pbScratch, cbScratch ); /* T6 := T1 * T5 = S1*J */
720721
SymCryptModAdd( FMod, peT[6], peT[6], peT[6], pbScratch, cbScratch ); /* T6 := T6 + T6 = 2*S1*J */
721722
SymCryptModSub( FMod, peT[3], peT[6], peT[3], pbScratch, cbScratch ); /* T3 := T6 - T3 = r*(V-X3) - 2*S1*J = Y3 */
722723

‎unittest/kat_ecdsa.dat‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4557,6 +4557,35 @@ R = 2bdbd8b0d759595662cc10b10236136ef6ce429641f68cf6480f472fcc77bc9f
45574557
S = 7e7df0c8b86f7db06caf1610166f7b9c4c75447f991d5aaf4dea720c25985c8c
45584558
Result = "P (0 )"
45594559

4560+
# [P-384,NONE]
4561+
4562+
curve = "nistP384"
4563+
hash = "NONE"
4564+
Msg = 5caf2df31c7bfd7a7d595b11203c3d921609c60776f35585b9bc15d70f50c510bddf97de488cb5fdf0f50b0bc6d07669
4565+
Qx = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4566+
Qy = 2205451118a64aa28d80ef59f90339e4f7d86e6697cec0077b5df73fb919c498bf42e87dfc5c3e78e9ff31b302c0d988
4567+
R = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4568+
S = 7fa840a80379a742615c166b1174253d05babb96ce53e3bf04a18cbdec8964db1f53b6bf174a8534c773a03e971bfe66
4569+
Result = "P (0 )"
4570+
4571+
curve = "nistP384"
4572+
hash = "NONE"
4573+
Msg = e4610aa9bbffbec3d0f082c0d7c52ac750637f992e0cb5447d38e84db89fdf95ffbd9e351bf05c465cd9812cf821d457
4574+
Qx = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4575+
Qy = 2205451118a64aa28d80ef59f90339e4f7d86e6697cec0077b5df73fb919c498bf42e87dfc5c3e78e9ff31b302c0d988
4576+
R = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4577+
S = 104614f27fd38830e9ef2abda56f357ccf372b0275818ce55924f9e352411fcb6a88501adb283178a623284d6a4f090c
4578+
Result = "P (0 )"
4579+
4580+
curve = "nistP384"
4581+
hash = "NONE"
4582+
Msg = e4610aa9bbffbec3d0f082c0d7c52ac750637f992e0cb5447d38e84db89fdf95ffbd9e351bf05c465cd9812cf821d457
4583+
Qx = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4584+
Qy = 2205451118a64aa28d80ef59f90339e4f7d86e6697cec0077b5df73fb919c498bf42e87dfc5c3e78e9ff31b302c0d988
4585+
R = 8abbcc65a6919460220063f2671c8862453a878556e9dabaa50d4354647bcd4adf9d5c9aa698611989b00d2dea9c98cb
4586+
S = 104614f27fd38830e9ef2abda56f357ccf372b0275818ce55924f9e352411fcb6a88501adb283178a623284d6a4f090d
4587+
Result = "F (0 )"
4588+
45604589
# [P-384,SHA-1]
45614590

45624591
curve = "nistP384"

‎unittest/lib/testEcc.cpp‎

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ SYMCRYPT_ECC_CURVES rgbInternalCurves[] = {
5353
////////////////////////////////////////////////////////////////////
5454

5555
#define SYMCRYPT_ECC_SHA1 "SHA1"
56+
#define SYMCRYPT_ECC_SHA224 "SHA224"
5657
#define SYMCRYPT_ECC_SHA256 "SHA256"
5758
#define SYMCRYPT_ECC_SHA384 "SHA384"
5859
#define SYMCRYPT_ECC_SHA512 "SHA512"
@@ -65,6 +66,7 @@ typedef struct _SYMCRYPT_ECC_HASH_ALGORITHMS {
6566

6667
SYMCRYPT_ECC_HASH_ALGORITHMS rgbHashAlgorithms[] = {
6768
{ SYMCRYPT_ECC_SHA1, SymCryptSha1Algorithm },
69+
{ SYMCRYPT_ECC_SHA224, NULL },
6870
{ SYMCRYPT_ECC_SHA256, SymCryptSha256Algorithm },
6971
{ SYMCRYPT_ECC_SHA384, SymCryptSha384Algorithm },
7072
{ SYMCRYPT_ECC_SHA512, SymCryptSha512Algorithm },
@@ -866,6 +868,8 @@ testEcdsaVerify(
866868
PSYMCRYPT_ECKEY pkPublic = NULL;
867869

868870
BYTE pbHashValue[SYMCRYPT_SHA512_RESULT_SIZE] = { 0 };
871+
PCBYTE pbDigest = NULL;
872+
UINT32 cbDigest = 0;
869873
BYTE pbSignature[2 * ((SYMCRYPT_BITSIZE_P521 + 7)/8)] = { 0 }; // big enough to hold any signature
870874
BYTE pbPublicKey[2 * ((SYMCRYPT_BITSIZE_P521 + 7)/8)] = { 0 }; // or the X,Y coordinates of a public key
871875

@@ -874,8 +878,16 @@ testEcdsaVerify(
874878
CHECK3( pkPublic!=NULL, "Failure to allocate public key for ECDSA record at line %lld", line );
875879

876880
// Hash the message
877-
CHECK3( SYMCRYPT_SHA512_RESULT_SIZE >= pHash->resultSize, "Hash result too big for ECDSA record at line %lld", line );
878-
SymCryptHash( pHash, pbMsg, cbMsg, pbHashValue, pHash->resultSize );
881+
if( pHash != NULL )
882+
{
883+
CHECK3( SYMCRYPT_SHA512_RESULT_SIZE >= pHash->resultSize, "Hash result too big for ECDSA record at line %lld", line );
884+
SymCryptHash( pHash, pbMsg, cbMsg, pbHashValue, pHash->resultSize );
885+
pbDigest = &pbHashValue[0];
886+
cbDigest = pHash->resultSize;
887+
} else {
888+
pbDigest = pbMsg;
889+
cbDigest = (UINT32) cbMsg;
890+
}
879891

880892
// Set the public key
881893
memcpy(pbPublicKey, pbQx, cbQx);
@@ -899,8 +911,8 @@ testEcdsaVerify(
899911
// Verify
900912
scError = SymCryptEcDsaVerify(
901913
pkPublic,
902-
pbHashValue,
903-
pHash->resultSize,
914+
pbDigest,
915+
cbDigest,
904916
pbSignature,
905917
cbR + cbS,
906918
SYMCRYPT_NUMBER_FORMAT_MSB_FIRST,
@@ -915,6 +927,10 @@ testEcdsaVerify(
915927
{
916928
CHECK3( scError == SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE, "Wrong EcDsaVerify result for ECDSA record at line %lld", line );
917929
}
930+
else
931+
{
932+
CHECK3( scError == SYMCRYPT_NO_ERROR, "Wrong EcDsaVerify result for ECDSA record at line %lld", line );
933+
}
918934

919935
SymCryptEckeyFree( pkPublic );
920936

@@ -954,6 +970,8 @@ testEcdsaSign(
954970
PSYMCRYPT_INT piK = NULL;
955971

956972
BYTE pbHashValue[SYMCRYPT_SHA512_RESULT_SIZE] = { 0 };
973+
PCBYTE pbDigest = NULL;
974+
UINT32 cbDigest = 0;
957975
BYTE pbSignature[2 * ((SYMCRYPT_BITSIZE_P521 + 7)/8)] = { 0 }; // big enough to hold any signature
958976

959977
// Allocate the private key and the random exponent K
@@ -963,6 +981,17 @@ testEcdsaSign(
963981
CHECK3( piK!=NULL, "Failure to allocate random exponent K for ECDSA record at line %lld", line );
964982

965983
// Hash the message
984+
if( pHash != NULL )
985+
{
986+
CHECK3( SYMCRYPT_SHA512_RESULT_SIZE >= pHash->resultSize, "Hash result too big for ECDSA record at line %lld", line );
987+
SymCryptHash( pHash, pbMsg, cbMsg, pbHashValue, pHash->resultSize );
988+
pbDigest = &pbHashValue[0];
989+
cbDigest = pHash->resultSize;
990+
} else {
991+
pbDigest = pbMsg;
992+
cbDigest = (UINT32) cbMsg;
993+
}
994+
CHECK( pHash != NULL, "Unsupported ")
966995
CHECK3( SYMCRYPT_SHA512_RESULT_SIZE >= pHash->resultSize, "Hash result too big for ECDSA record at line %lld", line );
967996
SymCryptHash( pHash, pbMsg, cbMsg, pbHashValue, pHash->resultSize );
968997

@@ -1000,8 +1029,8 @@ testEcdsaSign(
10001029
// Sign
10011030
scError = SymCryptEcDsaSignEx(
10021031
pkPrivate,
1003-
pbHashValue,
1004-
pHash->resultSize,
1032+
pbDigest,
1033+
cbDigest,
10051034
piK,
10061035
SYMCRYPT_NUMBER_FORMAT_MSB_FIRST,
10071036
0,
@@ -1248,14 +1277,21 @@ testEccEcdsaKats()
12481277
break;
12491278
}
12501279
}
1251-
if (!bHashFound)
1280+
1281+
if (bHashFound)
12521282
{
1253-
dprint( "Ecdsa record at line %lld is skipped due to unknown hash function.\n", line);
1254-
continue; // Skip this record if the hash algorithm is not in SymCrypt (e.g. SHA224)
1283+
pHash = rgbHashAlgorithms[i].pHash;
1284+
if( pHash == NULL )
1285+
{
1286+
dprint( "Ecdsa record at line %lld is skipped due to unsupported hash function (%s).\n", line, rgbHashAlgorithms[i].pszHashName);
1287+
continue; // Skip this record
1288+
}
1289+
}
1290+
else
1291+
{
1292+
dprint( "Assuming no hash function for unknown hash function.\n", line);
1293+
pHash = NULL;
12551294
}
1256-
1257-
pHash = rgbHashAlgorithms[i].pHash;
1258-
CHECK3( pHash != NULL, "NULL hash for ECDSA record at line %lld", line );
12591295

12601296
BString katMsg = katParseData( katItem, "msg" );
12611297
BString katQx = katParseData( katItem, "qx" );
@@ -1327,15 +1363,22 @@ testEccEcdsaKats()
13271363
break;
13281364
}
13291365
}
1330-
if (!bHashFound)
1366+
1367+
if (bHashFound)
1368+
{
1369+
pHash = rgbHashAlgorithms[i].pHash;
1370+
if( pHash == NULL )
1371+
{
1372+
dprint( "Ecdsa record at line %lld is skipped due to unsupported hash function (%s).\n", line, rgbHashAlgorithms[i].pszHashName);
1373+
continue; // Skip this record
1374+
}
1375+
}
1376+
else
13311377
{
1332-
dprint( "Ecdsa record at line %lld is skipped due to unknown hash function.\n", line);
1333-
continue; // Skip this record if the hash algorithm is not in SymCrypt (e.g. SHA224)
1378+
dprint( "Assuming no hash function for unknown hash function.\n", line);
1379+
pHash = NULL;
13341380
}
13351381

1336-
pHash = rgbHashAlgorithms[i].pHash;
1337-
CHECK3( pHash != NULL, "NULL hash for ECDSA record at line %lld", line );
1338-
13391382
BString katMsg = katParseData( katItem, "msg" );
13401383
BString katD = katParseData( katItem, "d" );
13411384
BString katQx = katParseData( katItem, "qx" );

0 commit comments

Comments
 (0)