diff --git a/.github/workflows/build-alpine-yaz.yaml b/.github/workflows/build-alpine-yaz.yaml index 913fd509..e28a5f8a 100644 --- a/.github/workflows/build-alpine-yaz.yaml +++ b/.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" diff --git a/.github/workflows/build-c23-yaz.yaml b/.github/workflows/build-c23-yaz.yaml index cec6721c..8cb92e29 100644 --- a/.github/workflows/build-c23-yaz.yaml +++ b/.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" diff --git a/.github/workflows/build-complete-yaz.yaml b/.github/workflows/build-complete-yaz.yaml index d9f36ad5..720a546a 100644 --- a/.github/workflows/build-complete-yaz.yaml +++ b/.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" diff --git a/.github/workflows/build-macos-yaz.yaml b/.github/workflows/build-macos-yaz.yaml index 98db5013..f1594b1b 100644 --- a/.github/workflows/build-macos-yaz.yaml +++ b/.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" diff --git a/.github/workflows/build-minimal-yaz.yaml b/.github/workflows/build-minimal-yaz.yaml index 4176329d..5e848e2d 100644 --- a/.github/workflows/build-minimal-yaz.yaml +++ b/.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" diff --git a/client/.gitignore b/client/.gitignore index d48149cb..9cface3a 100644 --- a/client/.gitignore +++ b/client/.gitignore @@ -1,2 +1,4 @@ *.raw +*.trs yaz-client +test_client_history diff --git a/client/Makefile.am b/client/Makefile.am index e2277039..18187bc4 100644 --- a/client/Makefile.am +++ b/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) diff --git a/client/client.c b/client/client.c index a0a60c63..80607d0a 100644 --- a/client/client.c +++ b/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; diff --git a/client/fhistory.c b/client/fhistory.c index 4286d104..9b83a108 100644 --- a/client/fhistory.c +++ b/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,6 +63,7 @@ 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"); @@ -64,8 +71,21 @@ int file_history_load(file_history_t fh) 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( diff --git a/client/fhistory.h b/client/fhistory.h index 25d464c6..ce776fce 100644 --- a/client/fhistory.h +++ b/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); diff --git a/client/test_client_history.c b/client/test_client_history.c new file mode 100644 index 00000000..cf0ddc1d --- /dev/null +++ b/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 +#endif +#include +#include +#include +#include +#include + +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; +} diff --git a/client/test_client_history.sh b/client/test_client_history.sh new file mode 100755 index 00000000..b54c8eab --- /dev/null +++ b/client/test_client_history.sh @@ -0,0 +1,46 @@ +#!/bin/sh +# This file is part of the YAZ toolkit. +# Copyright (C) Index Data +# See the file LICENSE for details. + +set -eu +history_dir=$(mktemp -d "${TMPDIR:-/tmp}/yaz-history.XXXXXX") +trap 'rm -rf "$history_dir"' EXIT +trap 'exit 1' HUP INT TERM +: > "$history_dir/rc" + +run_client() { + env HOME="$history_dir" ./yaz-client -f "$history_dir/rc" \ + "$history_dir/output" 2>&1 || { + result=$? + cat "$history_dir/output" + return "$result" + } +} + +# Previously persisted credentials must disappear, including abbreviations, +# nested history commands, and a final line without a newline. +printf '%s\n' 'find ordinary' 'auth user secret' 'a user other-secret' \ + 'push_command pu auth user nested-secret' 'list_all' \ + > "$history_dir/.yazclient.history" +printf '%s' 'authentication user/final-secret' \ + >> "$history_dir/.yazclient.history" +printf '%s\n' 'find ordinary' 'list_all' > "$history_dir/expected" +run_client +cmp "$history_dir/expected" "$history_dir/.yazclient.history" + +# An unused client with no previous history need not create a file. +rm "$history_dir/.yazclient.history" +run_client +test ! -e "$history_dir/.yazclient.history" + +# An entirely filtered file must be truncated, not left untouched. +printf '%s\n' 'auth user secret' > "$history_dir/.yazclient.history" +run_client +test ! -s "$history_dir/.yazclient.history" + +# Preserve an ordinary final line even when the old file lacks a newline. +printf '%s' 'find ordinary' > "$history_dir/.yazclient.history" +printf '%s\n' 'find ordinary' > "$history_dir/expected" +run_client +cmp "$history_dir/expected" "$history_dir/.yazclient.history" diff --git a/doc/yaz-client-man.xml b/doc/yaz-client-man.xml index 61ad80ca..9afe5b01 100644 --- a/doc/yaz-client-man.xml +++ b/doc/yaz-client-man.xml @@ -439,6 +439,23 @@ issued and the Z39.50 Initialize Request is sent, so this command must be used before open in order to be effective. + + Authentication commands remain available for recall with the up arrow + during the current session when readline is enabled, but are not saved + in $HOME/.yazclient.history. This also applies to + abbreviated commands and authentication commands inside + push_command. Entries containing embedded newlines + are likewise kept only in session history. Previously saved authentication + commands are discarded when history is loaded and removed from the + history file when the client exits normally. + + + Passwords and OpenStyle authentication strings are displayed as + *** in confirmations and settings output. Credentials + remain visible when typing or recalling a command. This does not redact + authentication from explicitly enabled protocol logs or existing copies + of history files. +