Keep yaz-client authentication out of disk history

.github/workflows/build-alpine-yaz.yaml

@@ -52,7 +52,7 @@ jobs:
       - name: Show build and test failures
         if: failure()
         run: |
-          for log in config.log test/test-suite.log ztest/test-suite.log; do
+          for log in config.log test/test-suite.log client/test-suite.log ztest/test-suite.log; do
             if test -f "$log"; then
               echo "::group::$log"
               cat "$log"

.github/workflows/build-c23-yaz.yaml

@@ -48,7 +48,7 @@ jobs:
       - name: Show build and test failures
         if: failure()
         run: |
-          for log in config.log test/test-suite.log ztest/test-suite.log; do
+          for log in config.log test/test-suite.log client/test-suite.log ztest/test-suite.log; do
             if test -f "$log"; then
               echo "::group::$log"
               cat "$log"

.github/workflows/build-complete-yaz.yaml

@@ -55,7 +55,7 @@ jobs:
       - name: Show build and test failures
         if: failure()
         run: |
-          for log in config.log test/test-suite.log ztest/test-suite.log; do
+          for log in config.log test/test-suite.log client/test-suite.log ztest/test-suite.log; do
             if test -f "$log"; then
               echo "::group::$log"
               cat "$log"

.github/workflows/build-macos-yaz.yaml

@@ -65,7 +65,7 @@ jobs:
       - name: Show build and test failures
         if: failure()
         run: |
-          for log in config.log test/test-suite.log ztest/test-suite.log; do
+          for log in config.log test/test-suite.log client/test-suite.log ztest/test-suite.log; do
             if test -f "$log"; then
               echo "::group::$log"
               cat "$log"

.github/workflows/build-minimal-yaz.yaml

@@ -45,7 +45,7 @@ jobs:
       - name: Show build and test failures
         if: failure()
         run: |
-          for log in config.log test/test-suite.log ztest/test-suite.log; do
+          for log in config.log test/test-suite.log client/test-suite.log ztest/test-suite.log; do
             if test -f "$log"; then
               echo "::group::$log"
               cat "$log"

client/.gitignore

@@ -1,2 +1,4 @@
 *.raw
+*.trs
 yaz-client
+test_client_history

client/Makefile.am

@@ -4,11 +4,17 @@
 bin_PROGRAMS=yaz-client 
 EXTRA_PROGRAMS=bertorture
 
-EXTRA_DIST = default.bib
+check_PROGRAMS = test_client_history
+check_SCRIPTS = test_client_history.sh
+TESTS = $(check_PROGRAMS) $(check_SCRIPTS)
+EXTRA_DIST = default.bib $(check_SCRIPTS)
 
 COMMON=admin.c admin.h tabcomplete.c tabcomplete.h fhistory.c fhistory.h
 yaz_client_SOURCES=client.c $(COMMON)
 
+test_client_history_SOURCES = test_client_history.c $(COMMON)
+test_client_history_LDADD = ../src/libyaz.la $(READLINE_LIBS)
+
 AM_CPPFLAGS=-I$(top_srcdir)/src $(XML2_CFLAGS)
 
 yaz_client_LDADD = ../src/libyaz.la $(READLINE_LIBS) 

client/client.c

@@ -188,6 +188,7 @@ struct eoi {
 } extraOtherInfos[maxOtherInfosSupported];
 
 static void process_cmd_line(char* line);
+static int history_may_persist(const char *line);
 #if HAVE_READLINE_READLINE_H
 static char **readline_completer(char *text, int start, int end);
 #endif
@@ -839,7 +840,7 @@ static int cmd_authentication(const char *arg)
         {
             auth->which = Z_IdAuthentication_open;
             auth->u.open = args[0];
-            printf("Authentication set to Open (%s)\n", args[0]);
+            printf("Authentication set to Open (***)\n");
         }
     }
     else if (r == 2)
@@ -852,7 +853,7 @@ static int cmd_authentication(const char *arg)
         auth->u.idPass->userId = !strcmp(args[0], "-") ? 0 : args[0];
         auth->u.idPass->password = !strcmp(args[1], "-") ? 0 : args[1];
         printf("Authentication set to User (%s), Pass (%s)\n",
-               args[0], args[1]);
+               args[0], auth->u.idPass->password ? "***" : "");
     }
     else if (r == 3)
     {
@@ -864,7 +865,7 @@ static int cmd_authentication(const char *arg)
         auth->u.idPass->userId = args[0];
         auth->u.idPass->password = args[2];
         printf("Authentication set to User (%s), Group (%s), Pass (%s)\n",
-               args[0], args[1], args[2]);
+               args[0], args[1], "***");
     }
     else
     {
@@ -4365,7 +4366,7 @@ static void initialize(const char *rc_file)
 
     cmd_format("usmarc");
 
-    file_history = file_history_new();
+    file_history = file_history_new(history_may_persist);
 
     source_rc_file(rc_file);
 
@@ -4975,11 +4976,11 @@ static int cmd_list_all(const char* args)
             printf("Authentication       : IdPass\n");
             printf("    Login User       : %s\n",auth->u.idPass->userId?auth->u.idPass->userId:"");
             printf("    Login Group      : %s\n",auth->u.idPass->groupId?auth->u.idPass->groupId:"");
-            printf("    Password         : %s\n",auth->u.idPass->password?auth->u.idPass->password:"");
+            printf("    Password         : %s\n",auth->u.idPass->password?"***":"");
             break;
         case Z_IdAuthentication_open:
             printf("Authentication       : psOpen\n");
-            printf("    Open string      : %s\n",auth->u.open);
+            printf("    Open string      : ***\n");
             break;
         default:
             printf("Authentication       : Unknown\n");
@@ -5269,6 +5270,33 @@ static int cmd_register_tab(const char* arg)
     return 1;
 }
 
+/* Match the command dispatcher, including abbreviations. Inspect nested
+ * push_command arguments iteratively so they cannot persist credentials. */
+static int history_may_persist(const char *line)
+{
+    /* One readline entry must not become multiple on-disk history records. */
+    if (strchr(line, '\n'))
+        return 0;
+    while (1)
+    {
+        char word[32];
+        int i, no_read = 0;
+        sscanf(line, "%31s%n", word, &no_read);
+        if (!no_read)
+            return 1;
+        for (i = 0; cmd_array[i].cmd; i++)
+            if (!strncmp(cmd_array[i].cmd, word, strlen(word)))
+                break;
+        if (!cmd_array[i].cmd)
+            return 1;
+        if (cmd_array[i].fun == cmd_authentication)
+            return 0;
+        if (cmd_array[i].fun != cmd_push_command)
+            return 1;
+        line += no_read;
+    }
+}
+
 static void process_cmd_line(char* line)
 {
     int i, res;

client/fhistory.c

@@ -24,12 +24,16 @@
 
 struct file_history {
     WRBUF wr;
+    int (*keep)(const char *line);
+    int loaded;
 };
 
-file_history_t file_history_new()
+file_history_t file_history_new(int (*keep)(const char *line))
 {
     file_history_t fh = (file_history_t) xmalloc(sizeof(*fh));
     fh->wr = wrbuf_alloc();
+    fh->keep = keep;
+    fh->loaded = 0;
     return fh;
 }
 
@@ -45,6 +49,8 @@ void file_history_destroy(file_history_t *fhp)
 
 void file_history_add_line(file_history_t fh, const char *line)
 {
+    if (fh->keep && !fh->keep(line))
+        return;
     wrbuf_puts(fh->wr, line);
     wrbuf_puts(fh->wr, "\n");
 }
@@ -57,15 +63,29 @@ int file_history_load(file_history_t fh)
     int ret = 0;
 
     wrbuf_rewind(fh->wr);
+    fh->loaded = 0;
     yaz_snprintf(fname, sizeof(fname), "%s%s%s", homedir ? homedir : "",
             homedir ? "/" : "", ".yazclient.history");
 
     f = fopen(fname, "r");
     if (f)
     {
         int c;
+        WRBUF line = wrbuf_alloc();
+        fh->loaded = 1;
         while ((c = fgetc(f)) != EOF)
-            wrbuf_putc(fh->wr, c);
+        {
+            if (c == '\n')
+            {
+                file_history_add_line(fh, wrbuf_cstr(line));
+                wrbuf_rewind(line);
+            }
+            else
+                wrbuf_putc(line, c);
+        }
+        if (wrbuf_len(line))
+            file_history_add_line(fh, wrbuf_cstr(line));
+        wrbuf_destroy(line);
         fclose(f);
     }
     return ret;
@@ -81,7 +101,8 @@ int file_history_save(file_history_t fh)
     int ret = 0;
     size_t sz = wrbuf_len(fh->wr);
 
-    if (!sz)
+    /* Also overwrite a history file containing only filtered lines. */
+    if (!sz && !fh->loaded)
         return 0;
     yaz_snprintf(fname, sizeof(fname), "%s%s%s", homedir ? homedir : "",
             homedir ? "/" : "", ".yazclient.history");
@@ -94,7 +115,7 @@ int file_history_save(file_history_t fh)
     else
     {
         size_t w;
-        const char *start = wrbuf_buf(fh->wr);
+        const char *start = wrbuf_cstr(fh->wr);
         if (sz > FILE_SAVE_HISTORY_MAX)
         {
             const char *nl = strchr(

client/fhistory.h

@@ -38,7 +38,8 @@ YAZ_BEGIN_CDECL
 
 typedef struct file_history *file_history_t;
 
-file_history_t file_history_new(void);
+/* The optional predicate returns nonzero for lines safe to persist. */
+file_history_t file_history_new(int (*keep)(const char *line));
 void file_history_destroy(file_history_t *fhp);
 void file_history_add_line(file_history_t fh, const char *line);
 int file_history_save(file_history_t fh);

client/test_client_history.c

@@ -0,0 +1,144 @@
+/* This file is part of the YAZ toolkit.
+ * Copyright (C) Index Data
+ * See the file LICENSE for details.
+ */
+#if HAVE_CONFIG_H
+#include <config.h>
+#endif
+#include <stdio.h>
+#include <stdarg.h>
+#include <string.h>
+#include <yaz/yaz-util.h>
+#include <yaz/test.h>
+
+static WRBUF captured;
+
+static int capture_printf(const char *fmt, ...)
+{
+    char buf[4096];
+    int ret;
+    va_list ap;
+    va_start(ap, fmt);
+    ret = vsnprintf(buf, sizeof(buf), fmt, ap);
+    va_end(ap);
+    wrbuf_puts(captured, buf);
+    return ret;
+}
+
+/* Exercise the real command handlers and classifier. */
+#undef HAVE_CONFIG_H
+#define main client_main
+#define printf capture_printf
+#include "client.c"
+#undef printf
+#undef main
+
+static void collect_history(void *data, const char *line)
+{
+    WRBUF wr = (WRBUF) data;
+    wrbuf_puts(wr, line);
+    wrbuf_putc(wr, '\n');
+}
+
+static void check_history(void)
+{
+    const char *secret[] = {
+        "authentication user secret", "auth user secret", "a user secret",
+        " \t au user group secret ", "auth user/secret", "auth -",
+        "auth", "push_command auth user secret",
+        "pu a user secret", "push_command push_command auth user secret",
+        "push_command find ordinary\nauth user secret",
+        "find ordinary\na user secret", "\n auth user secret",
+        "unknown\nauth user secret", "find ordinary\nlist_all"
+    };
+    const char *ordinary[] = {
+        "", "   ", "find title", "open localhost", "attributes",
+        "push_command find title", "push_command", "list_all"
+    };
+    size_t i;
+    file_history_t fh = file_history_new(history_may_persist);
+    WRBUF lines = wrbuf_alloc();
+    WRBUF expected = wrbuf_alloc();
+    for (i = 0; i < sizeof(secret) / sizeof(*secret); i++)
+    {
+        YAZ_CHECK(!history_may_persist(secret[i]));
+        file_history_add_line(fh, secret[i]);
+    }
+    for (i = 0; i < sizeof(ordinary) / sizeof(*ordinary); i++)
+    {
+        YAZ_CHECK(history_may_persist(ordinary[i]));
+        file_history_add_line(fh, ordinary[i]);
+        collect_history(expected, ordinary[i]);
+    }
+    file_history_trav(fh, lines, collect_history);
+    YAZ_CHECK(!strcmp(wrbuf_cstr(lines), wrbuf_cstr(expected)));
+    file_history_destroy(&fh);
+    wrbuf_destroy(lines);
+    wrbuf_destroy(expected);
+
+#if HAVE_READLINE_HISTORY_H
+    /* The explicit history command must still recall and replay auth. */
+    clear_history();
+    cmd_push_command("auth user session-secret");
+    YAZ_CHECK_EQ(history_length, 1);
+    if (history_length)
+    {
+        HIST_ENTRY *entry = history_get(history_base);
+        char *line = xstrdup(entry->line);
+        YAZ_CHECK(!strcmp(line, "auth user session-secret"));
+        process_cmd_line(line);
+        YAZ_CHECK(!strcmp(auth->u.idPass->password, "session-secret"));
+        xfree(line);
+    }
+    clear_history();
+    cmd_push_command("find ordinary\nauth user multiline-secret");
+    YAZ_CHECK_EQ(history_length, 1);
+    if (history_length)
+        YAZ_CHECK(!strcmp(history_get(history_base)->line,
+                          "find ordinary\nauth user multiline-secret"));
+    clear_history();
+#endif
+}
+
+static void check_auth(const char *command, const char *secret, int open_style)
+{
+    char *line = xstrdup(command);
+    wrbuf_rewind(captured);
+    process_cmd_line(line);
+    cmd_list_all("");
+    YAZ_CHECK(!strstr(wrbuf_cstr(captured), secret));
+    YAZ_CHECK(strstr(wrbuf_cstr(captured), "***"));
+    if (open_style)
+    {
+        YAZ_CHECK_EQ(auth->which, Z_IdAuthentication_open);
+        YAZ_CHECK(!strcmp(auth->u.open, secret));
+    }
+    else
+    {
+        YAZ_CHECK_EQ(auth->which, Z_IdAuthentication_idPass);
+        YAZ_CHECK(!strcmp(auth->u.idPass->password, secret));
+    }
+    xfree(line);
+}
+
+int main(int argc, char **argv)
+{
+    YAZ_CHECK_INIT(argc, argv);
+    YAZ_CHECK_LOG();
+    captured = wrbuf_alloc();
+    nmem_auth = nmem_create();
+    check_history();
+    check_auth("auth user top-secret", "top-secret", 0);
+    check_auth("a user group another-secret", "another-secret", 0);
+    check_auth("authentication user/open-secret", "user/open-secret", 1);
+    check_auth("auth user escaped\\ secret", "escaped secret", 0);
+    cmd_authentication("user -");
+    YAZ_CHECK(!auth->u.idPass->password);
+    cmd_authentication("-");
+    YAZ_CHECK_EQ(auth->which, Z_IdAuthentication_anonymous);
+    cmd_authentication("");
+    YAZ_CHECK(!auth);
+    nmem_destroy(nmem_auth);
+    wrbuf_destroy(captured);
+    YAZ_CHECK_TERM;
+}