Remove KeyInfo and use standard library to parse hex strings (#46)

* Remove KeyInfo and use standard library to parse hex strings

* One declaration per line, more useful name of key_from_hex_chars

src/Makefile.am

@@ -1,4 +1,4 @@
 bin_PROGRAMS = stenc
-AM_CXXFLAGS = $(INTI_CFLAGS) $(DEPS_CFLAGS)
+AM_CXXFLAGS = -std=c++17 $(INTI_CFLAGS) $(DEPS_CFLAGS)
-stenc_SOURCES = main.cpp scsiencrypt.cpp scsiencrypt.h keyinfo.h keyinfo.cpp
+stenc_SOURCES = main.cpp scsiencrypt.cpp scsiencrypt.h
 #stenc_LDADD = $(INTI_LIBS) 

src/keyinfo.cpp

@@ -1,122 +0,0 @@
-#include "keyinfo.h"
-#include "scsiencrypt.h"
-#include <config.h>
-#include <iostream>
-#include <sstream>
-#include <string.h>
-#include <string>
-
-Keyinfo::Keyinfo() {
-  valid = false;
-  check = "";
-  key = NULL;
-  keySize = 0;
-}
-void Keyinfo::load(std::string hexinput) {
-  valid = true;
-  if (hexinput.size() < 2) {
-    valid = false;
-    std::cout << "Key input too short!\n";
-    return;
-  }
-  // parse for invalid characters
-  for (unsigned int i = 0; i < hexinput.size(); i++) {
-    switch ((unsigned char)hexinput.at(i)) {
-    case '0':
-    case '1':
-    case '2':
-    case '3':
-    case '4':
-    case '5':
-    case '6':
-    case '7':
-    case '8':
-    case '9':
-    case 'a':
-    case 'b':
-    case 'c':
-    case 'd':
-    case 'e':
-    case 'f':
-    case 'A':
-    case 'B':
-    case 'C':
-    case 'D':
-    case 'E':
-    case 'F':
-      break;
-    default:
-      std::cout << "Invalid character '" << hexinput.at(i)
-                << "' found in key!\n";
-      valid = false;
-      return;
-    }
-  }
-  // delete the key if its already allocated
-  if (key != NULL)
-    delete[] key;
-  // check that the input size is divisible by 2
-  if (hexinput.size() % 2 != 0) {
-    valid = false;
-    std::cout << "Each hexadecimal byte must consist of 2 digits!\n";
-    return;
-  }
-  // convert the hex input to a char*
-  loadKey(hexinput);
-  // load the check value
-  loadCheck();
-  // check for oversized key
-  if (keySize == 0 || keySize > SSP_KEY_LENGTH) {
-    std::cout << "Key size cannot exceed " << (SSP_KEY_LENGTH * 8)
-              << " bits!\n";
-    std::cout << "Provided key is " << (keySize * 8) << " bits in length.\n";
-    valid = false;
-    return;
-  }
-  std::cout << "Provided key length is " << (keySize * 8) << " bits.\n";
-  std::cout << "Key checksum is " << check << ".\n";
-}
-void Keyinfo::loadCheck() {
-  int i;
-  int chk = 0;
-  for (i = 0; i < keySize; i++) {
-    chk += ((int)key[i]) * (i + 1);
-  }
-  std::stringstream retval;
-  retval << std::hex << chk;
-  check = retval.str();
-}
-Keyinfo::~Keyinfo() { delete[] key; }
-void Keyinfo::loadKey(std::string str) {
-  int length = str.size();
-  // make sure the input string has an even digit numbers
-  if (length % 2 == 1) {
-    str = "0" + str;
-    length++;
-  }
-
-  // allocate memory for the output array
-  key = new char[length / 2];
-  memset(key, 0, (length / 2) + 1);
-  keySize = length / 2;
-
-  std::stringstream sstr(str);
-  for (int i = 0; i < keySize; i++) {
-    char ch1, ch2;
-    sstr >> ch1 >> ch2;
-    int dig1 = 0, dig2 = 0;
-    if (isdigit(ch1))
-      dig1 = ch1 - '0';
-    else if (ch1 >= 'A' && ch1 <= 'F')
-      dig1 = ch1 - 'A' + 10;
-    else if (ch1 >= 'a' && ch1 <= 'f')
-      dig1 = ch1 - 'a' + 10;
-    if (isdigit(ch2))
-      dig2 = ch2 - '0';
-    else if (ch2 >= 'A' && ch2 <= 'F')
-      dig2 = ch2 - 'A' + 10;
-    else if (ch2 >= 'a' && ch2 <= 'f')
-      dig2 = ch2 - 'a' + 10;
-    key[i] = dig1 * 16 + dig2;
-  }
-}

src/keyinfo.h

@@ -1,20 +0,0 @@
-#ifndef KEYINFO_H
-#define KEYINFO_H
-#include <string>
-
-class Keyinfo {
-public:
-  char *key;
-  int keySize;
-  bool valid;
-  std::string check;
-  void load(std::string hexinput);
-  Keyinfo();
-  ~Keyinfo();
-
-private:
-  void loadKey(std::string str);
-  void loadCheck();
-};
-
-#endif

src/main.cpp

@@ -13,11 +13,10 @@ but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
 */
-#include "keyinfo.h"
 #include "scsiencrypt.h"
 
+#include <charconv>
 #include <config.h>
-#include <termios.h>
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
 #endif
@@ -25,10 +24,14 @@ GNU General Public License for more details.
 #include <iomanip>
 #include <ios>
 #include <iostream>
+#include <optional>
+#include <stdint.h>
 #include <sstream>
 #include <string>
 #include <sys/stat.h>
+#include <termios.h>
 #include <time.h>
+#include <vector>
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
 #endif
@@ -73,6 +76,33 @@ std::string timestamp();
 void echo(bool);
 std::ofstream logFile;
 
+static std::optional<std::vector<uint8_t>> key_from_hex_chars(const std::string& s)
+{
+  auto it = s.data();
+  std::vector<uint8_t> bytes;
+
+  if (s.size() % 2) {  // treated as if there is an implicit leading 0
+    uint8_t result;
+    auto [ptr, ec] { std::from_chars(it, it + 1, result, 16) };
+    if (ec != errc {}) {
+      return {};
+    }
+    bytes.push_back(result);
+    it = ptr;
+  }
+
+  while (*it) {
+    uint8_t result;
+    auto [ptr, ec] { std::from_chars(it, it + 2, result, 16) };
+    if (ec != errc {}) {
+      return {};
+    }
+    bytes.push_back(result);
+    it = ptr;
+  }
+  return bytes;
+}
+
 int main(int argc, char **argv) {
   bitcheck bc;
   memset(&bc, 0, 1);
@@ -262,11 +292,10 @@ int main(int argc, char **argv) {
     exit(EXIT_SUCCESS);
   }
 
-  Keyinfo ki{};
   if (drvOptions.cryptMode != CRYPTMODE_OFF) {
     if (keyFile == "") {
-      std::string p1 = "01";
+      std::string p1;
-      std::string p2 = "02";
+      std::string p2;
       bool done = false;
       while (!done) {
         std::cout << "Enter key in hex format: ";
@@ -280,17 +309,20 @@ int main(int argc, char **argv) {
         std::cout << "\n";
         if (p1 != p2) {
           std::cout << "Keys do not match!\n";
+        } else if (p1.empty()) {
+          std::cout << "Key cannot be empty!\n";
         } else {
-          ki.load(p1);
+          if (auto key_bytes = key_from_hex_chars(p1)) {
-          if (ki.valid) {
             std::cout << "Set encryption using this key? [y/n]: ";
             std::string ans = "";
             getline(std::cin, ans);
             if (ans == "y") {
+              drvOptions.cryptoKey = *key_bytes;
               done = true;
             }
-          } else
+          } else {
             std::cout << "Invalid key!\n";
+          }
         }
       }
       drvOptions.keyName = keyDesc;
@@ -303,14 +335,15 @@ int main(int argc, char **argv) {
         getline(myfile, keyInput);
         getline(myfile, keyDesc);
         myfile.close();
-        ki.load(keyInput);
+        if (auto key_bytes = key_from_hex_chars(keyInput)) {
-        if (!ki.valid)
+          drvOptions.cryptoKey = *key_bytes;
+        } else {
           errorOut("Invalid key found in '" + keyFile + "'");
+        }
         drvOptions.keyName = keyDesc;
       } else
         errorOut("Could not open '" + keyFile + "' for reading");
     }
-    drvOptions.cryptoKey.assign(ki.key, ki.keySize);
   }
 
   // Write the options to the tape device
@@ -332,10 +365,9 @@ int main(int argc, char **argv) {
     if (drvOptions.cryptMode != CRYPTMODE_OFF) {
       std::stringstream msg;
       msg << "Encryption turned on for device '" << tapeDrive << "'. ";
-      if (drvOptions.keyName.size() == 0)
+      if (!drvOptions.keyName.empty()) {
-        msg << "Key Checksum: " << ki.check;
-      else
         msg << "Key Descriptor: '" << drvOptions.keyName << "'";
+      }
       msg << " Key Instance: " << std::dec << BSLONG(opt->des.keyInstance)
           << std::endl;
 

src/scsiencrypt.cpp

@@ -222,21 +222,20 @@ int SCSIInitSDEPage(SCSIEncryptOptions *eOptions,
       break;
     default:
       byteswap((unsigned char *)options.keyLength, 2, DEFAULT_KEYSIZE);
-      eOptions->cryptoKey = ""; // blank the key
+      eOptions->cryptoKey.clear(); // blank the key
-      eOptions->keyName = ""; // blank the key name, not supported when turned off
+      eOptions->keyName.clear(); // blank the key name, not supported when turned off
       break;
   }
 
-  if (eOptions->cryptoKey != "") {
+  if (!eOptions->cryptoKey.empty()) {
     // byte swap the keylength
     byteswap((unsigned char *)&options.keyLength, 2,
              eOptions->cryptoKey.size());
     // copy the crypto key into the options
-    eOptions->cryptoKey.copy((char *)&options.keyData,
+    std::copy(eOptions->cryptoKey.begin(), eOptions->cryptoKey.end(), options.keyData);
-                             eOptions->cryptoKey.size(), 0);
   }
   // create the key descriptor
-  if (eOptions->keyName != "") {
+  if (!eOptions->keyName.empty()) {
     SSP_KAD kad;
     memset(&kad, 0, sizeof(kad));
     // set the descriptor length to the length of the keyName
@@ -381,7 +380,7 @@ bool SCSIExecute(std::string tapedrive, unsigned char *cmd_p, int cmd_len,
 
   std::cout << "SCSI Data: ";
   for (int i = 0; i < dxfer_len; i++) {
-    std::cout << HEX(dxfer_p[i]));
+    std::cout << HEX(dxfer_p[i]);
   }
   std::cout << std::endl;
 #endif
@@ -425,7 +424,7 @@ void byteswap(unsigned char *array, int size, int value) {
 SCSIEncryptOptions::SCSIEncryptOptions() {
   cryptMode = CRYPTMODE_OFF;
   algorithmIndex = DEFAULT_ALGORITHM;
-  cryptoKey = "";
+  cryptoKey = {};
   CKOD = false;
   keyName = "";
   rdmc = RDMC_DEFAULT;

src/scsiencrypt.h

@@ -53,6 +53,8 @@ GNU General Public License for more details.
 #include <sys/types.h>
 #endif
 
+#include <vector>
+
 #ifdef BYTE_ORDER
 #define STENC_BYTE_ORDER BYTE_ORDER
 #endif
@@ -343,7 +345,7 @@ public:
   bool CKOD;
   int cryptMode;
   unsigned int algorithmIndex;
-  std::string cryptoKey;
+  std::vector<uint8_t> cryptoKey;
   std::string keyName;
   SCSIEncryptOptions();
 };

tests/Makefile.am

@@ -1,4 +1,4 @@
-AM_CPPFLAGS=-I${top_srcdir}/src
+AM_CPPFLAGS=-std=c++17 -I${top_srcdir}/src
 TESTS=scsi
 check_PROGRAMS=scsi
 scsi_SOURCES=catch.hpp scsi.cpp ${top_srcdir}/src/scsiencrypt.cpp

tests/scsi.cpp

@@ -62,8 +62,12 @@ TEST_CASE("Enable encryption command", "[scsi]") {
 
   opt.cryptMode = CRYPTMODE_ON;
   opt.algorithmIndex = 1;
-  opt.cryptoKey = "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s
+  opt.cryptoKey = {
-                  "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s;
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+  };
   opt.keyName = ""s;
 
   int pagelen = SCSIInitSDEPage(&opt, buffer);
@@ -95,8 +99,12 @@ TEST_CASE("Enable encryption command with options", "[scsi]") {
   opt.CKOD = true;
   opt.cryptMode = CRYPTMODE_ON;
   opt.algorithmIndex = 1;
-  opt.cryptoKey = "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s
+  opt.cryptoKey = {
-                  "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s;
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+  };
   opt.keyName = ""s;
 
   int pagelen = SCSIInitSDEPage(&opt, buffer);
@@ -131,8 +139,12 @@ TEST_CASE("Enable encryption command with key name", "[scsi]") {
 
   opt.cryptMode = CRYPTMODE_ON;
   opt.algorithmIndex = 1;
-  opt.cryptoKey = "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s
+  opt.cryptoKey = {
-                  "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"s;
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF,
+  };
   opt.keyName = "Hello world!"s;
 
   int pagelen = SCSIInitSDEPage(&opt, buffer);