Handle errors when signing SSH certificates (#422)

common/util.c

@@ -69,11 +69,9 @@ bool read_ed25519_key(uint8_t *in, size_t in_len, uint8_t *out,
 
   uint8_t decoded[128];
   size_t decoded_len = sizeof(decoded);
-
   if (in_len < (28 + 26)) {
     return false;
   }
-
   if (memcmp(in, PEM_private_header, 28) != 0 ||
       memcmp(in + in_len - 26, PEM_private_trailer, 25) != 0) {
     return false;
@@ -95,13 +93,19 @@ bool read_ed25519_key(uint8_t *in, size_t in_len, uint8_t *out,
   BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
   BIO_push(b64, bio);
 
-  (void) BIO_write(bio, in + 28, in_len - 28 - 25);
-  (void) BIO_flush(bio);
+  if (BIO_write(bio, in + 28, in_len - 28 - 25) <= 0) {
+    BIO_free_all(b64);
+    return false;
+  }
+  if(BIO_flush(bio) != 1) {
+    BIO_free_all(b64);
+    return false;
+  }
   ret = BIO_read(b64, decoded, decoded_len);
 
   BIO_free_all(b64);
 
-  if (ret <= 0 || ret != 48) {
+  if (ret != 48) {
     return false;
   }
 
@@ -171,7 +175,10 @@ bool read_private_key(uint8_t *buf, size_t len, yh_algorithm *algo,
     return false;
   }
 
-  (void) BIO_write(bio, buf, len);
+  if(BIO_write(bio, buf, len) <= 0) {
+    BIO_free_all(bio);
+    return false;
+  }
 
   private_key = PEM_read_bio_PrivateKey(bio, NULL, NULL, /*password*/ NULL);
   BIO_free_all(bio);
@@ -376,7 +383,10 @@ bool read_public_key(uint8_t *buf, size_t len, yh_algorithm *algo,
     return false;
   }
 
-  (void) BIO_write(bio, buf, len);
+  if(BIO_write(bio, buf, len) <= 0) {
+    BIO_free_all(bio);
+    return false;
+  }
 
   EVP_PKEY *pubkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL);
   BIO_free_all(bio);
@@ -430,7 +440,7 @@ bool read_public_key(uint8_t *buf, size_t len, yh_algorithm *algo,
     return false;
   }
 
-  (void) i2o_ECPublicKey(ec, &bytes);
+  i2o_ECPublicKey(ec, &bytes);
   EC_KEY_free(ec);
 
   *bytes_len = data_len;
@@ -643,8 +653,14 @@ bool base64_decode(const char *in, uint8_t *out, size_t *len) {
   BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
   BIO_push(b64, bio);
 
-  (void) BIO_write(bio, in, strlen(in));
-  (void) BIO_flush(bio);
+  if(BIO_write(bio, in, strlen(in)) <= 0) {
+    BIO_free_all(b64);
+    return false;
+  }
+  if(BIO_flush(bio) != 1) {
+    BIO_free_all(b64);
+    return false;
+  }
   ret = BIO_read(b64, out, *len);
 
   BIO_free_all(b64);
@@ -680,10 +696,17 @@ bool write_file(const uint8_t *buf, size_t buf_len, FILE *fp, format_t format) {
     }
     bio = BIO_push(b64, bio);
 
-    (void) BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
-    (void) BIO_write(bio, buf, buf_len);
-    (void) BIO_flush(bio);
-    (void) BIO_get_mem_ptr(bio, &bufferPtr);
+    BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
+    if(BIO_write(bio, buf, buf_len) <= 0) {
+      BIO_free_all(bio);
+      return false;
+    }
+    if(BIO_flush(bio) != 1) {
+      BIO_free_all(bio);
+      return false;
+    }
+    BIO_get_mem_ptr(bio, &bufferPtr);
+
     p = (uint8_t *) bufferPtr->data;
     length = bufferPtr->length;
   } else if (format == _hex) {

examples/ssh.c

@@ -280,10 +280,10 @@ int main(void) {
   bio = BIO_push(b64, bio);
 
   fprintf(stdout, "[email protected] ");
-  (void) BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
-  (void) BIO_write(bio, ssh_req + 4 + 256,
-                   ssh_req_len + ssh_cert_len - 4 - 256);
-  (void) BIO_flush(bio);
+  BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
+  assert(BIO_write(bio, ssh_req + 4 + 256,
+                   ssh_req_len + ssh_cert_len - 4 - 256) > 0);
+  assert(BIO_flush(bio) == 1);
   fprintf(stdout, "\n");
 
   BIO_free_all(bio);

lib/yubihsm.c

@@ -74,6 +74,8 @@ uint8_t _yh_verbosity YH_INTERNAL = 0;
 FILE *_yh_output YH_INTERNAL = NULL;
 #endif
 
+#define UNUSED(x) (void) (x);
+
 static yh_rc compute_full_mac_ex(const uint8_t *data, uint16_t data_len,
                                  aes_context *aes_ctx, uint8_t *mac) {
 
@@ -4666,7 +4668,7 @@ yh_rc yh_init(void) {
 static yh_rc load_backend(const char *name,
                           void **backend,
                           struct backend_functions **bf) {
-  (void)backend;
+  UNUSED(backend);
   if (name == NULL) {
     DBG_ERR("No name given to load_backend");
     return YHR_GENERIC_ERROR;

lib/yubihsm_usb.c

@@ -28,6 +28,8 @@
 #include "yubihsm_usb.h"
 #include "debug_lib.h"
 
+#define UNUSED(x) (void) (x)
+
 #ifndef STATIC
 uint8_t YH_INTERNAL _yh_verbosity;
 FILE YH_INTERNAL *_yh_output;
@@ -51,7 +53,7 @@ static yh_rc backend_connect(yh_connector *connector, int timeout) {
   yh_rc ret = YHR_CONNECTOR_ERROR;
   yh_backend *backend = NULL;
 
-  (void) timeout;
+  UNUSED(timeout);
 
   if (parse_usb_url(connector->api_url, &serial) == false) {
     DBG_ERR("Failed to parse URL: '%s'", connector->api_url);
@@ -82,7 +84,7 @@ static yh_rc backend_send_msg(yh_backend *connection, Msg *msg, Msg *response,
   yh_rc ret = YHR_GENERIC_ERROR;
   unsigned long read_len = 0;
 
-  (void) identifier;
+  UNUSED(identifier);
 
   for (int i = 0; i <= 1; i++) {
     if (ret != YHR_GENERIC_ERROR) {
@@ -131,9 +133,9 @@ static void backend_cleanup(void) { DBG_INFO("backend_cleanup"); }
 
 static yh_rc backend_option(yh_backend *connection, yh_connector_option opt,
                             const void *val) {
-  (void) connection;
-  (void) opt;
-  (void) val;
+  UNUSED(connection);
+  UNUSED(opt);
+  UNUSED(val);
 
   DBG_ERR("Backend options not (yet?) supported for USB");
   return YHR_CONNECTOR_ERROR;

lib/yubihsm_winhttp.c

@@ -297,9 +297,9 @@ static yh_rc backend_send_msg(yh_backend *backend, Msg *msg, Msg *response,
 
 static yh_rc backend_option(yh_backend *connection, yh_connector_option opt,
                             const void *val) {
-  (void) connection;
-  (void) opt;
-  (void) val;
+  UNUSED(connection);
+  UNUSED(opt);
+  UNUSED(val);
 
   DBG_ERR("Backend options not (yet?) supported with winhttp");
   return YHR_CONNECTOR_ERROR;

pkcs11/util_pkcs11.c

@@ -3813,7 +3813,7 @@ void verify_mechanism_cleanup(yubihsm_pkcs11_op_info *op_info) {
 
 void decrypt_mechanism_cleanup(yubihsm_pkcs11_op_info *op_info) {
 
-  (void) op_info;
+  UNUSED(op_info);
 }
 
 void digest_mechanism_cleanup(yubihsm_pkcs11_op_info *op_info) {

src/commands.c

@@ -273,7 +273,7 @@ int yh_com_connect(yubihsm_context *ctx, Argument *argv, cmd_format in_fmt,
     }
     yrc = yh_connect(ctx->connector, 0);
     if (yrc == YHR_SUCCESS) {
-      (void) yh_com_keepalive_on(NULL, NULL, fmt_nofmt, fmt_nofmt);
+      yh_com_keepalive_on(NULL, NULL, fmt_nofmt, fmt_nofmt);
       return 0;
     }
     fprintf(stderr, "Failed connecting '%s': %s\n", ctx->connector_list[i],
@@ -1158,18 +1158,21 @@ int yh_com_get_pubkey(yubihsm_context *ctx, Argument *argv, cmd_format in_fmt,
     BIO *bio = BIO_new_fp(ctx->out, BIO_NOCLOSE);
     if (bio == NULL) {
       fprintf(stderr, "Unable to allocate BIO\n");
-      BIO_free_all(b64);
       error = true;
       goto getpk_base64_cleanup;
     }
 
     bio = BIO_push(b64, bio);
 
-    (void) i2d_PUBKEY_bio(bio, public_key);
+    i2d_PUBKEY_bio(bio, public_key);
 
-    (void) BIO_flush(bio);
-    (void) BIO_free_all(bio);
+    if (BIO_flush(bio) != 1) {
+      fprintf(stderr, "Unable to flush BIO\n");
+      error = true;
+      goto getpk_base64_cleanup;
+    }
   getpk_base64_cleanup:
+    BIO_free_all(b64);
     if (error) {
       EVP_PKEY_free(public_key);
       return -1;
@@ -1258,10 +1261,15 @@ int yh_com_get_device_pubkey(yubihsm_context *ctx, Argument *argv,
 
     bio = BIO_push(b64, bio);
 
-    (void) i2d_PUBKEY_bio(bio, public_key);
+    i2d_PUBKEY_bio(bio, public_key);
 
-    (void) BIO_flush(bio);
-    (void) BIO_free_all(bio);
+    if (BIO_flush(bio) != 1) {
+      fprintf(stderr, "Unable to flush BIO\n");
+      BIO_free_all(b64);
+      error = true;
+      goto getdpk_base64_cleanup;
+    }
+    BIO_free_all(bio);
   getdpk_base64_cleanup:
     if (error) {
       EVP_PKEY_free(public_key);
@@ -2467,7 +2475,11 @@ static bool read_rsa_pubkey(const uint8_t *buf, size_t len,
   if ((bio = BIO_new(BIO_s_mem())) == NULL)
     return false;
 
-  (void) BIO_write(bio, buf, len);
+  if(BIO_write(bio, buf, len) <= 0) {
+    fprintf(stderr, "%s: Failed to read RSA public key\n", __func__);
+    BIO_free_all(bio);
+    return false;
+  }
 
   RSA *rsa = NULL;
   EVP_PKEY *pubkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL);
@@ -3136,15 +3148,21 @@ int yh_com_sign_ssh_certificate(yubihsm_context *ctx, Argument *argv,
   }
   bio = BIO_push(b64, bio);
 
+  int ret = 0;
   int cert_len = argv[4].len - certdata_offset + response_len;
   BUF_MEM *bufferPtr = 0;
 
   BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
   if (BIO_write(bio, data + certdata_offset, cert_len) != cert_len) {
-        fprintf(stderr, "Failed to write SSH certificate.\n");
-        return -1;
+    fprintf(stderr, "Failed to write SSH certificate.\n");
+    ret = -1;
+    goto clean_bio;
+  }
+  if (BIO_flush(bio) != 1) {
+    fprintf(stderr, "Failed to sign SSH certificate.\n");
+    ret = -1;
+    goto clean_bio;
   }
-  BIO_flush(bio);
   BIO_get_mem_ptr(bio, &bufferPtr);
 
   const char *ssh_cert_str =
@@ -3154,24 +3172,27 @@ int yh_com_sign_ssh_certificate(yubihsm_context *ctx, Argument *argv,
         strlen(ssh_cert_str) ||
       ferror(ctx->out)) {
     fprintf(stderr, "Unable to write data to file\n");
-    return -1;
+    ret = -1;
+    goto clean_bio;
   }
 
   if (fwrite(bufferPtr->data, 1, bufferPtr->length, ctx->out) !=
         bufferPtr->length ||
       ferror(ctx->out)) {
     fprintf(stderr, "Unable to write data to file\n");
-    return -1;
+    ret = -1;
+    goto clean_bio;
   }
 
   if (fwrite("\n", 1, 1, ctx->out) != 1 || ferror(ctx->out)) {
     fprintf(stderr, "Unable to write data to file\n");
-    return -1;
+    ret = -1;
   }
 
-  (void) BIO_free_all(bio); // TODO: fix this leak.
+clean_bio:
+  BIO_free_all(bio);
 
-  return 0;
+  return ret;
 }
 
 static void time_elapsed(struct timeval *after, struct timeval *before,