Merge branch 'pr-188'

lib/internal.c

@@ -241,6 +241,7 @@ des_rc des_import_key(const int type, const unsigned char* keyraw, const size_t
 EXIT:
 #ifdef _WINDOWS
   if (pbSessionBlob) {
+    yc_memzero(pbSessionBlob, cbSessionBlob);
     free(pbSessionBlob);
     pbSessionBlob = NULL;
   }
@@ -353,6 +354,7 @@ EXIT:
 
 bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) {
 #ifdef _WINDOWS
+  bool rv = false;
   /* defined weak keys, borrowed from openssl to be consistent across platforms */
   static const unsigned char weak_keys[][DES_LEN_DES] = {
     /* weak keys */
@@ -400,11 +402,13 @@ bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) {
     if ((0 == memcmp(weak_keys[i], tmp, DES_LEN_DES)) ||
         (0 == memcmp(weak_keys[i], tmp + DES_LEN_DES, DES_LEN_DES)) ||
         (0 == memcmp(weak_keys[i], tmp + 2*DES_LEN_DES, DES_LEN_DES))) {
-      return true;
+      rv = true;
+      break;
     }
   }
 
-  return false;
+  yc_memzero(tmp, DES_LEN_3DES);
+  return rv;
 #else
   (void)cb_key; /* unused */
 
@@ -458,6 +462,8 @@ pkcs5_rc pkcs5_pbkdf2_sha1(const uint8_t* password, const size_t cb_password, co
   */
 
   if (STATUS_SUCCESS == BCryptOpenAlgorithmProvider(&hAlg, BCRYPT_SHA1_ALGORITHM, NULL, BCRYPT_ALG_HANDLE_HMAC_FLAG)) {
+    /* suppress const qualifier warning b/c BCrypt doesn't take const input buffers */
+#pragma warning(suppress: 4090)
     if (STATUS_SUCCESS != BCryptDeriveKeyPBKDF2(hAlg, (PUCHAR)password, (ULONG)cb_password, (PUCHAR)salt, (ULONG)cb_salt, iterations, key, (ULONG)cb_key, 0)) {
       rc = PKCS5_GENERAL_ERROR;
     }

lib/internal.h

@@ -126,6 +126,7 @@ extern "C"
 #define CB_OBJ_TAG_MIN      2                       // 1 byte tag + 1 byte len
 #define CB_OBJ_TAG_MAX      (CB_OBJ_TAG_MIN + 2)      // 1 byte tag + 3 bytes len
 
+#define CB_PIN_MAX          8
 #define member_size(type, member) sizeof(((type*)0)->member)
 
 typedef enum {
@@ -192,6 +193,7 @@ ykpiv_rc _ykpiv_ensure_application_selected(ykpiv_state *state);
 ykpiv_rc _ykpiv_select_application(ykpiv_state *state);
 unsigned int _ykpiv_set_length(unsigned char *buffer, size_t length);
 unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len);
+bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len);
 
 void* _ykpiv_alloc(ykpiv_state *state, size_t size);
 void* _ykpiv_realloc(ykpiv_state *state, void *address, size_t size);
@@ -236,6 +238,21 @@ typedef enum _yc_log_level_t {
 
 void yc_log_event(uint32_t id, yc_log_level_t level, const char *sz_format, ...);
 
+#ifdef _WIN32
+#include <windows.h>
+#define yc_memzero SecureZeroMemory
+#elif defined(BSD)
+#include <strings.h>
+#define yc_memzero explicit_bzero
+#elif defined(__linux__)
+#include <openssl/crypto.h>
+#define yc_memzero OPENSSL_cleanse
+#else
+#define __STDC_WANT_LIB_EXT1__ 1
+#include <string.h>
+#define yc_memzero(_p, _n) (void)memset_s(_p, (rsize_t)_n, 0, (rsize_t)_n)
+#endif
+
 #ifdef __cplusplus
 }
 #endif

lib/util.c

@@ -218,6 +218,7 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key
   ykpiv_rc res = YKPIV_OK;
   ykpiv_key *pKey = NULL;
   uint8_t *pData = NULL;
+  uint8_t *pTemp = NULL;
   size_t cbData = 0;
   size_t offset = 0;
   uint8_t buf[CB_BUF_MAX];
@@ -274,17 +275,21 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key
 
   for (i = 0; i < sizeof(SLOTS); i++) {
     cbBuf = sizeof(buf);
+    res = _read_certificate(state, SLOTS[i], buf, &cbBuf);
 
-    if (YKPIV_OK == (res = _read_certificate(state, SLOTS[i], buf, &cbBuf))) {
+    if ((res == YKPIV_OK) && (cbBuf > 0)) {
       // add current slot to result, grow result buffer if necessary
 
       cbRealloc = (sizeof(ykpiv_key) + cbBuf - 1) > (cbData - offset) ? MAX((sizeof(ykpiv_key) + cbBuf - 1) - (cbData - offset), CB_PAGE) : 0;
 
       if (0 != cbRealloc) {
-        if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) {
+        if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) {
+          /* realloc failed, pData will be freed in cleanup */
           res = YKPIV_MEMORY_ERROR;
           goto Cleanup;
         }
+        pData = pTemp;
+        pTemp = NULL;
       }
 
       cbData += cbRealloc;
@@ -555,6 +560,7 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat
   int object_id = 0;
   uint8_t tag = 0;
   uint8_t *pData = NULL;
+  uint8_t *pTemp = NULL;
   size_t cbData = 0;
   size_t cbRealloc = 0;
   size_t offset = 0;
@@ -605,10 +611,13 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat
     cbRealloc = len > (cbData - offset) ? len - (cbData - offset) : 0;
 
     if (0 != cbRealloc) {
-      if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) {
+      if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) {
+        /* realloc failed, pData will be freed in cleanup */
         res = YKPIV_MEMORY_ERROR;
         goto Cleanup;
       }
+      pData = pTemp;
+      pTemp = NULL;
     }
 
     cbData += cbRealloc;
@@ -1145,7 +1154,7 @@ ykpiv_rc ykpiv_util_get_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) {
 
 Cleanup:
 
-  memset(data, 0, sizeof(data));
+  yc_memzero(data, sizeof(data));
 
   _ykpiv_end_transaction(state);
   return res;
@@ -1183,7 +1192,7 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) {
     }
   }
 
-  if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) return YKPIV_PCSC_ERROR;
+  if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) { res = YKPIV_PCSC_ERROR; goto Cleanup; }
   if (YKPIV_OK != (res = _ykpiv_ensure_application_selected(state))) goto Cleanup;
 
   /* try to set the mgm key as long as we don't encounter a fatal error */
@@ -1278,8 +1287,8 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) {
 
 Cleanup:
 
-  memset(data, 0, sizeof(data));
+  yc_memzero(data, sizeof(data));
-  memset(mgm_key, 0, sizeof(mgm_key));
+  yc_memzero(mgm_key, sizeof(mgm_key));
 
   _ykpiv_end_transaction(state);
   return res;
@@ -1398,8 +1407,10 @@ static ykpiv_rc _write_certificate(ykpiv_state *state, uint8_t slot, uint8_t *da
   // calculate the required length of the encoded object
   req_len = 1 /* cert tag */ + 3 /* compression tag + data*/ + 2 /* lrc */;
   req_len += _ykpiv_set_length(buf, data_len);
+  req_len += data_len;
 
-  if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR;
+  if (req_len < data_len) return YKPIV_SIZE_ERROR; /* detect overflow of unsigned size_t */
+  if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; /* obj_size_max includes limits for TLV encoding */
 
   buf[offset++] = TAG_CERT;
   offset += _ykpiv_set_length(buf + offset, data_len);
@@ -1446,6 +1457,11 @@ static ykpiv_rc _get_metadata_item(uint8_t *data, size_t cb_data, uint8_t tag, u
 
   while (p_temp < (data + cb_data)) {
     tag_temp = *p_temp++;
+
+    if (!_ykpiv_has_valid_length(p_temp, (data + cb_data - p_temp))) {
+      return YKPIV_SIZE_ERROR;
+    }
+
     p_temp += _ykpiv_get_length(p_temp, &cb_temp);
 
     if (tag_temp == tag) {

lib/ykpiv.c

@@ -186,6 +186,19 @@ unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len) {
   return 0;
 }
 
+bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len) {
+  if ((buffer[0] < 0x81) && (len > 0)) {
+    return true;
+  }
+  else if (((*buffer & 0x7f) == 1) && (len > 1)) {
+    return true;
+  }
+  else if (((*buffer & 0x7f) == 2) && (len > 2)) {
+    return true;
+  }
+  return false;
+}
+
 static unsigned char *set_object(int object_id, unsigned char *buffer) {
   *buffer++ = 0x5c;
   if(object_id == YKPIV_OBJ_DISCOVERY) {
@@ -291,8 +304,8 @@ ykpiv_rc _ykpiv_select_application(ykpiv_state *state) {
    * can determine how to get the serial number, which for the NEO/Yk4
    * will result in another selection of the PIV applet. */
 
-  _ykpiv_get_version(state, NULL);
+  res = _ykpiv_get_version(state, NULL);
-  _ykpiv_get_serial(state, NULL, false);
+  if (res == YKPIV_OK) res = _ykpiv_get_serial(state, NULL, false);
 
   return res;
 }
@@ -684,6 +697,7 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) {
   uint32_t recv_len = sizeof(data);
   int sw;
   ykpiv_rc res;
+  des_rc drc = DES_OK;
   des_key* mgm_key = NULL;
   size_t out_len = 0;
 
@@ -728,7 +742,12 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) {
     unsigned char *dataptr = apdu.st.data;
     unsigned char response[8];
     out_len = sizeof(response);
-    des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len);
+    drc = des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len);
+
+    if (drc != DES_OK) {
+      res = YKPIV_AUTHENTICATION_ERROR;
+      goto Cleanup;
+    }
 
     recv_len = sizeof(data);
     memset(apdu.raw, 0, sizeof(apdu));
@@ -766,7 +785,13 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) {
   {
     unsigned char response[8];
     out_len = sizeof(response);
-    des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len);
+    drc = des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len);
+
+    if (drc != DES_OK) {
+      res = YKPIV_AUTHENTICATION_ERROR;
+      goto Cleanup;
+    }
+
     if (memcmp(response, data + 4, 8) == 0) {
       res = YKPIV_OK;
     }
@@ -838,6 +863,7 @@ ykpiv_rc ykpiv_set_mgmkey2(ykpiv_state *state, const unsigned char *new_key, con
   res = YKPIV_GENERIC_ERROR;
 
 Cleanup:
+  yc_memzero(&apdu, sizeof(APDU));
   _ykpiv_end_transaction(state);
   return res;
 }
@@ -1044,6 +1070,12 @@ static ykpiv_rc _ykpiv_get_version(ykpiv_state *state, ykpiv_version_t *p_versio
   if((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) {
     return res;
   } else if(sw == SW_SUCCESS) {
+
+    /* check that we received enough data for the verson number */
+    if (recv_len < 3) {
+      return YKPIV_SIZE_ERROR;
+    }
+
     state->ver.major = data[0];
     state->ver.minor = data[1];
     state->ver.patch = data[2];
@@ -1182,6 +1214,11 @@ static ykpiv_rc _ykpiv_get_serial(ykpiv_state *state, uint32_t *p_serial, bool f
     }
   }
 
+  /* check that we received enough data for the serial number */
+  if (recv_len < 4) {
+    return YKPIV_SIZE_ERROR;
+  }
+
   p_temp = (uint8_t*)(&state->serial);
 
   *p_temp++ = data[3];
@@ -1222,6 +1259,7 @@ static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len) {
     return YKPIV_OK;
   }
   if (state->pin) {
+    yc_memzero(state->pin, strnlen(state->pin, CB_PIN_MAX));
     _ykpiv_free(state, state->pin);
     state->pin = NULL;
   }
@@ -1244,7 +1282,7 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le
   int sw;
   ykpiv_rc res;
 
-  if (pin_len > 8) {
+  if (pin_len > CB_PIN_MAX) {
     return YKPIV_SIZE_ERROR;
   }
 
@@ -1255,27 +1293,36 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le
   apdu.st.lc = pin ? 0x08 : 0;
   if (pin) {
     memcpy(apdu.st.data, pin, pin_len);
-    if (pin_len < 8) {
+    if (pin_len < CB_PIN_MAX) {
-      memset(apdu.st.data + pin_len, 0xff, 8 - pin_len);
+      memset(apdu.st.data + pin_len, 0xff, CB_PIN_MAX - pin_len);
     }
   }
-  if ((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) {
+
+  res = _send_data(state, &apdu, data, &recv_len, &sw);
+  yc_memzero(&apdu, sizeof(apdu));
+
+  if (res != YKPIV_OK) {
     return res;
-  } else if (sw == SW_SUCCESS) {
+  }
+  else if (sw == SW_SUCCESS) {
     if (pin && pin_len) {
       // Intentionally ignore errors.  If the PIN fails to save, it will only
       // be a problem if a reconnect is attempted.  Failure deferred until then.
       _cache_pin(state, pin, pin_len);
     }
+
     if (tries) *tries = (sw & 0xf);
     return YKPIV_OK;
-  } else if ((sw >> 8) == 0x63) {
+  }
+  else if ((sw >> 8) == 0x63) {
     if (tries) *tries = (sw & 0xf);
     return YKPIV_WRONG_PIN;
-  } else if (sw == SW_ERR_AUTH_BLOCKED) {
+  }
+  else if (sw == SW_ERR_AUTH_BLOCKED) {
     if (tries) *tries = 0;
     return YKPIV_WRONG_PIN;
-  } else {
+  }
+  else {
     return YKPIV_GENERIC_ERROR;
   }
 }
@@ -1361,10 +1408,10 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c
   unsigned char data[0xff];
   unsigned long recv_len = sizeof(data);
   ykpiv_rc res;
-  if (current_pin_len > 8) {
+  if (current_pin_len > CB_PIN_MAX) {
     return YKPIV_SIZE_ERROR;
   }
-  if (new_pin_len > 8) {
+  if (new_pin_len > CB_PIN_MAX) {
     return YKPIV_SIZE_ERROR;
   }
   if(action == CHREF_ACT_UNBLOCK_PIN) {
@@ -1374,14 +1421,17 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c
     templ[3] = 0x81;
   }
   memcpy(indata, current_pin, current_pin_len);
-  if(current_pin_len < 8) {
+  if(current_pin_len < CB_PIN_MAX) {
-    memset(indata + current_pin_len, 0xff, 8 - current_pin_len);
+    memset(indata + current_pin_len, 0xff, CB_PIN_MAX - current_pin_len);
   }
-  memcpy(indata + 8, new_pin, new_pin_len);
+  memcpy(indata + CB_PIN_MAX, new_pin, new_pin_len);
-  if(new_pin_len < 8) {
+  if(new_pin_len < CB_PIN_MAX) {
-    memset(indata + 8 + new_pin_len, 0xff, 8 - new_pin_len);
+    memset(indata + CB_PIN_MAX + new_pin_len, 0xff, CB_PIN_MAX - new_pin_len);
   }
+
   res = ykpiv_transfer_data(state, templ, indata, sizeof(indata), data, &recv_len, &sw);
+  yc_memzero(indata, sizeof(indata));
+
   if(res != YKPIV_OK) {
     return res;
   } else if(sw != SW_SUCCESS) {
@@ -1477,8 +1527,14 @@ ykpiv_rc _ykpiv_fetch_object(ykpiv_state *state, int object_id,
   }
 
   if(sw == SW_SUCCESS) {
-    size_t outlen;
+    size_t outlen = 0;
-    unsigned int offs = _ykpiv_get_length(data + 1, &outlen);
+    unsigned int offs = 0;
+    
+    if ((*len < 2) || !_ykpiv_has_valid_length(data + 1, *len - 1)) {
+      return YKPIV_SIZE_ERROR;
+    }
+
+    offs = _ykpiv_get_length(data + 1, &outlen);
     if(offs == 0) {
       return YKPIV_SIZE_ERROR;
     }
@@ -1658,7 +1714,8 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u
     padding = elem_len - lens[i];
     remaining = (uintptr_t)key_data + sizeof(key_data) - (uintptr_t)in_ptr;
     if (padding > remaining) {
-      return YKPIV_ALGORITHM_ERROR;
+      res = YKPIV_ALGORITHM_ERROR;
+      goto Cleanup;
     }
     memset(in_ptr, 0, padding);
     in_ptr += padding;
@@ -1693,6 +1750,7 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u
   }
 
 Cleanup:
+  yc_memzero(key_data, sizeof(key_data));
   _ykpiv_end_transaction(state);
   return res;
 }
@@ -1811,6 +1869,7 @@ ykpiv_rc ykpiv_auth_verifyresponse(ykpiv_state *state, uint8_t *response, const
 
 Cleanup:
 
+  yc_memzero(&apdu, sizeof(apdu));
   _ykpiv_end_transaction(state);
   return res;
 }