Merge "keyring load" command and other keyring improvements into 'development'

2025-04-07 11:08:36 +00:00 · 2013-09-07 04:22:55 +09:30 · 2013-09-07 04:22:55 +09:30 · f0a74e6cf2
commit f0a74e6cf2
parent 3be2ed615d 18e2916cec
7 changed files with 967 additions and 485 deletions
--- a/commandline.c
+++ b/commandline.c
@ -482,7 +482,7 @@ int app_echo(const struct cli_parsed *parsed, struct cli_context *context)
      DEBUGF("echo:argv[%d]=\"%s\"", i, arg);
    if (escapes) {
      unsigned char buf[strlen(arg)];
-      size_t len = str_fromprint(buf, arg);
+      size_t len = strn_fromprint(buf, sizeof buf, arg, '\0', NULL);
      cli_write(context, buf, len);
    } else
      cli_puts(context, arg);
@ -1743,8 +1743,10 @@ int app_keyring_create(const struct cli_parsed *parsed, struct cli_context *cont
 {
  if (config.debug.verbose)
    DEBUG_cli_parsed(parsed);
-  if (!keyring_open_instance())
+  keyring_file *k = keyring_open_instance();
+  if (!k)
    return -1;
+  keyring_free(k);
  return 0;
 }

@ -1760,14 +1762,61 @@ int app_keyring_dump(const struct cli_parsed *parsed, struct cli_context *contex
  if (!k)
    return -1;
  FILE *fp = path ? fopen(path, "w") : stdout;
-  if (fp == NULL)
-    return WHYF_perror("fopen(%s, \"w\")", alloca_str_toprint(path));
+  if (fp == NULL) {
+    WHYF_perror("fopen(%s, \"w\")", alloca_str_toprint(path));
+    keyring_free(k);
+    return -1;
+  }
  int ret = keyring_dump(k, XPRINTF_STDIO(fp), include_secret);
-  if (fp != stdout && fclose(fp) == EOF)
-    return WHYF_perror("fclose(%s)", alloca_str_toprint(path));
+  if (fp != stdout && fclose(fp) == EOF) {
+    WHYF_perror("fclose(%s)", alloca_str_toprint(path));
+    keyring_free(k);
+    return -1;
+  }
+  keyring_free(k);
  return ret;
 }

+int app_keyring_load(const struct cli_parsed *parsed, struct cli_context *context)
+{
+  if (config.debug.verbose)
+    DEBUG_cli_parsed(parsed);
+  const char *path;
+  if (cli_arg(parsed, "file", &path, cli_path_regular, NULL) == -1)
+    return -1;
+  unsigned pinc = 0;
+  unsigned i;
+  for (i = 0; i < parsed->labelc; ++i)
+    if (strn_str_cmp(parsed->labelv[i].label, parsed->labelv[i].len, "pin") == 0)
+      ++pinc;
+  const char *pinv[pinc];
+  unsigned pc = 0;
+  for (i = 0; i < parsed->labelc; ++i)
+    if (strn_str_cmp(parsed->labelv[i].label, parsed->labelv[i].len, "pin") == 0) {
+      assert(pc < pinc);
+      pinv[pc++] = parsed->labelv[i].text;
+    }
+  keyring_file *k = keyring_open_instance_cli(parsed);
+  if (!k)
+    return -1;
+  FILE *fp = path && strcmp(path, "-") != 0 ? fopen(path, "r") : stdin;
+  if (fp == NULL) {
+    WHYF_perror("fopen(%s, \"r\")", alloca_str_toprint(path));
+    keyring_free(k);
+    return -1;
+  }
+  if (keyring_load(k, 0, pinc, pinv, fp) == -1) {
+    keyring_free(k);
+    return -1;
+  }
+  if (keyring_commit(k) == -1) {
+    keyring_free(k);
+    return WHY("Could not write new identity");
+  }
+  keyring_free(k);
+  return 0;
+}
+
 int app_keyring_list(const struct cli_parsed *parsed, struct cli_context *context)
 {
  if (config.debug.verbose)
@ -1788,6 +1837,7 @@ int app_keyring_list(const struct cli_parsed *parsed, struct cli_context *contex
 	cli_put_string(context, name, "\n");
      }
    }
+  keyring_free(k);
  return 0;
 }

@ -2299,7 +2349,9 @@ int app_network_scan(const struct cli_parsed *parsed, struct cli_context *contex

 /* See cli_parse() for details of command specification syntax.
 */
-#define KEYRING_PIN_OPTIONS ,"[--keyring-pin=<pin>]","[--entry-pin=<pin>]..."
+#define KEYRING_PIN_OPTION	  ,"[--keyring-pin=<pin>]"
+#define KEYRING_ENTRY_PIN_OPTION  ,"[--entry-pin=<pin>]"
+#define KEYRING_PIN_OPTIONS	  KEYRING_PIN_OPTION KEYRING_ENTRY_PIN_OPTION "..."
 struct cli_schema command_line_options[]={
  {commandline_usage,{"help|-h|--help","...",NULL},CLIFLAG_PERMISSIVE_CONFIG,
   "Display command usage."},
@ -2307,7 +2359,7 @@ struct cli_schema command_line_options[]={
   "Output the supplied string."},
  {app_log,{"log","error|warn|hint|info|debug","<message>",NULL},CLIFLAG_PERMISSIVE_CONFIG,
   "Log the supplied message at given level."},
-  {app_server_start,{"start","[foreground|exec <path>]",NULL}, 0,
+  {app_server_start,{"start" KEYRING_PIN_OPTIONS, "[foreground|exec <path>]",NULL}, 0,
   "Start daemon with instance path from SERVALINSTANCE_PATH environment variable."},
  {app_server_stop,{"stop",NULL},CLIFLAG_PERMISSIVE_CONFIG,
   "Stop a running daemon with instance path from SERVALINSTANCE_PATH environment variable."},
@ -2380,6 +2432,8 @@ struct cli_schema command_line_options[]={
   "Create a new keyring file."},
  {app_keyring_dump,{"keyring","dump" KEYRING_PIN_OPTIONS,"[--secret]","[<file>]",NULL}, 0,
   "Dump all keyring identities that can be accessed using the specified PINs"},
+  {app_keyring_load,{"keyring","load" KEYRING_PIN_OPTIONS,"<file>","[<pin>]...",NULL}, 0,
+   "Load identities from the given dump text and insert them into the keyring using the specified entry PINs"},
  {app_keyring_list,{"keyring","list" KEYRING_PIN_OPTIONS,NULL}, 0,
   "List identities that can be accessed using the supplied PINs"},
  {app_keyring_add,{"keyring","add" KEYRING_PIN_OPTIONS,"[<pin>]",NULL}, 0,
--- a/keyring.c
+++ b/keyring.c
--- a/serval.h
+++ b/serval.h
@ -253,10 +253,7 @@ typedef struct keyring_file {
 void keyring_free(keyring_file *k);
 void keyring_free_context(keyring_context *c);
 void keyring_free_identity(keyring_identity *id);
-void keyring_free_keypair(keypair *kp);
-int keyring_identity_mac(keyring_context *c,keyring_identity *id,
-			 unsigned char *pkrsalt,unsigned char *mac);
-#define KEYTYPE_CRYPTOBOX 0x01
+#define KEYTYPE_CRYPTOBOX 0x01 // must be lowest
 #define KEYTYPE_CRYPTOSIGN 0x02
 #define KEYTYPE_RHIZOME 0x03
 /* DIDs aren't really keys, but the keyring is a real handy place to keep them,
@ -286,6 +283,7 @@ int keyring_commit(keyring_file *k);
 keyring_identity *keyring_create_identity(keyring_file *k,keyring_context *c, const char *pin);
 int keyring_seed(keyring_file *k);
 void keyring_identity_extract(const keyring_identity *id, const unsigned char **sidp, const char **didp, const char **namep);
+int keyring_load(keyring_file *k, int cn, unsigned pinc, const char **pinv, FILE *input);
 int keyring_dump(keyring_file *k, XPRINTF xpf, int include_secret);

 /* Make sure we have space to put bytes of the packet as we go along */
--- a/str.c
+++ b/str.c
@ -43,32 +43,67 @@ char *tohex(char *dstHex, const unsigned char *srcBinary, size_t bytes)
 }

 /* Convert nbinary*2 ASCII hex characters [0-9A-Fa-f] to nbinary bytes of data.  Can be used to
-   perform the conversion in-place, eg, fromhex(buf, (char*)buf, n);  Returns -1 if a non-hex-digit
-   character is encountered, otherwise returns the number of binary bytes produced (= nbinary).
-   @author Andrew Bettison <andrew@servalproject.com>
+ * perform the conversion in-place, eg, fromhex(buf, (char*)buf, n);  Returns -1 if a non-hex-digit
+ * character is encountered, otherwise returns the number of binary bytes produced (= nbinary).
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
 */
 size_t fromhex(unsigned char *dstBinary, const char *srcHex, size_t nbinary)
 {
-  size_t count = 0;
-  while (count != nbinary) {
-    unsigned char high = hexvalue(*srcHex++);
-    if (high & 0xf0) return -1;
-    unsigned char low = hexvalue(*srcHex++);
-    if (low & 0xf0) return -1;
-    dstBinary[count++] = (high << 4) + low;
-  }
-  return count;
+  if (strn_fromhex(dstBinary, nbinary, srcHex, NULL) == nbinary)
+    return nbinary;
+  return -1;
 }

-/* Convert nbinary*2 ASCII hex characters [0-9A-Fa-f] followed by a nul '\0' character to nbinary bytes of data.  Can be used to
-   perform the conversion in-place, eg, fromhex(buf, (char*)buf, n);  Returns -1 if a non-hex-digit
-   character is encountered or the character immediately following the last hex digit is not a nul,
-   otherwise returns zero.
-   @author Andrew Bettison <andrew@servalproject.com>
+/* Convert nbinary*2 ASCII hex characters [0-9A-Fa-f] followed by a nul '\0' character to nbinary
+ * bytes of data.  Can be used to perform the conversion in-place, eg, fromhex(buf, (char*)buf, n);
+ * Returns -1 if a non-hex-digit character is encountered or the character immediately following the
+ * last hex digit is not a nul, otherwise returns zero.
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
 */
 int fromhexstr(unsigned char *dstBinary, const char *srcHex, size_t nbinary)
 {
-  return (fromhex(dstBinary, srcHex, nbinary) == nbinary && srcHex[nbinary * 2] == '\0') ? 0 : -1;
+  const char *p;
+  if (strn_fromhex(dstBinary, nbinary, srcHex, &p) == nbinary && *p == '\0')
+    return 0;
+  return -1;
+}
+
+/* Decode pairs of ASCII hex characters [0-9A-Fa-f] into binary data with an optional upper limit on
+ * the number of binary bytes produced (destination buffer size).  Returns the number of binary
+ * bytes decoded.  If 'afterHex' is not NULL, then sets *afterHex to point to the source character
+ * immediately following the last hex digit consumed.
+ *
+ * Can be used to perform a conversion in-place, eg:
+ *
+ *    strn_fromhex((unsigned char *)buf, n, (const char *)buf, NULL);
+ *
+ * Can also be used to count hex digits without converting, eg:
+ *
+ *    strn_fromhex(NULL, -1, buf, NULL);
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstlen, const char *srcHex, const char **afterHex)
+{
+  unsigned char *dstorig = dstBinary;
+  unsigned char *dstend = dstBinary + dstlen;
+  while (dstlen == -1 || dstBinary < dstend) {
+    int high = hexvalue(srcHex[0]);
+    if (high == -1)
+      break;
+    int low = hexvalue(srcHex[1]);
+    if (low == -1)
+      break;
+    if (dstorig != NULL)
+      *dstBinary = (high << 4) + low;
+    ++dstBinary;
+    srcHex += 2;
+  }
+  if (afterHex)
+    *afterHex = srcHex;
+  return dstBinary - dstorig;
 }

 /* Does this whole buffer contain the same value? */
@ -343,37 +378,42 @@ size_t toprint_str_len(const char *srcStr, const char quotes[2])
  return srcStr ? strbuf_count(strbuf_toprint_quoted(strbuf_local(NULL, 0), quotes, srcStr)) : 4;
 }

-size_t str_fromprint(unsigned char *dst, const char *src)
+size_t strn_fromprint(unsigned char *dst, size_t dstlen, const char *src, char endquote, const char **afterp)
 {
  unsigned char *const odst = dst;
-  while (*src) {
+  unsigned char *const edst = dst + dstlen;
+  while (*src && *src != endquote && dst < edst) {
    switch (*src) {
    case '\\':
      ++src;
+      unsigned char d;
      switch (*src) {
-      case '\0': *dst++ = '\\'; break;
-      case '0': *dst++ = '\0'; ++src; break;
-      case 'n': *dst++ = '\n'; ++src; break;
-      case 'r': *dst++ = '\r'; ++src; break;
-      case 't': *dst++ = '\t'; ++src; break;
+      case '\0': d = '\\'; break;
+      case '0': d = '\0'; ++src; break;
+      case 'n': d = '\n'; ++src; break;
+      case 'r': d = '\r'; ++src; break;
+      case 't': d = '\t'; ++src; break;
      case 'x':
 	if (isxdigit(src[1]) && isxdigit(src[2])) {
 	  ++src;
-	  fromhex(dst++, src, 1);
+	  fromhex(&d, src, 1);
 	  src += 2;
 	  break;
 	}
 	// fall through
      default:
-	*dst++ = *src++;
+	d = *src++;
 	break;
      }
+      *dst++ = d;
      break;
    default:
      *dst++ = *src++;
      break;
    }
  }
+  if (afterp)
+    *afterp = src;
  return dst - odst;
 }

--- a/str.h
+++ b/str.h
@ -64,24 +64,41 @@ extern const char hexdigit[16];
 char *tohex(char *dstHex, const unsigned char *srcBinary, size_t bytes);
 size_t fromhex(unsigned char *dstBinary, const char *srcHex, size_t nbinary);
 int fromhexstr(unsigned char *dstBinary, const char *srcHex, size_t nbinary);
-int is_all_matching(const unsigned char *ptr, size_t len, unsigned char value);
-char *str_toupper_inplace(char *s);
+size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstlen, const char *src, const char **afterp);

 #define alloca_tohex(buf,len)           tohex((char *)alloca((len)*2+1), (buf), (len))

 __STR_INLINE int hexvalue(char c)
 {
-  if (c >= '0' && c <= '9') return c - '0';
-  if (c >= 'A' && c <= 'F') return c - 'A' + 10;
-  if (c >= 'a' && c <= 'f') return c - 'a' + 10;
+  switch (c) {
+  case '0': return 0;
+  case '1': return 1;
+  case '2': return 2;
+  case '3': return 3;
+  case '4': return 4;
+  case '5': return 5;
+  case '6': return 6;
+  case '7': return 7;
+  case '8': return 8;
+  case '9': return 9;
+  case 'a': case 'A': return 10;
+  case 'b': case 'B': return 11;
+  case 'c': case 'C': return 12;
+  case 'd': case 'D': return 13;
+  case 'e': case 'E': return 14;
+  case 'f': case 'F': return 15;
+  }
  return -1;
 }

+int is_all_matching(const unsigned char *ptr, size_t len, unsigned char value);
+char *str_toupper_inplace(char *s);
+
 char *toprint(char *dstStr, ssize_t dstBufSiz, const char *srcBuf, size_t srcBytes, const char quotes[2]);
 char *toprint_str(char *dstStr, ssize_t dstBufSiz, const char *srcStr, const char quotes[2]);
 size_t toprint_len(const char *srcBuf, size_t srcBytes, const char quotes[2]);
 size_t toprint_str_len(const char *srcStr, const char quotes[2]);
-size_t str_fromprint(unsigned char *dst, const char *src);
+size_t strn_fromprint(unsigned char *dst, size_t dstlen, const char *src, char endquote, const char **afterp);

 #define alloca_toprint(dstlen,buf,len)  toprint((char *)alloca((dstlen) == -1 ? toprint_len((const char *)(buf),(len), "``") + 1 : (dstlen)), (dstlen), (const char *)(buf), (len), "``")
 #define alloca_str_toprint_quoted(str, quotes)  toprint_str((char *)alloca(toprint_str_len((str), (quotes)) + 1), -1, (str), (quotes))
--- a/testframework.sh
+++ b/testframework.sh
@ -982,8 +982,6 @@ assertGrep() {
   fi
   _tfw_dump_on_fail "$1"
   _tfw_get_content "$1" || return $?
-   local s=s
-   local message
   _tfw_assert_grep "${_tfw_opt_line_msg:+$_tfw_opt_line_msg of }$1" "$_tfw_tmp/content" "$2" || _tfw_failexit
 }

@ -1222,6 +1220,7 @@ _tfw_getopts() {
   _tfw_opt_line=
   _tfw_opt_line_sed=
   _tfw_opt_line_msg=
+   _tfw_opt_grepopts=()
   _tfw_getopts_shift=0
   local oo
   _tfw_shopt oo -s extglob
@ -1241,6 +1240,7 @@ _tfw_getopts() {
      wait_until:--timeout=*) _tfw_error "invalid value: $1";;
      wait_until:--sleep=@(+([0-9])?(.+([0-9]))|*([0-9]).+([0-9]))) _tfw_opt_sleep="${1#*=}";;
      wait_until:--sleep=*) _tfw_error "invalid value: $1";;
+      *grep:--fixed-strings) _tfw_opt_grepopts+=(-F);;
      assertcontentgrep:--matches=+([0-9])) _tfw_opt_matches="${1#*=}";;
      assertcontentgrep:--matches=*) _tfw_error "invalid value: $1";; 
      assertcontent*:--line=+([0-9])) _tfw_opt_line="${1#*=}"; _tfw_opt_line_msg="line $_tfw_opt_line";;
@ -1398,7 +1398,7 @@ _tfw_assert_grep() {
      _tfw_error "$file is not readable"
      ret=$?
   else
-      local matches=$(( $($GREP --regexp="$pattern" "$file" | wc -l) + 0 ))
+      local matches=$(( $($GREP "${_tfw_opt_grepopts[@]}" --regexp="$pattern" "$file" | wc -l) + 0 ))
      local done=false
      local ret=0
      local info="$matches match"$([ $matches -ne 1 ] && echo "es")
--- a/tests/keyring
+++ b/tests/keyring
@ -25,11 +25,19 @@ shopt -s extglob

 setup() {
    setup_servald
-    executeOk_servald config \
-      set log.console.level debug \
-      set debug.keyring on
-    executeOk_servald keyring list
-    assert_keyring_list 0
+    setup_instances +A
+    set_instance +A
+}
+
+setup_instances() {
+   for arg; do
+      set_instance $arg
+      executeOk_servald config \
+         set log.console.level debug \
+         set debug.keyring on
+      executeOk_servald keyring list
+      assert_keyring_list 0
+   done
 }

 assert_keyring_list() {
@ -138,6 +146,109 @@ teardown_KeyringAutoCreate() {
   report_servald_server
 }

+doc_Load="Load keyring entries from a keyring dump"
+setup_Load() {
+   setup_servald
+   setup_instances +A +B
+   set_instance +A
+   executeOk_servald keyring add ''
+   executeOk_servald keyring add ''
+   executeOk_servald keyring dump --secret dA
+   set_instance +B
+   executeOk_servald keyring add ''
+   executeOk_servald keyring dump --secret dB
+   set_instance +A
+   tfw_cat dA dB
+   assert ! cmp dA dB
+}
+test_Load() {
+   set_instance +B
+   executeOk_servald keyring load dA
+   tfw_cat --stderr
+   executeOk_servald keyring dump --secret dBA
+   tfw_cat dBA
+   assert [ $(cat dBA | wc -l) -eq $(( $(cat dA | wc -l) + $(cat dB | wc -l) )) ]
+   while read line; do
+      assertGrep --fixed-strings dBA "${line#[0-9]}"
+   done < dA
+   while read line; do
+      assertGrep --fixed-strings dBA "${line#[0-9]}"
+   done < dB
+   set_instance +A
+   executeOk_servald keyring load dB
+   tfw_cat --stderr
+   executeOk_servald keyring dump --secret dAB
+   assert cmp dAB dBA
+}
+
+doc_LoadAtomic="Load is atomic: all entries are loaded or none"
+setup_LoadAtomic() {
+   setup_Load
+   echo blah >>dA
+}
+test_LoadAtomic() {
+   set_instance +B
+   execute --exit-status=255 $servald keyring load dA
+   executeOk_servald keyring dump --secret dB2
+   tfw_cat dB2
+   assert cmp dB2 dB
+}
+
+doc_LoadDuplicates="Load de-duplicates keyring entries by SID"
+setup_LoadDuplicates() {
+   setup
+   executeOk_servald keyring add ''
+   executeOk_servald keyring dump --secret dA
+   tfw_cat dA
+}
+test_LoadDuplicates() {
+   executeOk_servald keyring load dA
+   tfw_cat --stderr
+   executeOk_servald keyring dump --secret dAA
+   tfw_cat dAA
+   assert cmp dA dAA
+}
+
+doc_LoadPins="Load keyring entries with PIN arguments"
+setup_LoadPins() {
+   setup_servald
+   setup_instances +A +B
+   set_instance +A
+   executeOk_servald keyring add ''
+   executeOk_servald keyring add ''
+   executeOk_servald keyring add ''
+   executeOk_servald keyring dump --secret dA
+   set_instance +B
+   executeOk_servald keyring add ''
+   executeOk_servald keyring dump --secret dB
+   set_instance +A
+   tfw_cat dA dB
+   assert ! cmp dA dB
+}
+test_LoadPins() {
+   set_instance +B
+   executeOk_servald keyring load dA pin1 '' pin3
+   tfw_cat --stderr
+   for pin in '' pin1 pin3; do
+      executeOk_servald keyring dump --entry-pin="$pin" --secret dBA
+      tfw_cat --stderr dBA
+      let n=0
+      while read line; do
+         case "$pin=$line" in
+         pin1=0:* | *=1:* | pin3=2:* )
+            assertGrep --fixed-strings dBA "${line#[0-9]}"
+            let ++n
+            ;;
+         esac
+      done < dA
+      while read line; do
+         assertGrep --fixed-strings dBA "${line#[0-9]}"
+         let ++n
+      done < dB
+      assert [ $(cat dBA | wc -l) -eq $n ]
+   done
+}
+
 doc_CompatibleBack1="Can read old keyring file (1)"
 setup_CompatibleBack1() {
    setup_servald
@ -196,7 +307,7 @@ test_KeyringPathEnv() {
    assert [ "$(replayStdout)" == "$orig" ]
 }

-doc_ReadOnlyEnv="Use keyring specified by environment variable"
+doc_ReadOnlyEnv="Read-only keyring environment variable"
 setup_ReadOnlyEnv() {
    setup
    executeOk_servald keyring add ''