From 0c187dba2e43aede0833b292171712062eb54b3c Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Sat, 7 Nov 2026 15:04:37 +0100 Subject: [PATCH] Keep yaz-client authentication out of disk history Preserve authentication recall within the current session while excluding these commands from persistent history. Filter existing entries on load and remove them from disk on normal exit. Redact passwords and OpenStyle strings in confirmations and settings. Cover abbreviations, nested push_command entries, and history cleanup with regression tests. Document the behavior. --- .github/workflows/build-alpine-yaz.yaml | 2 +- .github/workflows/build-c23-yaz.yaml | 2 +- .github/workflows/build-complete-yaz.yaml | 2 +- .github/workflows/build-macos-yaz.yaml | 2 +- .github/workflows/build-minimal-yaz.yaml | 2 +- client/.gitignore | 2 + client/Makefile.am | 8 +- client/client.c | 40 +++++- client/fhistory.c | 29 ++++- client/fhistory.h | 3 +- client/test_client_history.c | 144 ++++++++++++++++++++++ client/test_client_history.sh | 46 +++++++ doc/yaz-client-man.xml | 17 +++ 13 files changed, 282 insertions(+), 17 deletions(-) create mode 100644 client/test_client_history.c create mode 100755 client/test_client_history.sh diff --git a/.github/workflows/build-alpine-yaz.yaml b/.github/workflows/build-alpine-yaz.yaml index 913fd5094..e28a5f8a2 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 cec6721c6..8cb92e295 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 d9f36ad55..720a546ae 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 98db50135..f1594b1b7 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 4176329dc..5e848e2da 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 d48149cbb..9cface3ad 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 e22770394..18187bc41 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 a0a60c639..80607d0a4 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 4286d1041..9b83a108c 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 25d464c6b..ce776fce0 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 000000000..cf0ddc1d0 --- /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 000000000..b54c8eab6 --- /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 61ad80ca8..9afe5b01e 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. +