Skip to content

Commit

Permalink
Display error message when signing with X25519 key (#469)
Browse files Browse the repository at this point in the history
Fix minor bugs when signing with ED25519 and X25519
  • Loading branch information
aveenismail authored Jan 30, 2024
1 parent ac4d7e4 commit 47079b4
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 46 deletions.
2 changes: 1 addition & 1 deletion lib/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ ykpiv_rc ykpiv_util_generate_key(ykpiv_state *state, uint8_t slot, uint8_t algor

if ((algorithm == YKPIV_ALGO_RSA3072 || algorithm == YKPIV_ALGO_RSA4096 || YKPIV_IS_25519(algorithm))
&& !is_version_compatible(state, 5, 7, 0)) {
DBG("RSA3072 and RSA4096 keys are only supported in YubiKey version 5.7.0 and above");
DBG("RSA3072, RSA4096, ED25519 and X25519 keys are only supported in YubiKey version 5.7.0 and newer");
return YKPIV_NOT_SUPPORTED;
}
if ((algorithm == YKPIV_ALGO_RSA1024 || algorithm == YKPIV_ALGO_RSA2048) && !is_version_compatible(state, 4, 3, 5)) {
Expand Down
26 changes: 21 additions & 5 deletions lib/ykpiv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ static ykpiv_rc _general_authenticate(ykpiv_state *state,
const unsigned char *sign_in, size_t in_len,
unsigned char *out, size_t *out_len,
unsigned char algorithm, unsigned char key, bool decipher) {
unsigned char indata[1024] = {0};
unsigned char indata[YKPIV_OBJ_MAX_SIZE] = {0};
unsigned char *dataptr = indata;
unsigned char data[1024] = {0};
unsigned char templ[] = {0, YKPIV_INS_AUTHENTICATE, algorithm, key};
Expand Down Expand Up @@ -1223,8 +1223,6 @@ static ykpiv_rc _general_authenticate(ykpiv_state *state,
}
break;
case YKPIV_ALGO_ECCP256:
case YKPIV_ALGO_ED25519:
case YKPIV_ALGO_X25519:
key_len = 32;
// fall through
case YKPIV_ALGO_ECCP384:
Expand All @@ -1238,18 +1236,36 @@ static ykpiv_rc _general_authenticate(ykpiv_state *state,
return YKPIV_SIZE_ERROR;
}
break;
case YKPIV_ALGO_X25519:
if(!decipher) {
DBG("Signing with x25519 keys is not supported");
return YKPIV_NOT_SUPPORTED;
}
if(in_len != 32) {
return YKPIV_SIZE_ERROR;
}
break;
case YKPIV_ALGO_ED25519:
if(decipher) {
DBG("Deciphering with ed25519 keys is not supported");
return YKPIV_NOT_SUPPORTED;
}
break;
default:
return YKPIV_ALGORITHM_ERROR;
}

bytes = _ykpiv_get_length_size(in_len);

*dataptr++ = 0x7c;
dataptr += _ykpiv_set_length(dataptr, in_len + bytes + 3);
*dataptr++ = 0x82;
*dataptr++ = 0x00;
*dataptr++ = (YKPIV_IS_EC(algorithm) || YKPIV_IS_25519(algorithm)) && decipher ? 0x85 : 0x81;
*dataptr++ = !YKPIV_IS_RSA(algorithm) && decipher ? 0x85 : 0x81;
dataptr += _ykpiv_set_length(dataptr, in_len);
if(dataptr - indata + in_len > sizeof(indata)) {
return YKPIV_SIZE_ERROR;
}
memcpy(dataptr, sign_in, in_len);
dataptr += in_len;

Expand Down
92 changes: 92 additions & 0 deletions resources/scripts/cmdline_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ do
fi

$BIN -a verify-pin -P123456 --sign -s $slot -A RSA1024 -i data.txt -o data.sig
openssl dgst -sha256 -verify key.pub -signature data.sig data.txt
done

echo "********************** Generate RSA2048 in all slots ********************* "
Expand Down Expand Up @@ -160,9 +161,100 @@ do
fi

$BIN -a verify-pin -P123456 --sign -s $slot -A RSA2048 -i data.txt -o data.sig
openssl dgst -sha256 -verify key.pub -signature data.sig data.txt
done

echo "********************** Generate RSA3072 in all slots ********************* "

for slot in "${SLOTS[@]}"
do
# Generate key on-board, issue certificate, and verify it
$BIN -agenerate -s$slot -ARSA3072 -o key.pub
$BIN -averify -P123456 -s$slot -S'/CN=YubicoTestRSA3072/OU=YubicoGenerated/O=yubico.com/' -aselfsign -i key.pub -o cert.pem
$BIN -averify -P123456 -s$slot -atest-signature -i cert.pem
$BIN -aimport-certificate -P123456 -s$slot -i cert.pem

# Read status and validate fields
STATUS=$($BIN -astatus)
echo "$STATUS"
ALGO=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Algorithm" |tr -d "[:blank:]")
if [ "x$ALGO" != "xAlgorithm:RSA3072" ]; then
echo "$ALGO"
echo "Generated algorithm incorrect." >/dev/stderr
exit 1
fi

SUBJECT=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Subject DN" |tr -d "[:blank:]")
if [ "x$SUBJECT" != "xSubjectDN:CN=YubicoTestRSA3072,OU=YubicoGenerated,O=yubico.com" ]; then
echo "$SUBJECT"
echo "Certificate subject incorrect." >/dev/stderr
exit 1
fi

$BIN -a verify-pin -P123456 --sign -s $slot -A RSA3072 -i data.txt -o data.sig
openssl dgst -sha256 -verify key.pub -signature data.sig data.txt
done

echo "********************** Generate RSA4096 in all slots ********************* "

for slot in "${SLOTS[@]}"
do
# Generate key on-board, issue certificate, and verify it
$BIN -agenerate -s$slot -ARSA4096 -o key.pub
$BIN -averify -P123456 -s$slot -S'/CN=YubicoTestRSA4096/OU=YubicoGenerated/O=yubico.com/' -aselfsign -i key.pub -o cert.pem
$BIN -averify -P123456 -s$slot -atest-signature -i cert.pem
$BIN -aimport-certificate -P123456 -s$slot -i cert.pem

# Read status and validate fields
STATUS=$($BIN -astatus)
echo "$STATUS"
ALGO=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Algorithm" |tr -d "[:blank:]")
if [ "x$ALGO" != "xAlgorithm:RSA4096" ]; then
echo "$ALGO"
echo "Generated algorithm incorrect." >/dev/stderr
exit 1
fi

SUBJECT=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Subject DN" |tr -d "[:blank:]")
if [ "x$SUBJECT" != "xSubjectDN:CN=YubicoTestRSA4096,OU=YubicoGenerated,O=yubico.com" ]; then
echo "$SUBJECT"
echo "Certificate subject incorrect." >/dev/stderr
exit 1
fi

$BIN -a verify-pin -P123456 --sign -s $slot -A RSA4096 -i data.txt -o data.sig
openssl dgst -sha256 -verify key.pub -signature data.sig data.txt
done

echo "********************** Generate ED25519 in all slots ********************* "

for slot in "${SLOTS[@]}"
do
# Generate key on-board, issue certificate, and verify it
$BIN -agenerate -s$slot -AED25519 -o key.pub
$BIN -averify -P123456 -s$slot -S'/CN=YubicoTestED25519/OU=YubicoGenerated/O=yubico.com/' -aselfsign -i key.pub -o cert.pem
$BIN -aimport-certificate -P123456 -s$slot -i cert.pem

# Read status and validate fields
STATUS=$($BIN -astatus)
echo "$STATUS"
ALGO=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Algorithm" |tr -d "[:blank:]")
if [ "x$ALGO" != "xAlgorithm:ED25519" ]; then
echo "$ALGO"
echo "Generated algorithm incorrect." >/dev/stderr
exit 1
fi

SUBJECT=$(echo "$STATUS" |grep "Slot $slot" -A 6 |grep "Subject DN" |tr -d "[:blank:]")
if [ "x$SUBJECT" != "xSubjectDN:CN=YubicoTestED25519,OU=YubicoGenerated,O=yubico.com" ]; then
echo "$SUBJECT"
echo "Certificate subject incorrect." >/dev/stderr
exit 1
fi

$BIN -a verify-pin -P123456 --sign -s $slot -A ED25519 -i data.txt -o data.sig
openssl pkeyutl -verify -pubin -inkey key.pub -rawin -in data.txt -sigfile data.sig
done



Expand Down
66 changes: 40 additions & 26 deletions tool/yubico-piv-tool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1153,11 +1153,15 @@ static bool selfsign_certificate(ykpiv_state *state, enum enum_key_format key_fo
if(algorithm == 0) {
goto selfsign_out;
}
if(algorithm == YKPIV_ALGO_X25519) {
fprintf(stderr, "Signing with X25519 keys is not supported.\n");
goto selfsign_out;
}

size_t oid_len = 0;
const unsigned char *oid = 0;
const EVP_MD *md = NULL;
if (!YKPIV_IS_25519(algorithm)) {
if (algorithm != YKPIV_ALGO_ED25519) {
md = get_hash(hash, &oid, &oid_len);
if (md == NULL) {
goto selfsign_out;
Expand Down Expand Up @@ -1531,10 +1535,10 @@ static bool sign_file(ykpiv_state *state, const char *input, const char *output,
FILE *output_file = NULL;
int key;
unsigned int hash_len;
unsigned char hashed[EVP_MAX_MD_SIZE * 2] = {0};
unsigned char hashed[YKPIV_OBJ_MAX_SIZE] = {0};
bool ret = false;
int algo;
const EVP_MD *md;
const EVP_MD *md = NULL;

key = get_slot_hex(slot);

Expand Down Expand Up @@ -1562,35 +1566,45 @@ static bool sign_file(ykpiv_state *state, const char *input, const char *output,

{
EVP_MD_CTX *mdctx;

md = get_hash(hash, NULL, NULL);
if(md == NULL) {
if(algo == YKPIV_ALGO_X25519) {
fprintf(stderr, "Signing with X25519 key is not supported\n");
goto out;
}
} else if (algo == YKPIV_ALGO_ED25519) {
hash_len = fread(hashed, 1, sizeof(hashed), input_file);
if(hash_len >= sizeof(hashed)) {
fprintf(stderr, "Cannot perform signature. File too big.\n");
goto out;
}
} else {
md = get_hash(hash, NULL, NULL);
if (md == NULL) {
goto out;
}

mdctx = EVP_MD_CTX_create();
if(EVP_DigestInit_ex(mdctx, md, NULL) != 1) {
fprintf(stderr, "Failed to initialize digest operation\n");
goto out;
}
while(!feof(input_file)) {
char buf[1024] = {0};
size_t len = fread(buf, 1, 1024, input_file);
if(EVP_DigestUpdate(mdctx, buf, len) != 1) {
fprintf(stderr, "Failed to update digest data\n");
mdctx = EVP_MD_CTX_create();
if (EVP_DigestInit_ex(mdctx, md, NULL) != 1) {
fprintf(stderr, "Failed to initialize digest operation\n");
goto out;
}
while (!feof(input_file)) {
char buf[8192] = {0};
size_t len = fread(buf, 1, sizeof(buf), input_file);
if (EVP_DigestUpdate(mdctx, buf, len) != 1) {
fprintf(stderr, "Failed to update digest data\n");
goto out;
}
}
if (EVP_DigestFinal_ex(mdctx, hashed, &hash_len) != 1) {
fprintf(stderr, "Failed to finalize digest operation\n");
goto out;
}
}
if(EVP_DigestFinal_ex(mdctx, hashed, &hash_len) != 1) {
fprintf(stderr, "Failed to finalize digest operation\n");
goto out;
}

if(verbosity) {
fprintf(stderr, "File hashed as: ");
dump_data(hashed, hash_len, stderr, true, format_arg_hex);
if (verbosity) {
fprintf(stderr, "File hashed as: ");
dump_data(hashed, hash_len, stderr, true, format_arg_hex);
}
EVP_MD_CTX_destroy(mdctx);
}
EVP_MD_CTX_destroy(mdctx);
}

if(YKPIV_IS_RSA(algo)) {
Expand Down
26 changes: 13 additions & 13 deletions ykcs11/tests/ykcs11_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,19 @@ static void test_mechanism_list_and_info() {
CKM_SHA512};

static const CK_MECHANISM_INFO token_mechanism_infos[] = { // KEEP ALIGNED WITH token_mechanisms
{1024, 2048, CKF_HW | CKF_GENERATE_KEY_PAIR},
{1024, 2048, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT},
{1024, 2048, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 2048, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_GENERATE_KEY_PAIR},
{1024, 4096, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT},
{1024, 4096, CKF_HW | CKF_ENCRYPT | CKF_DECRYPT | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{1024, 4096, CKF_HW | CKF_SIGN | CKF_VERIFY},
{256, 384, CKF_HW | CKF_GENERATE_KEY_PAIR | CKF_EC_F_P | CKF_EC_NAMEDCURVE | CKF_EC_UNCOMPRESS},
{256, 384, CKF_HW | CKF_SIGN | CKF_VERIFY | CKF_EC_F_P | CKF_EC_NAMEDCURVE | CKF_EC_UNCOMPRESS},
{256, 384, CKF_HW | CKF_SIGN | CKF_VERIFY | CKF_EC_F_P | CKF_EC_NAMEDCURVE | CKF_EC_UNCOMPRESS},
Expand Down
2 changes: 1 addition & 1 deletion ykcs11/token.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include "../common/util.h"

#define MIN_RSA_KEY_SIZE 1024
#define MAX_RSA_KEY_SIZE 2048
#define MAX_RSA_KEY_SIZE 4096
#define MIN_ECC_KEY_SIZE 256
#define MAX_ECC_KEY_SIZE 384

Expand Down

0 comments on commit 47079b4

Please sign in to comment.