Minor housecleaning: (#91)

- Replace `HEX` macro with inline function
- Replace `atoi` with `strtoul` and validate range of algorithm index
- Add back debug logging of SCSI traffic that got deleted a while ago
    * Remove logging "additional sense bytes" since it was interpreting
      the sense data wrong. Still print out the whole sense result in
      debug mode.
- Fix some compiler warnings
    * replace switch statement with if-else blocks since we don't use
      all the enum values

src/main.cpp

@@ -267,17 +267,14 @@ static void print_device_status(std::ostream& os, const scsi::page_des& opt)
        << static_cast<unsigned int>(opt.algorithm_index) << '\n';
   }
   for (const scsi::kad& kd: scsi::read_page_kads(opt)) {
-    switch (kd.type) {
-    case scsi::kad_type::ukad:
+    if (kd.type == scsi::kad_type::ukad) {
       os << std::setw(25) << "Drive Key Desc.(uKAD): ";
       os.write(reinterpret_cast<const char *>(kd.descriptor), ntohs(kd.length));
       os.put('\n');
-      break;
-    case scsi::kad_type::akad:
+    } else if (kd.type == scsi::kad_type::akad) {
       os << std::setw(25) << "Drive Key Desc.(aKAD): ";
       os.write(reinterpret_cast<const char *>(kd.descriptor), ntohs(kd.length));
       os.put('\n');
-      break;
     }
   }
 }
@@ -323,19 +320,16 @@ static void print_volume_status(std::ostream& os, const scsi::page_nbes& opt)
   case 6u << scsi::page_nbes::status_encryption_pos:
     os << "Encrypted, but unable to decrypt due to invalid key.\n";
     for (const scsi::kad& kd: read_page_kads(opt)) {
-      switch (kd.type) {
-      case scsi::kad_type::ukad:
+      if (kd.type == scsi::kad_type::ukad) {
         os << std::setw(25) << "Volume Key Desc.(uKAD): ";
         os.write(reinterpret_cast<const char *>(kd.descriptor),
                  ntohs(kd.length));
         os.put('\n');
-        break;
-      case scsi::kad_type::akad:
+      } else if (kd.type == scsi::kad_type::akad) {
         os << std::setw(25) << "Volume Key Desc.(aKAD): ";
         os.write(reinterpret_cast<const char *>(kd.descriptor),
                  ntohs(kd.length));
         os.put('\n');
-        break;
       }
     }
     if ((opt.flags & scsi::page_nbes::flags_rdmds_mask) ==
@@ -406,9 +400,17 @@ int main(int argc, char **argv)
   while ((opt_char = getopt_long(argc, argv, "+a:d:e:f:k:h", long_options,
                                  nullptr)) != -1) {
     switch (opt_char) {
-    case 'a':
-      algorithm_index = std::atoi(optarg);
-      break;
+    case 'a': {
+      char *endptr;
+      auto conv_result {std::strtoul(optarg, &endptr, 10)};
+      if (errno || *endptr ||
+          conv_result > std::numeric_limits<
+                            decltype(algorithm_index)::value_type>::max()) {
+        std::cerr << "stenc: Algorithm index " << optarg << " out of range\n";
+        std::exit(EXIT_FAILURE);
+      }
+      algorithm_index = conv_result;
+    } break;
     case 'd': {
       std::string arg {optarg};
       if (arg == "on"s) {
@@ -421,8 +423,7 @@ int main(int argc, char **argv)
         print_usage(std::cerr);
         std::exit(EXIT_FAILURE);
       }
-      break;
-    }
+    } break;
     case 'e': {
       std::string arg {optarg};
       if (arg == "on"s) {
@@ -433,8 +434,7 @@ int main(int argc, char **argv)
         print_usage(std::cerr);
         std::exit(EXIT_FAILURE);
       }
-      break;
-    }
+    } break;
     case 'f':
       tapeDrive = optarg;
       break;

src/scsiencrypt.cpp

@@ -33,11 +33,12 @@ GNU General Public License for more details.
 #if defined(OS_LINUX)
 #include <scsi/scsi.h>
 #include <scsi/sg.h>
-#define SCSI_TIMEOUT 5000
+constexpr unsigned int SCSI_TIMEOUT {5000u};
 #elif defined(OS_FREEBSD)
 #include <cam/scsi/scsi_message.h>
 #include <camlib.h>
-#define SCSI_TIMEOUT 5000
+constexpr unsigned int SCSI_TIMEOUT {5000u};
+constexpr unsigned int RETRYCOUNT {1u};
 #else
 #error "OS type is not set"
 #endif
@@ -49,8 +50,6 @@ constexpr std::uint8_t SSP_SPOUT_OPCODE = 0xb5;
 constexpr std::uint8_t SSP_SP_CMD_LEN = 12;
 constexpr std::uint8_t SSP_SP_PROTOCOL_TDE = 0x20;
 
-constexpr int RETRYCOUNT = 1;
-
 #define BSINTTOCHAR(x)                                                         \
   static_cast<std::uint8_t>((x) >> 24), static_cast<std::uint8_t>((x) >> 16),  \
       static_cast<std::uint8_t>((x) >> 8), static_cast<std::uint8_t>((x))
@@ -85,10 +84,42 @@ using unique_fd =
 
 enum class scsi_direction { to_device, from_device };
 
+struct hex {
+  std::uint8_t value;
+};
+
+inline std::ostream& operator<<(std::ostream& os, hex h)
+{
+  auto fill {os.fill('0')};
+  auto flags {os.flags(std::ios_base::hex | std::ios_base::right)};
+
+  os << std::setw(2) << static_cast<unsigned int>(h.value);
+
+  os.flags(flags);
+  os.fill(fill);
+  return os;
+}
+
 static void scsi_execute(const std::string& device, const std::uint8_t *cmd_p,
                          std::size_t cmd_len, std::uint8_t *dxfer_p,
                          std::size_t dxfer_len, scsi_direction direction)
 {
+#if defined(DEBUGSCSI)
+  std::cerr << "SCSI Command: ";
+  for (std::size_t i = 0; i < cmd_len; i++) {
+    std::cerr << hex {cmd_p[i]} << ' ';
+  }
+  std::cerr << '\n';
+
+  if (direction == scsi_direction::to_device && dxfer_len > 0u) {
+    std::cerr << "SCSI Data: ";
+    for (std::size_t i = 0; i < dxfer_len; i++) {
+      std::cerr << hex {dxfer_p[i]} << ' ';
+    }
+    std::cerr << '\n';
+  }
+#endif
+
 #if defined(OS_LINUX)
   unique_fd fd {open(device.c_str(), O_RDONLY | O_NDELAY)};
   if (!fd) {
@@ -185,6 +216,18 @@ void get_des(const std::string& device, std::uint8_t *buffer,
   };
   scsi_execute(device, spin_des_command, sizeof(spin_des_command), buffer,
                length, scsi_direction::from_device);
+
+#if defined(DEBUGSCSI)
+  std::cerr << "SCSI Response: ";
+  auto& page {reinterpret_cast<const page_des&>(*buffer)};
+  std::uint8_t *it {buffer};
+  const std::uint8_t *end {
+      buffer + std::min(length, sizeof(page_header) + ntohs(page.length))};
+  while (it < end) {
+    std::cerr << hex {*it++} << ' ';
+  }
+  std::cerr << '\n';
+#endif
 }
 
 void get_nbes(const std::string& device, std::uint8_t *buffer,
@@ -203,6 +246,18 @@ void get_nbes(const std::string& device, std::uint8_t *buffer,
   };
   scsi_execute(device, spin_nbes_command, sizeof(spin_nbes_command), buffer,
                length, scsi_direction::from_device);
+
+#if defined(DEBUGSCSI)
+  std::cerr << "SCSI Response: ";
+  auto& page {reinterpret_cast<const page_nbes&>(*buffer)};
+  std::uint8_t *it {buffer};
+  const std::uint8_t *end {
+      buffer + std::min(length, sizeof(page_header) + ntohs(page.length))};
+  while (it < end) {
+    std::cerr << hex {*it++} << ' ';
+  }
+  std::cerr << '\n';
+#endif
 }
 
 void get_dec(const std::string& device, std::uint8_t *buffer,
@@ -221,6 +276,18 @@ void get_dec(const std::string& device, std::uint8_t *buffer,
   };
   scsi_execute(device, spin_dec_command, sizeof(spin_dec_command), buffer,
                length, scsi_direction::from_device);
+
+#if defined(DEBUGSCSI)
+  std::cerr << "SCSI Response: ";
+  auto& page {reinterpret_cast<const page_dec&>(*buffer)};
+  std::uint8_t *it {buffer};
+  const std::uint8_t *end {
+      buffer + std::min(length, sizeof(page_header) + ntohs(page.length))};
+  while (it < end) {
+    std::cerr << hex {*it++} << ' ';
+  }
+  std::cerr << '\n';
+#endif
 }
 
 inquiry_data get_inquiry(const std::string& device)
@@ -232,6 +299,20 @@ inquiry_data get_inquiry(const std::string& device)
   scsi_execute(device, scsi_inq_command, sizeof(scsi_inq_command),
                reinterpret_cast<std::uint8_t *>(&inq), sizeof(inq),
                scsi_direction::from_device);
+
+#if defined(DEBUGSCSI)
+  std::cerr << "SCSI Response: ";
+  std::uint8_t *it {reinterpret_cast<std::uint8_t *>(&inq)};
+  const std::uint8_t *end {
+      reinterpret_cast<std::uint8_t *>(&inq) +
+      std::min(sizeof(inquiry_data),
+               inquiry_data::header_size + inq.additional_length)};
+  while (it < end) {
+    std::cerr << hex {*it++} << ' ';
+  }
+  std::cerr << '\n';
+#endif
+
   return inq;
 }
 
@@ -331,30 +412,21 @@ void print_sense_data(std::ostream& os, const sense_data& sd)
     break;
   }
 
-  os << " (0x" << HEX(sense_key) << ")\n";
+  os << " (0x" << hex {static_cast<std::uint8_t>(sense_key)} << ")\n";
 
   os << std::left << std::setw(25) << " ASC:"
-     << "0x" << HEX(sd.additional_sense_code) << '\n';
+     << "0x" << hex {sd.additional_sense_code} << '\n';
 
   os << std::left << std::setw(25) << " ASCQ:"
-     << "0x" << HEX(sd.additional_sense_qualifier) << '\n';
+     << "0x" << hex {sd.additional_sense_qualifier} << '\n';
 
-  if (sd.additional_sense_length > 0) {
-    os << std::left << std::setw(25) << " Additional data: "
-       << "0x";
+#if defined(DEBUGSCSI)
+  auto sense_data_length {sd.additional_sense_length + sense_data::header_size};
+  auto rawsense {reinterpret_cast<const std::uint8_t *>(&sd)};
 
-    for (int i = 0; i < sd.additional_sense_length; i++) {
-      os << HEX(sd.additional_sense_bytes[i]);
-    }
-    os << '\n';
-  }
-#ifdef DEBUGSCSI
-  os << std::left << std::setw(25) << " Raw Sense:"
-     << "0x";
-  char *rawsense = (char *)&sd;
-
-  for (int i = 0; i < sense_data::maximum_size; i++) {
-    os << HEX(rawsense[i]);
+  os << std::left << std::setw(25) << " Raw sense data:";
+  for (std::size_t i = 0; i < sense_data_length; i++) {
+    os << hex {rawsense[i]} << ' ';
   }
   os << '\n';
 #endif
@@ -371,7 +443,7 @@ read_algorithms(const page_dec& page)
   while (it < end) {
     auto elem {reinterpret_cast<const algorithm_descriptor *>(it)};
     v.push_back(std::cref(*elem));
-    it += ntohs(elem->length) + 4u; // length field + preceding 4 byte header
+    it += ntohs(elem->length) + algorithm_descriptor::header_size;
   }
   return v;
 }

src/scsiencrypt.h

@@ -36,13 +36,6 @@ GNU General Public License for more details.
 #include <sys/types.h>
 #endif
 
-constexpr std::size_t SSP_PAGE_ALLOCATION = 8192;
-
-// outputs hex in a 2 digit pair
-#define HEX(x)                                                                 \
-  std::right << std::setw(2) << std::setfill('0') << std::hex << (int)(x)      \
-             << std::setfill(' ')
-
 namespace scsi {
 
 enum class encrypt_mode : std::uint8_t {
@@ -150,6 +143,7 @@ struct __attribute__((packed)) page_des {
 };
 static_assert(sizeof(page_des) == 24u);
 
+constexpr std::size_t SSP_PAGE_ALLOCATION = 8192;
 using page_buffer = std::uint8_t[SSP_PAGE_ALLOCATION];
 
 // set data encryption page
@@ -193,10 +187,11 @@ static_assert(sizeof(page_sde) == 20u);
 
 enum class sde_rdmc : std::uint8_t {
   algorithm_default = 0u << page_sde::flags_rdmc_pos,
-  enabled = 2u << page_sde::flags_rdmc_pos,  // corresponds to --allow-raw-read
-                                             // command line option
-  disabled = 3u << page_sde::flags_rdmc_pos, // corresponds to --no-allow-raw-read command
-                                             // line option
+  enabled = 2u << page_sde::flags_rdmc_pos, // corresponds to --allow-raw-read
+                                            // command line option
+  disabled =
+      3u << page_sde::flags_rdmc_pos, // corresponds to --no-allow-raw-read
+                                      // command line option
 };
 
 // next block encryption status page
@@ -290,6 +285,8 @@ struct __attribute__((packed)) algorithm_descriptor {
   std::uint16_t maximum_eedk_size;
   std::byte reserved2[2];
   std::uint32_t security_algorithm_code;
+
+  static constexpr std::size_t header_size {4u};
 };
 static_assert(sizeof(algorithm_descriptor) == 24u);
 
@@ -326,6 +323,8 @@ struct __attribute__((packed)) inquiry_data {
   std::byte reserved1[2];
   std::uint16_t version_descriptor[8];
   std::byte reserved2[22];
+
+  static constexpr std::size_t header_size {5u};
 };
 static_assert(sizeof(inquiry_data) == 96u);
 
@@ -366,7 +365,8 @@ struct __attribute__((packed)) sense_data {
   static constexpr std::byte data_protect {7u};
   static constexpr std::byte blank_check {8u};
 
-  static constexpr auto maximum_size {252u}; // per SPC-5
+  static constexpr std::size_t header_size {8u};
+  static constexpr std::size_t maximum_size {252u}; // per SPC-5
 };
 static_assert(sizeof(sense_data) == 18u);
 
@@ -377,7 +377,7 @@ using sense_buffer = std::array<std::uint8_t, sense_data::maximum_size>;
 class scsi_error : public std::runtime_error {
 public:
   explicit scsi_error(std::unique_ptr<sense_buffer>&& buf)
-      : sense_buf {std::move(buf)}, std::runtime_error {"SCSI I/O error"}
+      : std::runtime_error {"SCSI I/O error"}, sense_buf {std::move(buf)}
   {}
   const sense_data& get_sense() const
   {
@@ -425,7 +425,8 @@ void get_dec(const std::string& device, std::uint8_t *buffer,
 std::unique_ptr<const std::uint8_t[]>
 make_sde(encrypt_mode enc_mode, decrypt_mode dec_mode,
          std::uint8_t algorithm_index, const std::vector<std::uint8_t>& key,
-         const std::string& key_name, kadf key_format, sde_rdmc rdmc, bool ckod);
+         const std::string& key_name, kadf key_format, sde_rdmc rdmc,
+         bool ckod);
 // Write set data encryption parameters to device
 void write_sde(const std::string& device, const std::uint8_t *sde_buffer);
 void print_sense_data(std::ostream& os, const sense_data& sd);