Merge pull request #56 from canokeys/fix/oath-piv

Bug fix on recent changes of OATH & PIV

.github/workflows/tests.yml

@@ -12,17 +12,23 @@ jobs:
         sudo apt-get install -q -y git gcc g++ cmake swig psmisc procps pcscd pcsc-tools yubico-piv-tool libhidapi-dev libassuan-dev libgcrypt20-dev libksba-dev libnpth0-dev opensc openssl openssh-server libpcsclite-dev libudev-dev libcmocka-dev python3-pip python3-setuptools python3-wheel lcov yubikey-manager libcbor-dev
         pip3 install --upgrade pip
 
-    - name: Set up Go 1.13
-      uses: actions/setup-go@v1
+    - name: Set up Go 1.16
+      uses: actions/setup-go@v4
       with:
-        go-version: 1.13
+        go-version: "^1.16.1"
       id: go
 
     - name: Check out code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v4
       with:
         submodules: recursive
 
+    - name: Check out piv-go
+      uses: actions/checkout@v4
+      with:
+        repository: canokeys/piv-go
+        path: piv-go
+
     - name: Cache GO Modules
       uses: actions/cache@v3
       env:
@@ -323,7 +329,9 @@ jobs:
         opensc-tool -r "$RDID" -s '00 F8 00 00' | grep 'SW1=0x90, SW2=0x00' # PIV_INS_GET_SERIAL, Yubico
         opensc-tool -r "$RDID" -s '00 FD 00 00' | grep 'SW1=0x90, SW2=0x00' # PIV_INS_GET_VERSION, Yubico
         pkcs15-tool --reader "$RDID" -D
-        PIV_EXT_AUTH_KEY=test-via-pcsc/PIV_EXT_AUTH_KEY.txt piv-tool --reader "$RDID" --admin A:9B:03 || true # External Auth
+
+        cd piv-go; go test -v ./piv --wipe-yubikey; cd -
+
         yubico-piv-tool -r "$RDID" -a verify-pin -P 123456
         yubico-piv-tool -r "$RDID" -a change-pin -P 123456 -N 654321
         yubico-piv-tool -r "$RDID" -a verify-pin -P 654321
@@ -332,6 +340,9 @@ jobs:
         yubico-piv-tool -r "$RDID" -a verify-pin -P 654321
         yubico-piv-tool -r "$RDID" -a set-mgm-key -n F1F2F3F4F5F6F7F8F1F2F3F4F5F6F7F8F1F2F3F4F5F6F7F8
         yubico-piv-tool -r "$RDID" -a set-mgm-key --key=F1F2F3F4F5F6F7F8F1F2F3F4F5F6F7F8F1F2F3F4F5F6F7F8 -n 010203040506070801020304050607080102030405060708
+        #export PIV_EXT_AUTH_KEY=test-via-pcsc/PIV_EXT_AUTH_KEY.txt 
+        #piv-tool --reader "$RDID" --admin A:9B:03  # External Auth
+        #piv-tool --reader "$RDID" --admin M:9B:03  # Mutual Auth
         ## Key generation
         PIVGenKeyCert() {
           key=$1
@@ -350,24 +361,22 @@ jobs:
           if [[ -z "$op" || d = "$op" ]]; then yubico-piv-tool -r "$RDID" $pinArgs -a test-decipher -s $key < /tmp/cert-$key.pem; fi
         }
         ## RSA2048 tests
-        for s in 9a 9c 9d 9e; do PIVGenKeyCert $s "/CN=CertAtSlot$s/" RSA2048; done
+        for s in 9a 9c 9d 9e 82 83; do PIVGenKeyCert $s "/CN=CertAtSlot$s/" RSA2048; done
         yubico-piv-tool -r "$RDID" -a status
         PIVSignDec 9e # PIN not required for key 9e
-        for s in 9a 9c 9d; do PIVSignDec $s 1; done
+        for s in 9a 9c 9d 82 83; do PIVSignDec $s 1; done
         pkcs15-tool --reader "$RDID" --read-certificate 04 | openssl x509 -text | grep 'CN = CertAtSlot9e'
         echo -n hello >/tmp/hello.txt
         pkcs11-tool --slot "$RDID" -d 04 -s -m SHA256-RSA-PKCS -i /tmp/hello.txt -o /tmp/hello-signed --pin 654321
         openssl dgst -sha256 -verify /tmp/pubkey-9e.pem -signature /tmp/hello-signed /tmp/hello.txt
         ## ECC256 tests
-        for s in 9a 9c 9d 9e; do PIVGenKeyCert $s "/CN=CertAtSlot$s/" ECCP256; done
+        for s in 9a 9c 9d 9e 82 83; do PIVGenKeyCert $s "/CN=CertAtSlot$s/" ECCP256; done
         yubico-piv-tool -r "$RDID" -a status
-        for s in 9a 9c 9e; do PIVSignDec $s 1 s; done # 9a/9c/9e only do the ECDSA
-        PIVSignDec 9d 1 d # 9d only do the ECDH
+        for s in 9a 9c 9d 9e 82 83; do PIVSignDec $s 1 s;PIVSignDec $s 1 d; done
         ## ECC384 tests
         for s in 9a 9c 9d 9e; do PIVGenKeyCert $s "/CN=CertAtSlot$s/" ECCP384; done
         yubico-piv-tool -r "$RDID" -a status
-        for s in 9a 9c 9e; do PIVSignDec $s 1 s; done # 9a/9c/9e only do the ECDSA
-        PIVSignDec 9d 1 d # 9d only do the ECDH
+        for s in 9a 9c 9d 9e 82 83; do PIVSignDec $s 1 s;PIVSignDec $s 1 d; done
         ## PIN unblock
         yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s 9a < /tmp/cert-9a.pem
         yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s 9c < /tmp/cert-9c.pem
@@ -382,13 +391,17 @@ jobs:
         ## Key import
         openssl ecparam -name prime256v1 -out p256.pem
         openssl req -x509 -newkey ec:p256.pem -keyout key.pem -out cert.pem -days 365 -nodes -subj "/CN=www.example.com"
-        yubico-piv-tool -r "$RDID" -a import-key -s 9a -i key.pem
-        yubico-piv-tool -r "$RDID" -a import-certificate -s 9a -i cert.pem
-        yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s 9a <cert.pem
+        for s in 9a 9d 82 83; do
+            yubico-piv-tool -r "$RDID" -a import-key -s $s -i key.pem
+            yubico-piv-tool -r "$RDID" -a import-certificate -s $s -i cert.pem
+            yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s $s <cert.pem
+        done
         openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365 -nodes -subj "/CN=www.example.com"
-        yubico-piv-tool -r "$RDID" -a import-key -s 9c -i key.pem
-        yubico-piv-tool -r "$RDID" -a import-certificate -s 9c -i cert.pem
-        yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s 9c <cert.pem
+        for s in 9c 9d 82 83; do
+            yubico-piv-tool -r "$RDID" -a import-key -s $s -i key.pem
+            yubico-piv-tool -r "$RDID" -a import-certificate -s $s -i cert.pem
+            yubico-piv-tool -r "$RDID" -P 654321 -a verify-pin -a test-signature -s $s <cert.pem
+        done
         ## Factory reset
         yubico-piv-tool -r "$RDID" -a change-puk -P 12345678 -N 11111111 2>&1 | grep 'Failed verifying puk code, now 2 tries left before blocked'
         yubico-piv-tool -r "$RDID" -a change-puk -P 12345678 -N 11111111 2>&1 | grep 'Failed verifying puk code, now 1 tries left before blocked'
@@ -401,7 +414,7 @@ jobs:
         yubico-piv-tool -r "$RDID" -a unblock-pin -P 12345678 -N 654321 2>&1 | grep 'Successfully unblocked the pin code'
         ## Test long data object
         yubico-piv-tool -r "$RDID" -a set-ccc -a set-chuid -a status
-        for s in 9a 9c 9d 9e; do
+        for s in 9a 9c 9d 9e 82 83; do
           PIVGenKeyCert $s "/CN=CertAtSlot$s/" RSA2048
           yubico-piv-tool -r "$RDID" -a import-certificate -s $s -i test-via-pcsc/long-cert.pem
         done

applets/oath/oath.c

@@ -18,7 +18,7 @@ static enum {
   REMAINING_LIST,
 } oath_remaining_type;
 
-static uint8_t challenge[MAX_CHALLENGE_LEN], challenge_len, record_idx, is_validated;
+static uint8_t auth_challenge[MAX_CHALLENGE_LEN], record_idx, is_validated;
 
 void oath_poweroff(void) {
   oath_remaining_type = REMAINING_NONE;
@@ -53,14 +53,14 @@ static int oath_select(const CAPDU *capdu, RAPDU *rapdu) {
   if (ret == 0) { // no key is set
     is_validated = true;
   } else {
-    random_buffer(challenge, sizeof(challenge));
+    random_buffer(auth_challenge, sizeof(auth_challenge));
     RDATA[7 + HANDLE_LEN] = OATH_TAG_CHALLENGE;
-    RDATA[8 + HANDLE_LEN] = sizeof(challenge);
-    memcpy(RDATA + 9 + HANDLE_LEN, challenge, sizeof(challenge));
-    RDATA[9 + HANDLE_LEN + sizeof(challenge)] = OATH_TAG_ALGORITHM;
-    RDATA[10 + HANDLE_LEN + sizeof(challenge)] = 1;
-    RDATA[11 + HANDLE_LEN + sizeof(challenge)] = OATH_ALG_SHA1;
-    LL += 5 + sizeof(challenge);
+    RDATA[8 + HANDLE_LEN] = sizeof(auth_challenge);
+    memcpy(RDATA + 9 + HANDLE_LEN, auth_challenge, sizeof(auth_challenge));
+    RDATA[9 + HANDLE_LEN + sizeof(auth_challenge)] = OATH_TAG_ALGORITHM;
+    RDATA[10 + HANDLE_LEN + sizeof(auth_challenge)] = 1;
+    RDATA[11 + HANDLE_LEN + sizeof(auth_challenge)] = OATH_ALG_SHA1;
+    LL += 5 + sizeof(auth_challenge);
   }
 
   return 0;
@@ -165,6 +165,13 @@ static int oath_delete(const CAPDU *capdu, RAPDU *rapdu) {
   for (size_t i = 0; i != nRecords; ++i) {
     if (read_file(OATH_FILE, &record, i * sizeof(OATH_RECORD), sizeof(OATH_RECORD)) < 0) return -1;
     if (record.name_len == name_len && memcmp(record.name, name_ptr, name_len) == 0) {
+      uint32_t default_item;
+      if (read_attr(OATH_FILE, ATTR_DEFAULT_RECORD, &default_item, sizeof(default_item)) < 0) return -1;
+      if (default_item == i) { // clear the default set if it is to be deleted
+        default_item = 0xffffffff;
+        if (write_attr(OATH_FILE, ATTR_DEFAULT_RECORD, &default_item, sizeof(default_item)) < 0) return -1;
+      }
+
       record.name_len = 0;
       return write_file(OATH_FILE, &record, i * sizeof(OATH_RECORD), sizeof(OATH_RECORD), 0);
     }
@@ -193,45 +200,55 @@ static int oath_rename(const CAPDU *capdu, RAPDU *rapdu) {
   // find the record
   int size = get_file_size(OATH_FILE);
   if (size < 0) return -1;
-  uint32_t nRecords = size / sizeof(OATH_RECORD), i;
+  uint32_t nRecords = size / sizeof(OATH_RECORD), i, idx_old;
   uint32_t file_offset;
   OATH_RECORD record;
-  for (i = 0; i != nRecords; ++i) {
+  for (i = 0, idx_old = nRecords; i < nRecords; ++i) {
     file_offset = i * sizeof(OATH_RECORD);
     if (read_file(OATH_FILE, &record, file_offset, sizeof(OATH_RECORD)) < 0) return -1;
-    if (record.name_len == old_name_len && memcmp(record.name, old_name_ptr, old_name_len) == 0) break;
+    if (idx_old == nRecords && record.name_len == old_name_len && memcmp(record.name, old_name_ptr, old_name_len) == 0) idx_old = i;
+    if (record.name_len == new_name_len && memcmp(record.name, new_name_ptr, new_name_len) == 0) {
+      DBG_MSG("dup name\n");
+      EXCEPT(SW_CONDITIONS_NOT_SATISFIED);
+    }
   }
-  if (i == nRecords) EXCEPT(SW_DATA_INVALID);
+  if (idx_old == nRecords) EXCEPT(SW_DATA_INVALID);
 
   // update the name
+  if (read_file(OATH_FILE, &record, idx_old * sizeof(OATH_RECORD), sizeof(OATH_RECORD)) < 0) return -1;
   record.name_len = new_name_len;
   memcpy(record.name, new_name_ptr, new_name_len);
-  return write_file(OATH_FILE, &record, i * sizeof(OATH_RECORD), sizeof(OATH_RECORD), 0);
+  return write_file(OATH_FILE, &record, idx_old * sizeof(OATH_RECORD), sizeof(OATH_RECORD), 0);
 }
 
 static int oath_set_code(const CAPDU *capdu, RAPDU *rapdu) {
   if (P1 != 0x00 || P2 != 0x00) EXCEPT(SW_WRONG_P1P2);
 
+  uint8_t key_len, chal_len, resp_len;
+  uint8_t *key_ptr, *chal_ptr, *resp_ptr;
   // check input data first
   uint16_t offset = 0;
+  if (LC == 0) goto clear_code;
   if (LC < 2) EXCEPT(SW_WRONG_LENGTH);
   if (DATA[offset++] != OATH_TAG_KEY) EXCEPT(SW_WRONG_DATA);
-  uint8_t key_len = DATA[offset++];
-  uint8_t *key_ptr = &DATA[offset];
+  key_len = DATA[offset++];
+  key_ptr = &DATA[offset];
   if (key_len == 0) { // clear the code
+clear_code:
+    is_validated = 1;
     return write_attr(OATH_FILE, ATTR_KEY, NULL, 0);
   }
   if (key_len != KEY_LEN + 1) EXCEPT(SW_WRONG_DATA);
   offset += key_len;
   if (LC <= offset + 2) EXCEPT(SW_WRONG_LENGTH);
   if (DATA[offset++] != OATH_TAG_CHALLENGE) EXCEPT(SW_WRONG_DATA);
-  uint8_t chal_len = DATA[offset++];
-  uint8_t *chal_ptr = &DATA[offset];
+  chal_len = DATA[offset++];
+  chal_ptr = &DATA[offset];
   offset += chal_len;
   if (LC <= offset + 2) EXCEPT(SW_WRONG_LENGTH);
   if (DATA[offset++] != OATH_TAG_FULL_RESPONSE) EXCEPT(SW_WRONG_DATA);
-  uint8_t resp_len = DATA[offset++];
-  uint8_t *resp_ptr = &DATA[offset];
+  resp_len = DATA[offset++];
+  resp_ptr = &DATA[offset];
   if (resp_len != SHA1_DIGEST_LENGTH) EXCEPT(SW_WRONG_DATA);
   offset += resp_len;
   if (LC != offset) EXCEPT(SW_WRONG_LENGTH);
@@ -241,6 +258,7 @@ static int oath_set_code(const CAPDU *capdu, RAPDU *rapdu) {
   hmac_sha1(key_ptr + 1, KEY_LEN, chal_ptr, chal_len, hmac);
   if (memcmp_s(hmac, resp_ptr, SHA1_DIGEST_LENGTH) != 0) EXCEPT(SW_DATA_INVALID);
 
+  is_validated = 0;
   // save the key
   return write_attr(OATH_FILE, ATTR_KEY, key_ptr + 1, key_len - 1);
 }
@@ -269,9 +287,9 @@ static int oath_validate(const CAPDU *capdu, RAPDU *rapdu) {
   if (ret < 0) return -1;
   if (ret == 0) EXCEPT(SW_DATA_INVALID);
   uint8_t hmac[SHA1_DIGEST_LENGTH];
-  hmac_sha1(key, KEY_LEN, challenge, sizeof(challenge), hmac);
-  if (memcmp_s(hmac, resp_ptr, SHA1_DIGEST_LENGTH) != 0) EXCEPT(SW_DATA_INVALID);
-  is_validated = true;
+  hmac_sha1(key, KEY_LEN, auth_challenge, sizeof(auth_challenge), hmac);
+  is_validated = (memcmp_s(hmac, resp_ptr, SHA1_DIGEST_LENGTH) == 0);
+  if (!is_validated) EXCEPT(SW_WRONG_DATA);
 
   // build the response
   hmac_sha1(key, KEY_LEN, chal_ptr, chal_len, hmac);
@@ -323,7 +341,7 @@ static int oath_update_challenge_field(OATH_RECORD *record, size_t file_offset)
                     sizeof(record->challenge), 0);
 }
 
-static int oath_enforce_increasing(OATH_RECORD *record, size_t file_offset) {
+static int oath_enforce_increasing(OATH_RECORD *record, size_t file_offset, uint8_t challenge_len, uint8_t challenge[MAX_CHALLENGE_LEN]) {
   if ((record->prop & OATH_PROP_INC)) {
     if (challenge_len != sizeof(record->challenge)) return -1;
     DBG_MSG("challenge_len=%u %hhu %hhu\n", challenge_len, record->challenge[7], challenge[7]);
@@ -344,7 +362,7 @@ static int oath_increase_counter(OATH_RECORD *record) {
   return i >= 0 ? 0 : -1;
 }
 
-static uint8_t *oath_digest(OATH_RECORD *record, uint8_t buffer[SHA512_DIGEST_LENGTH]) {
+static uint8_t *oath_digest(OATH_RECORD *record, uint8_t buffer[SHA512_DIGEST_LENGTH], uint8_t challenge_len, uint8_t challenge[MAX_CHALLENGE_LEN]) {
   uint8_t digest_length;
   if ((record->key[0] & OATH_ALG_MASK) == OATH_ALG_SHA1) {
     hmac_sha1(record->key + 2, record->key_len - 2, challenge, challenge_len, buffer);
@@ -366,6 +384,8 @@ static int oath_calculate_by_offset(size_t file_offset, uint8_t result[4]) {
   if (file_offset % sizeof(OATH_RECORD) != 0) return -2;
   int size = get_file_size(OATH_FILE);
   if (size < 0 || file_offset >= size) return -2;
+  uint8_t challenge_len;
+  uint8_t challenge[MAX_CHALLENGE_LEN];
   OATH_RECORD record;
   if (read_file(OATH_FILE, &record, file_offset, sizeof(OATH_RECORD)) < 0) return -1;
 
@@ -383,10 +403,12 @@ static int oath_calculate_by_offset(size_t file_offset, uint8_t result[4]) {
 
     challenge_len = sizeof(record.challenge);
     memcpy(challenge, record.challenge, challenge_len);
+  } else {
+    return -1;
   }
 
   uint8_t hash[SHA512_DIGEST_LENGTH];
-  memcpy(result, oath_digest(&record, hash), 4);
+  memcpy(result, oath_digest(&record, hash, challenge_len, challenge), 4);
   return record.key[1]; // the number of digits
 }
 
@@ -454,6 +476,8 @@ static int oath_calculate(const CAPDU *capdu, RAPDU *rapdu) {
     }
   }
 
+  uint8_t challenge_len;
+  uint8_t challenge[MAX_CHALLENGE_LEN];
   if ((record.key[0] & OATH_TYPE_MASK) == OATH_TYPE_TOTP) {
     if (offset + 1 >= LC) EXCEPT(SW_WRONG_LENGTH);
     if (DATA[offset++] != OATH_TAG_CHALLENGE) EXCEPT(SW_WRONG_DATA);
@@ -467,7 +491,7 @@ static int oath_calculate(const CAPDU *capdu, RAPDU *rapdu) {
     offset += challenge_len;
     if (offset > LC) EXCEPT(SW_WRONG_LENGTH);
 
-    if (oath_enforce_increasing(&record, file_offset) < 0) EXCEPT(SW_SECURITY_STATUS_NOT_SATISFIED);
+    if (oath_enforce_increasing(&record, file_offset, challenge_len, challenge) < 0) EXCEPT(SW_SECURITY_STATUS_NOT_SATISFIED);
 
   } else if ((record.key[0] & OATH_TYPE_MASK) == OATH_TYPE_HOTP) {
 
@@ -476,19 +500,24 @@ static int oath_calculate(const CAPDU *capdu, RAPDU *rapdu) {
 
     challenge_len = sizeof(record.challenge);
     memcpy(challenge, record.challenge, challenge_len);
+  } else {
+    return -1;
   }
 
   RDATA[0] = OATH_TAG_RESPONSE;
   RDATA[1] = 5;
   RDATA[2] = record.key[1];
 
   uint8_t hash[SHA512_DIGEST_LENGTH];
-  memcpy(RDATA + 3, oath_digest(&record, hash), 4);
+  memcpy(RDATA + 3, oath_digest(&record, hash, challenge_len, challenge), 4);
   LL = 7;
   return 0;
 }
 
 static int oath_calculate_all(const CAPDU *capdu, RAPDU *rapdu) {
+  static uint8_t challenge_len;
+  static uint8_t challenge[MAX_CHALLENGE_LEN];
+
   if (P2 != 0x00 && P2 != 0x01) EXCEPT(SW_WRONG_P1P2);
 
   oath_remaining_type = REMAINING_CALC;
@@ -547,14 +576,14 @@ static int oath_calculate_all(const CAPDU *capdu, RAPDU *rapdu) {
       continue;
     }
 
-    if (oath_enforce_increasing(&record, file_offset) < 0) EXCEPT(SW_SECURITY_STATUS_NOT_SATISFIED);
+    if (oath_enforce_increasing(&record, file_offset, challenge_len, challenge) < 0) EXCEPT(SW_SECURITY_STATUS_NOT_SATISFIED);
 
     RDATA[off_out++] = OATH_TAG_RESPONSE;
     RDATA[off_out++] = 5;
     RDATA[off_out++] = record.key[1];
 
     uint8_t hash[SHA512_DIGEST_LENGTH];
-    memmove(RDATA + off_out, oath_digest(&record, hash), 4);
+    memmove(RDATA + off_out, oath_digest(&record, hash, challenge_len, challenge), 4);
     off_out += 4;
   }
   LL = off_out;