From 8135a5520089ad71e01e33d1688ab16b1bb2c121 Mon Sep 17 00:00:00 2001
From: Trevor Bentley <trevor@yubico.com>
Date: Wed, 13 Sep 2017 15:55:55 +0200
Subject: [PATCH] yubico-piv-tool: Switch to ykpiv_set_pin_retries()

---
 lib/tests/util.c       |  2 +-
 lib/ykpiv.c            | 18 +++++++++++++-----
 lib/ykpiv.h            |  5 ++++-
 tool/yubico-piv-tool.c | 22 ++++++----------------
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/lib/tests/util.c b/lib/tests/util.c
index 6c19f38..efbce40 100644
--- a/lib/tests/util.c
+++ b/lib/tests/util.c
@@ -589,7 +589,7 @@ START_TEST(test_reset) {
   test_authenticate(0);
   res = ykpiv_verify(g_state, "123456", NULL);
   ck_assert_int_eq(res, YKPIV_OK);
-  res = ykpiv_set_pin_retries(g_state, 8);
+  res = ykpiv_set_pin_retries(g_state, 8, 3);
   ck_assert_int_eq(res, YKPIV_OK);
 
   // Block and reset again, verifying increased PIN retries
diff --git a/lib/ykpiv.c b/lib/ykpiv.c
index 3d82c7d..2721c50 100644
--- a/lib/ykpiv.c
+++ b/lib/ykpiv.c
@@ -963,28 +963,36 @@ ykpiv_rc ykpiv_get_pin_retries(ykpiv_state *state, int* tries) {
   if (NULL == state || NULL == tries) {
     return YKPIV_ARGUMENT_ERROR;
   }
+  // Force a re-select to unverify, because once verified the spec dictates that
+  // subsequent verify calls will return a "verification not needed" instead of
+  // the number of tries left...
   res = _ykpiv_select_application(state);
+  if (res != YKPIV_OK) return res;
   ykrc = ykpiv_verify(state, NULL, tries);
   // WRONG_PIN is expected on successful query.
   return ykrc == YKPIV_WRONG_PIN ? YKPIV_OK : ykrc;
 }
 
 
-ykpiv_rc ykpiv_set_pin_retries(ykpiv_state *state, const int tries) {
+ykpiv_rc ykpiv_set_pin_retries(ykpiv_state *state, int pin_tries, int puk_tries) {
   ykpiv_rc res = YKPIV_OK;
-  unsigned char templ[] = {0, YKPIV_INS_SET_PIN_RETRIES, (unsigned char)tries, YKPIV_RETRIES_DEFAULT};
+  unsigned char templ[] = {0, YKPIV_INS_SET_PIN_RETRIES, 0, 0};
   unsigned char data[0xff];
   unsigned long recv_len = sizeof(data);
   int sw = 0;
 
-  if (0 == tries) {
-    //zero value means no change in retry count according to minidriver spec
+  // Special case: if either retry count is 0, it's a successful no-op
+  if (pin_tries == 0 || puk_tries == 0) {
     return YKPIV_OK;
   }
-  if (tries > YKPIV_RETRIES_MAX || tries < 1) {
+
+  if (pin_tries > 0xff || puk_tries > 0xff || pin_tries < 1 || puk_tries < 1) {
     return YKPIV_RANGE_ERROR;
   }
 
+  templ[2] = (unsigned char)pin_tries;
+  templ[3] = (unsigned char)puk_tries;
+
   res = ykpiv_transfer_data(state, templ, NULL, 0, data, &recv_len, &sw);
   if (YKPIV_OK == res) {
     if (SW_SUCCESS == sw) {
diff --git a/lib/ykpiv.h b/lib/ykpiv.h
index 637f655..e39d9a8 100644
--- a/lib/ykpiv.h
+++ b/lib/ykpiv.h
@@ -122,8 +122,10 @@ extern "C"
                                     const unsigned char *qinv, size_t qinv_len,
                                     const unsigned char *ec_data, unsigned char ec_data_len,
                                     const unsigned char pin_policy, const unsigned char touch_policy);
+  // TREV TODO: document that this only works when NOT verified, as per spec (NIST SP 800-73-3 part 2 page 11)
   ykpiv_rc ykpiv_get_pin_retries(ykpiv_state *state, int* tries);
-  ykpiv_rc ykpiv_set_pin_retries(ykpiv_state *state, const int tries);
+  // TREV TODO: document that 0 == successful no-op.
+  ykpiv_rc ykpiv_set_pin_retries(ykpiv_state *state, int pin_tries, int puk_tries);
 
 #define YKPIV_ALGO_TAG 0x80
 #define YKPIV_ALGO_3DES 0x03
@@ -203,6 +205,7 @@ extern "C"
 #define YKPIV_INS_AUTHENTICATE 0x87
 #define YKPIV_INS_GET_DATA 0xcb
 #define YKPIV_INS_PUT_DATA 0xdb
+  // TREV TODO: why aren't all of them here?  ex: select app (0xa4)
 
 /* sw is status words, see NIST special publication 800-73-4, section 5.6 */
 #define SW_SUCCESS 0x9000
diff --git a/tool/yubico-piv-tool.c b/tool/yubico-piv-tool.c
index 2f3ca28..58ed1bd 100644
--- a/tool/yubico-piv-tool.c
+++ b/tool/yubico-piv-tool.c
@@ -257,26 +257,16 @@ static bool reset(ykpiv_state *state) {
 }
 
 static bool set_pin_retries(ykpiv_state *state, int pin_retries, int puk_retries, int verbose) {
-  unsigned char templ[] = {0, YKPIV_INS_SET_PIN_RETRIES, pin_retries, puk_retries};
-  unsigned char data[0xff];
-  unsigned long recv_len = sizeof(data);
-  int sw;
-
-  if(pin_retries > 0xff || puk_retries > 0xff || pin_retries < 1 || puk_retries < 1) {
-    fprintf(stderr, "pin and puk retries must be between 1 and 255.\n");
-    return false;
-  }
+  ykpiv_rc res;
 
   if(verbose) {
     fprintf(stderr, "Setting pin retries to %d and puk retries to %d.\n", pin_retries, puk_retries);
   }
-
-  if(ykpiv_transfer_data(state, templ, NULL, 0, data, &recv_len, &sw) != YKPIV_OK) {
-    return false;
-  } else if(sw == SW_SUCCESS) {
-    return true;
+  res = ykpiv_set_pin_retries(state, pin_retries, puk_retries);
+  if (res == YKPIV_RANGE_ERROR) {
+    fprintf(stderr, "pin and puk retries must be between 1 and 255.\n");
   }
-  return false;
+  return res == YKPIV_OK;
 }
 
 static bool import_key(ykpiv_state *state, enum enum_key_format key_format,
@@ -1812,7 +1802,7 @@ int main(int argc, char *argv[]) {
         }
         break;
       case action_arg_pinMINUS_retries:
-        if(!args_info.pin_retries_arg || !args_info.puk_retries_arg) {
+        if(!args_info.pin_retries_given || !args_info.puk_retries_given) {
           fprintf(stderr, "The '%s' action needs both --pin-retries and --puk-retries arguments.\n",
               cmdline_parser_action_values[action]);
           return EXIT_FAILURE;