[Opencryptoki-tech] [PATCH] testcases: fix wrong testcase and ber en/decoding for integers
Brought to you by:
ebarretto
Menu
▾
▴
[Opencryptoki-tech] [PATCH] testcases: fix wrong testcase and ber en/decoding for integers
|
From: Harald F. <fr...@li...> - 2017-04-03 18:40:05
|
The ber encoding/decoding did not correctly append (on encode)
and remove (on decode) leading 0x00 to indicate unsigned values.
This has been fixed. The tok_rsa testcase even tolerated wrong
byte sizes of the modulus part of an RSA key (caused by wrong
decoding of an ber encoded big integer).
However, all this only affects the EP11 token. This is the only
caller of the ber en/decode stuff within the opencryptoki
common code.
Signed-off-by: Harald Freudenberger <fr...@li...>
---
testcases/misc_tests/tok_rsa.c | 140 +++++++++++++++++++++++++++++++----------
usr/lib/pkcs11/common/asn1.c | 106 +++++++++++++++++++++----------
2 files changed, 180 insertions(+), 66 deletions(-)
diff --git a/testcases/misc_tests/tok_rsa.c b/testcases/misc_tests/tok_rsa.c
index 29d1ca8..f44fd3d 100644
--- a/testcases/misc_tests/tok_rsa.c
+++ b/testcases/misc_tests/tok_rsa.c
@@ -18,7 +18,49 @@
int do_GetFunctionList(void);
CK_FUNCTION_LIST *funcs;
-CK_SLOT_ID SLOT_ID;
+
+
+CK_RV do_Cleanup(CK_SESSION_HANDLE sess)
+{
+ CK_RV rv;
+ CK_ULONG count;
+ CK_OBJECT_HANDLE handle;
+ CK_CHAR label[128];
+ CK_ATTRIBUTE tlabel = { CKA_LABEL, label, sizeof(label) };
+
+ rv = funcs->C_FindObjectsInit(sess, NULL, 0);
+ if (rv != CKR_OK) {
+ show_error(" C_FindObjectsInit #1", rv );
+ return rv;
+ }
+
+ while (1) {
+ rv = funcs->C_FindObjects(sess, &handle, 1, &count);
+ if (rv != CKR_OK) {
+ show_error(" C_FindObjects #1", rv );
+ return rv;
+ }
+ if (count < 1) break;
+ rv = funcs->C_GetAttributeValue(sess, handle, &tlabel, 1);
+ if (rv != CKR_OK)
+ continue;
+ if (strncmp(label, "XXX DELETE ME", 13) == 0) {
+ rv = funcs->C_DestroyObject(sess, handle);
+ if (rv != CKR_OK) {
+ show_error(" C_DestroyObject", rv );
+ }
+ }
+ }
+
+ rv = funcs->C_FindObjectsFinal(sess);
+ if (rv != CKR_OK) {
+ show_error(" C_FindObjectsFinal #1", rv );
+ return rv;
+ }
+
+ return rv;
+}
+
CK_RV
do_VerifyTokenRSAKeyPair(CK_SESSION_HANDLE sess, CK_BYTE *label, CK_ULONG bits)
@@ -81,9 +123,9 @@ do_VerifyTokenRSAKeyPair(CK_SESSION_HANDLE sess, CK_BYTE *label, CK_ULONG bits)
}
/* The public exponent is element 0 and modulus is element 1 */
- if (pub_attrs[0].ulValueLen > 514 || pub_attrs[1].ulValueLen > 514) {
- PRINT_ERR("e_size (%lu) or n_size (%lu) too big!",
- pub_attrs[0].ulValueLen, pub_attrs[1].ulValueLen);
+ if (pub_attrs[0].ulValueLen > (bits/8) || pub_attrs[1].ulValueLen > (bits/8)) {
+ PRINT_ERR("RSA public key '%s' e_size (%lu) or n_size (%lu) too big!",
+ label, pub_attrs[0].ulValueLen, pub_attrs[1].ulValueLen);
return CKR_FUNCTION_FAILED;
}
@@ -171,28 +213,23 @@ do_GenerateTokenRSAKeyPair(CK_SESSION_HANDLE sess, CK_BYTE *label, CK_ULONG bits
}
-//
-//
- int
-main( int argc, char **argv )
+int main( int argc, char **argv )
{
CK_C_INITIALIZE_ARGS cinit_args;
- int i, bits;
- CK_RV rv;
- SLOT_ID = 0;
+ int i, bits, ec = 0;
+ CK_RV rv;
CK_BYTE user_pin[128];
CK_ULONG user_pin_len;
- CK_SLOT_ID slot_id;
+ CK_SLOT_ID slot_id = 0;
CK_SESSION_HANDLE session;
CK_FLAGS flags;
CK_MECHANISM_INFO rsakeygeninfo;
CK_BYTE label[256];
-
for (i=1; i < argc; i++) {
if (strcmp(argv[i], "-slot") == 0) {
++i;
- SLOT_ID = atoi(argv[i]);
+ slot_id = atoi(argv[i]);
}
if (strcmp(argv[i], "-h") == 0) {
@@ -204,14 +241,13 @@ main( int argc, char **argv )
}
}
- printf("Using slot #%lu...\n\n", SLOT_ID );
-
- slot_id = SLOT_ID;
+ printf("Using slot #%lu...\n\n", slot_id );
rv = do_GetFunctionList();
if (rv != TRUE) {
show_error("do_GetFunctionList", rv);
- return -1;
+ ec++;
+ goto out;
}
memset( &cinit_args, 0x0, sizeof(cinit_args) );
@@ -221,7 +257,8 @@ main( int argc, char **argv )
if ((rv = funcs->C_Initialize( &cinit_args ))) {
show_error("C_Initialize", rv);
- return -1;
+ ec++;
+ goto out;
}
if (get_user_pin(user_pin))
@@ -232,19 +269,29 @@ main( int argc, char **argv )
rv = funcs->C_OpenSession( slot_id, flags, NULL, NULL, &session );
if (rv != CKR_OK) {
show_error(" C_OpenSession #1", rv );
- return rv;
+ ec++;
+ goto finalize;
}
rv = funcs->C_Login( session, CKU_USER, user_pin, user_pin_len );
if (rv != CKR_OK) {
show_error(" C_Login #1", rv );
- return rv;
+ ec++;
+ goto close_session;
+ }
+
+ rv = do_Cleanup(session);
+ if (rv != CKR_OK) {
+ show_error("do_Cleanup()", rv);
+ ec++;
+ goto close_session;
}
rv = funcs->C_GetMechanismInfo(slot_id, CKM_RSA_PKCS_KEY_PAIR_GEN, &rsakeygeninfo);
if (rv != CKR_OK) {
show_error("C_GetMechanismInfo(CKM_RSA_PKCS_KEY_PAIR_GEN)", rv);
- return -1;
+ ec++;
+ goto close_session;
}
bits = 512;
@@ -253,7 +300,8 @@ main( int argc, char **argv )
rv = do_GenerateTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_GenerateTokenRSAKeyPair(512)", rv);
- return -1;
+ ec++;
+ goto close_session;
}
} else {
testcase_skip("do_GenerateTokenRSAKeyPair(512)");
@@ -265,7 +313,8 @@ main( int argc, char **argv )
rv = do_GenerateTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_GenerateTokenRSAKeyPair(1024)", rv);
- return -1;
+ ec++;
+ goto close_session;
}
} else {
testcase_skip("do_GenerateTokenRSAKeyPair(1024)");
@@ -277,7 +326,8 @@ main( int argc, char **argv )
rv = do_GenerateTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_GenerateTokenRSAKeyPair(2048)", rv);
- return -1;
+ ec++;
+ goto close_session;
}
} else {
testcase_skip("do_GenerateTokenRSAKeyPair(2048)");
@@ -289,7 +339,8 @@ main( int argc, char **argv )
rv = do_GenerateTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_GenerateTokenRSAKeyPair(4096)", rv);
- return -1;
+ ec++;
+ goto close_session;
}
} else {
testcase_skip("do_GenerateTokenRSAKeyPair(4096)");
@@ -298,30 +349,35 @@ main( int argc, char **argv )
rv = funcs->C_CloseSession( session );
if (rv != CKR_OK) {
show_error(" C_CloseSession #3", rv );
- return rv;
+ ec++;
+ goto finalize;
}
rv = funcs->C_Finalize( NULL );
if (rv != CKR_OK) {
show_error("C_Finalize", rv);
- return -1;
+ ec++;
+ goto out;
}
/* Open a new session and re-login */
if ((rv = funcs->C_Initialize( &cinit_args ))) {
show_error("C_Initialize", rv);
- return -1;
+ ec++;
+ goto out;
}
rv = funcs->C_OpenSession( slot_id, flags, NULL, NULL, &session );
if (rv != CKR_OK) {
show_error(" C_OpenSession #2", rv );
+ ec++;
goto finalize;
}
rv = funcs->C_Login( session, CKU_USER, user_pin, user_pin_len );
if (rv != CKR_OK) {
show_error(" C_Login #2", rv );
+ ec++;
goto close_session;
}
@@ -331,6 +387,7 @@ main( int argc, char **argv )
rv = do_VerifyTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_VerifyTokenRSAKeyPair(512)", rv);
+ ec++;
goto close_session;
}
}
@@ -341,6 +398,7 @@ main( int argc, char **argv )
rv = do_VerifyTokenRSAKeyPair(session, label, 1024);
if (rv != CKR_OK) {
show_error("do_VerifyTokenRSAKeyPair(1024)", rv);
+ ec++;
goto close_session;
}
}
@@ -351,6 +409,8 @@ main( int argc, char **argv )
rv = do_VerifyTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_VerifyTokenRSAKeyPair(2048)", rv);
+ ec++;
+ goto close_session;
}
}
@@ -360,23 +420,35 @@ main( int argc, char **argv )
rv = do_VerifyTokenRSAKeyPair(session, label, bits);
if (rv != CKR_OK) {
show_error("do_VerifyTokenRSAKeyPair(4096)", rv);
+ ec++;
+ goto close_session;
}
}
+ rv = do_Cleanup(session);
+ if (rv != CKR_OK) {
+ show_error("do_Cleanup()", rv);
+ ec++;
+ goto close_session;
+ }
+
close_session:
rv = funcs->C_CloseSession( session );
if (rv != CKR_OK) {
show_error(" C_CloseSession #3", rv );
- return rv;
+ ec++;
}
finalize:
rv = funcs->C_Finalize( NULL );
if (rv != CKR_OK) {
show_error("C_Finalize", rv);
- return -1;
+ ec++;
}
+out:
+ if (ec == 0)
+ printf("%s: Success\n", argv[0]);
+ else
+ printf("%s: Failure\n", argv[0]);
- printf("%s: Success\n", argv[0]);
-
- return 0;
+ return ec;
}
diff --git a/usr/lib/pkcs11/common/asn1.c b/usr/lib/pkcs11/common/asn1.c
index 8d0e33a..d6a0384 100755
--- a/usr/lib/pkcs11/common/asn1.c
+++ b/usr/lib/pkcs11/common/asn1.c
@@ -315,22 +315,31 @@ ber_encode_INTEGER( CK_BBOOL length_only,
CK_ULONG data_len )
{
CK_BYTE *buf = NULL;
- CK_ULONG len;
+ CK_ULONG len, padding = 0;
+
+ // ber encoded integers are alway signed. So if the msb of the first byte
+ // is set, this would indicate an negative value if we just copy the
+ // (unsigned) big integer from *data to the ber buffer. So in this case
+ // a preceding 0x00 byte is stored before the actual data. The decode
+ // function does the reverse and may skip this padding.
+ if ((length_only && (!data || *data & 0x80))
+ || (*data & 0x80))
+ padding = 1;
// if data_len < 127 use short-form length id
// if data_len < 256 use long-form length id with 1-byte length field
// if data_len < 65536 use long-form length id with 2-byte length field
// if data_len < 16777216 use long-form length id with 3-byte length field
//
- if (data_len < 128)
- len = 1 + 1 + data_len;
- else if (data_len < 256)
- len = 1 + (1 + 1) + data_len;
- else if (data_len < (1 << 16))
- len = 1 + (1 + 2) + data_len;
- else if (data_len < (1 << 24))
- len = 1 + (1 + 3) + data_len;
+ if (data_len + padding < 128)
+ len = 1 + 1 + padding + data_len;
+ else if (data_len + padding < 256)
+ len = 1 + (1 + 1) + padding + data_len;
+ else if (data_len + padding < (1 << 16))
+ len = 1 + (1 + 2) + padding + data_len;
+ else if (data_len + padding < (1 << 24))
+ len = 1 + (1 + 3) + padding + data_len;
else{
TRACE_ERROR("%s\n", ock_err(ERR_FUNCTION_FAILED));
return CKR_FUNCTION_FAILED;
@@ -345,47 +354,59 @@ ber_encode_INTEGER( CK_BBOOL length_only,
TRACE_ERROR("%s\n", ock_err(ERR_HOST_MEMORY));
return CKR_HOST_MEMORY;
}
- if (data_len < 128) {
+ if (data_len + padding < 128) {
buf[0] = 0x02;
- buf[1] = data_len;
- memcpy( &buf[2], data, data_len );
-
+ buf[1] = data_len + padding;
+ if (padding) {
+ buf[2] = 0x00;
+ memcpy( &buf[3], data, data_len );
+ } else
+ memcpy( &buf[2], data, data_len );
*ber_int_len = len;
*ber_int = buf;
return CKR_OK;
}
- if (data_len < 256) {
+ if (data_len + padding < 256) {
buf[0] = 0x02;
buf[1] = 0x81;
- buf[2] = data_len;
- memcpy( &buf[3], data, data_len );
-
+ buf[2] = data_len + padding;
+ if (padding) {
+ buf[3] = 0x00;
+ memcpy( &buf[4], data, data_len );
+ } else
+ memcpy( &buf[3], data, data_len );
*ber_int_len = len;
*ber_int = buf;
return CKR_OK;
}
- if (data_len < (1 << 16)) {
+ if (data_len + padding < (1 << 16)) {
buf[0] = 0x02;
buf[1] = 0x82;
- buf[2] = (data_len >> 8) & 0xFF;
- buf[3] = (data_len ) & 0xFF;
- memcpy( &buf[4], data, data_len );
-
+ buf[2] = ((data_len + padding) >> 8) & 0xFF;
+ buf[3] = ((data_len + padding) ) & 0xFF;
+ if (padding) {
+ buf[4] = 0x00;
+ memcpy( &buf[5], data, data_len );
+ } else
+ memcpy( &buf[4], data, data_len );
*ber_int_len = len;
*ber_int = buf;
return CKR_OK;
}
- if (data_len < (1 << 24)) {
+ if (data_len + padding < (1 << 24)) {
buf[0] = 0x02;
buf[1] = 0x83;
- buf[2] = (data_len >> 16) & 0xFF;
- buf[3] = (data_len >> 8) & 0xFF;
- buf[4] = (data_len ) & 0xFF;
- memcpy( &buf[5], data, data_len );
-
+ buf[2] = ((data_len + padding) >> 16) & 0xFF;
+ buf[3] = ((data_len + padding) >> 8) & 0xFF;
+ buf[4] = ((data_len + padding) ) & 0xFF;
+ if (padding) {
+ buf[5] = 0x00;
+ memcpy( &buf[6], data, data_len );
+ } else
+ memcpy( &buf[5], data, data_len );
*ber_int_len = len;
*ber_int = buf;
return CKR_OK;
@@ -417,13 +438,25 @@ ber_decode_INTEGER( CK_BYTE * ber_int,
TRACE_ERROR("%s\n", ock_err(ERR_FUNCTION_FAILED));
return CKR_FUNCTION_FAILED;
}
+
+ // ber encoded integers are alway signed. So it may be that the very first
+ // byte is just a padding 0x00 value because the following byte has the msb
+ // set and without the padding the value would indicate a negative value.
+ // However, opencryptoki always stores big integers 'unsigned' meaning
+ // even when the msb is set, there is no preceding 0x00. Even more some
+ // tests may fail e.g. the size in bytes of a modulo big integer should be
+ // modulo bits / 8 which is not true with preceeding 0x00 byte.
+
// short form lengths are easy
//
if ((ber_int[1] & 0x80) == 0) {
len = ber_int[1] & 0x7F;
-
*data = &ber_int[2];
*data_len = len;
+ if (ber_int[2] == 0x00) {
+ *data = &ber_int[3];
+ *data_len = len - 1;
+ }
*field_len = 1 + 1 + len;
return CKR_OK;
}
@@ -432,9 +465,12 @@ ber_decode_INTEGER( CK_BYTE * ber_int,
if (length_octets == 1) {
len = ber_int[2];
-
*data = &ber_int[3];
*data_len = len;
+ if (ber_int[3] == 0x00) {
+ *data = &ber_int[4];
+ *data_len = len - 1;
+ }
*field_len = 1 + (1 + 1) + len;
return CKR_OK;
}
@@ -443,9 +479,12 @@ ber_decode_INTEGER( CK_BYTE * ber_int,
len = ber_int[2];
len = len << 8;
len |= ber_int[3];
-
*data = &ber_int[4];
*data_len = len;
+ if (ber_int[4] == 0x00) {
+ *data = &ber_int[5];
+ *data_len = len - 1;
+ }
*field_len = 1 + (1 + 2) + len;
return CKR_OK;
}
@@ -456,9 +495,12 @@ ber_decode_INTEGER( CK_BYTE * ber_int,
len |= ber_int[3];
len = len << 8;
len |= ber_int[4];
-
*data = &ber_int[5];
*data_len = len;
+ if (ber_int[5] == 0x00) {
+ *data = &ber_int[6];
+ *data_len = len - 1;
+ }
*field_len = 1 + (1 + 3) + len;
return CKR_OK;
}
--
2.7.4
|
