commit 92c4d483da9310e1f7ceb38a78949a5f8f2ff60c
parent b8a2b654c8c171fb8e72ce46fc8d8fc7b9490d11
Author: Florian Dold <dold@taler.net>
Date: Fri, 11 Sep 2026 20:31:15 +0200
secmod: use lowercase denomination key directories
Normalize RSA and CS denomination section names and migrate existing
key directories to lowercase before loading keys. Preserve key material
when only the capitalization of a configuration section changes.
Reject section names containing slashes, symlink entries, and conflicting
directory names before migration. Stop startup on migration errors.
Issue: https://bugs.taler.net/n/11239
Diffstat:
8 files changed, 524 insertions(+), 5 deletions(-)
diff --git a/src/util/meson.build b/src/util/meson.build
@@ -122,7 +122,7 @@ executable(
install: true,
)
-executable(
+secmod_rsa = executable(
'taler-exchange-secmod-rsa',
['taler-exchange-secmod-rsa.c'],
dependencies: [gnunetutil_dep, libtalerutil_dep, gcrypt_dep],
@@ -130,7 +130,7 @@ executable(
install: true,
)
-executable(
+secmod_cs = executable(
'taler-exchange-secmod-cs',
['taler-exchange-secmod-cs.c'],
dependencies: [gnunetutil_dep, libtalerutil_dep, gcrypt_dep],
@@ -156,6 +156,16 @@ talerutil_tests = [
'test_url',
]
+foreach cipher, helper : {'rsa': secmod_rsa, 'cs': secmod_cs}
+ test(
+ 'test_secmod_key_directory_@0@'.format(cipher),
+ py3,
+ args: [files('test_secmod_key_directory.py'), helper, cipher],
+ suite: ['util'],
+ timeout: 60,
+ )
+endforeach
+
talerutil_tests_installcheck = [
'test_helper_eddsa',
'test_helper_rsa',
diff --git a/src/util/secmod_common.c b/src/util/secmod_common.c
@@ -27,6 +27,206 @@
#endif
+char *
+TES_normalize_section (const char *section)
+{
+ char *result = GNUNET_strdup (section);
+
+ for (char *p = result; '\0' != *p; p++)
+ *p = (char) tolower ((unsigned char) *p);
+ return result;
+}
+
+
+/**
+ * A denomination directory to rename after checking for conflicts.
+ */
+struct KeyDirectory
+{
+ char *old_name;
+ char *new_name;
+};
+
+
+/**
+ * Closure for #check_section_name and #collect_key_directory.
+ */
+struct KeyDirectoryContext
+{
+ const char *cprefix;
+ struct KeyDirectory *dirs;
+ unsigned int num_dirs;
+ bool invalid_section_name;
+};
+
+
+/**
+ * Reject section names that would use nested key directories.
+ *
+ * @param cls a `struct KeyDirectoryContext`
+ * @param section configuration section name to check
+ */
+static void
+check_section_name (void *cls,
+ const char *section)
+{
+ struct KeyDirectoryContext *ctx = cls;
+ size_t prefix_len = strlen (ctx->cprefix);
+
+ if ( (0 != strncasecmp (section,
+ ctx->cprefix,
+ prefix_len)) ||
+ (NULL == strchr (section + prefix_len, '/')) )
+ return;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Invalid denomination section `%s': suffix must not contain '/'\n",
+ section);
+ ctx->invalid_section_name = true;
+}
+
+
+/**
+ * Collect directories requiring migration without modifying the directory
+ * being scanned. Reject symlinks and check all destinations before starting
+ * the migration.
+ *
+ * @param cls a `struct KeyDirectoryContext`
+ * @param filename directory entry to inspect
+ * @return #GNUNET_OK to continue, #GNUNET_SYSERR on a symlink, conflict or I/O error
+ */
+static enum GNUNET_GenericReturnValue
+collect_key_directory (void *cls,
+ const char *filename)
+{
+ struct KeyDirectoryContext *ctx = cls;
+ const char *base = strrchr (filename, '/') + 1;
+ struct KeyDirectory dir;
+ struct stat sb;
+ struct stat target;
+ char *normalized;
+
+ if (0 != lstat (filename, &sb))
+ {
+ GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+ "lstat",
+ filename);
+ return GNUNET_SYSERR;
+ }
+ if (S_ISLNK (sb.st_mode))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Invalid key directory entry `%s': symbolic links are not allowed\n",
+ filename);
+ return GNUNET_SYSERR;
+ }
+ if (0 != strncasecmp (base,
+ ctx->cprefix,
+ strlen (ctx->cprefix)))
+ return GNUNET_OK;
+ if (! S_ISDIR (sb.st_mode))
+ return GNUNET_OK;
+ normalized = TES_normalize_section (base);
+ if (0 == strcmp (base, normalized))
+ {
+ GNUNET_free (normalized);
+ return GNUNET_OK;
+ }
+ GNUNET_asprintf (&dir.new_name,
+ "%.*s%s",
+ (int) (base - filename),
+ filename,
+ normalized);
+ GNUNET_free (normalized);
+ for (unsigned int i = 0; i < ctx->num_dirs; i++)
+ {
+ if (0 != strcmp (dir.new_name, ctx->dirs[i].new_name))
+ continue;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Conflicting denomination key directories `%s' and `%s'; resolve their capitalization before restarting\n",
+ filename,
+ ctx->dirs[i].old_name);
+ GNUNET_free (dir.new_name);
+ return GNUNET_SYSERR;
+ }
+ if (0 == lstat (dir.new_name, &target))
+ {
+ /* On a case-insensitive filesystem both spellings may already refer to
+ the same directory. Still rename it to update the stored spelling. */
+ if ( (sb.st_dev != target.st_dev) ||
+ (sb.st_ino != target.st_ino) )
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Cannot rename denomination key directory `%s' to `%s': destination exists; resolve the conflict before restarting\n",
+ filename,
+ dir.new_name);
+ GNUNET_free (dir.new_name);
+ return GNUNET_SYSERR;
+ }
+ }
+ else if (ENOENT != errno)
+ {
+ GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+ "lstat",
+ dir.new_name);
+ GNUNET_free (dir.new_name);
+ return GNUNET_SYSERR;
+ }
+ dir.old_name = GNUNET_strdup (filename);
+ GNUNET_array_append (ctx->dirs, ctx->num_dirs, dir);
+ return GNUNET_OK;
+}
+
+
+enum GNUNET_GenericReturnValue
+TES_normalize_key_directory (const struct GNUNET_CONFIGURATION_Handle *cfg,
+ const char *keydir,
+ const char *cprefix)
+{
+ struct KeyDirectoryContext ctx = {
+ .cprefix = cprefix
+ };
+ enum GNUNET_GenericReturnValue ret = GNUNET_OK;
+
+ GNUNET_CONFIGURATION_iterate_sections (cfg,
+ &check_section_name,
+ &ctx);
+ if (ctx.invalid_section_name)
+ return GNUNET_SYSERR;
+ if (GNUNET_OK != GNUNET_DISK_directory_create (keydir))
+ return GNUNET_SYSERR;
+ if (0 > GNUNET_DISK_directory_scan (keydir,
+ &collect_key_directory,
+ &ctx))
+ ret = GNUNET_SYSERR;
+ for (unsigned int i = 0; i < ctx.num_dirs; i++)
+ {
+ struct KeyDirectory *dir = &ctx.dirs[i];
+
+ if (GNUNET_OK == ret)
+ {
+ if (0 != rename (dir->old_name, dir->new_name))
+ {
+ GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR,
+ "rename",
+ dir->old_name);
+ ret = GNUNET_SYSERR;
+ }
+ else
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Renamed denomination key directory `%s' to `%s'\n",
+ dir->old_name,
+ dir->new_name);
+ }
+ }
+ GNUNET_free (dir->old_name);
+ GNUNET_free (dir->new_name);
+ }
+ GNUNET_free (ctx.dirs);
+ return ret;
+}
+
+
/**
* Head of DLL of clients connected to us.
*/
diff --git a/src/util/secmod_common.h b/src/util/secmod_common.h
@@ -1,6 +1,6 @@
/*
This file is part of GNU Taler
- Copyright (C) 2021 Taler Systems SA
+ Copyright (C) 2021, 2026 Taler Systems SA
GNU Taler is free software; you can redistribute it and/or modify it under the
terms of the GNU General Public License as published by the Free Software
@@ -28,6 +28,35 @@
/**
+ * Convert a configuration section name to the lowercase spelling used for
+ * denomination key directories and announcements.
+ *
+ * @param section configuration section name
+ * @return newly allocated lowercase name
+ */
+char *
+TES_normalize_section (const char *section);
+
+
+/**
+ * Validate denomination section names and rename key directories to lowercase
+ * before loading any keys. Section names containing '/' and conflicting
+ * directory names are rejected before changing any directory. Entries in
+ * @a keydir must not be symbolic links.
+ *
+ * @param cfg configuration containing the denomination sections
+ * @param keydir directory containing denomination key directories
+ * @param cprefix configuration section prefix identifying denominations
+ * @return #GNUNET_OK on success, #GNUNET_SYSERR on an invalid section name,
+ * symlink, conflict or I/O error
+ */
+enum GNUNET_GenericReturnValue
+TES_normalize_key_directory (const struct GNUNET_CONFIGURATION_Handle *cfg,
+ const char *keydir,
+ const char *cprefix);
+
+
+/**
* Create the listen socket for a secmod daemon.
*
* This function is not thread-safe, as it changes and
diff --git a/src/util/secmod_cs.c b/src/util/secmod_cs.c
@@ -2202,7 +2202,7 @@ parse_denomination_cfg (const struct GNUNET_CONFIGURATION_Handle *cfg,
}
}
GNUNET_free (secname);
- denom->section = GNUNET_strdup (ct);
+ denom->section = TES_normalize_section (ct);
return GNUNET_OK;
}
@@ -2421,6 +2421,15 @@ TALER_SECMOD_cs_run (void *cls,
GNUNET_free (secname);
return;
}
+ if (GNUNET_OK !=
+ TES_normalize_key_directory (cfg,
+ keydir,
+ opt->cprefix))
+ {
+ opt->global_ret = EXIT_FAILURE;
+ GNUNET_free (secname);
+ return;
+ }
opt->global_ret = TES_listen_start (cfg,
secname,
&cb);
diff --git a/src/util/secmod_rsa.c b/src/util/secmod_rsa.c
@@ -1977,7 +1977,7 @@ parse_denomination_cfg (const struct GNUNET_CONFIGURATION_Handle *cfg,
}
GNUNET_free (secname);
denom->rsa_keysize = (unsigned int) rsa_keysize;
- denom->section = GNUNET_strdup (ct);
+ denom->section = TES_normalize_section (ct);
return GNUNET_OK;
}
@@ -2191,6 +2191,15 @@ TALER_SECMOD_rsa_run (void *cls,
GNUNET_free (secname);
return;
}
+ if (GNUNET_OK !=
+ TES_normalize_key_directory (cfg,
+ keydir,
+ opt->cprefix))
+ {
+ opt->global_ret = EXIT_FAILURE;
+ GNUNET_free (secname);
+ return;
+ }
opt->global_ret = TES_listen_start (cfg,
secname,
&cb);
diff --git a/src/util/taler-exchange-secmod-cs.conf b/src/util/taler-exchange-secmod-cs.conf
@@ -8,6 +8,10 @@
OVERLAP_DURATION = 5 m
# Where do we store the generated private keys.
+# Denomination subdirectories use lowercase section names. Existing mixed-case
+# directories are renamed on startup; conflicting names must be resolved first.
+# Denomination section names must not contain '/'.
+# Entries in KEY_DIR must not be symbolic links.
KEY_DIR = ${TALER_DATA_HOME}secmod-cs/keys
# Where does the helper listen for requests?
diff --git a/src/util/taler-exchange-secmod-rsa.conf b/src/util/taler-exchange-secmod-rsa.conf
@@ -8,6 +8,10 @@
OVERLAP_DURATION = 0 m
# Where do we store the generated private keys.
+# Denomination subdirectories use lowercase section names. Existing mixed-case
+# directories are renamed on startup; conflicting names must be resolved first.
+# Denomination section names must not contain '/'.
+# Entries in KEY_DIR must not be symbolic links.
KEY_DIR = ${TALER_DATA_HOME}secmod-rsa/keys
# Where does the helper listen for requests?
diff --git a/src/util/test_secmod_key_directory.py b/src/util/test_secmod_key_directory.py
@@ -0,0 +1,254 @@
+#!/usr/bin/env python3
+#
+# This file is part of TALER
+# Copyright (C) 2026 Taler Systems SA
+#
+# TALER is free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 3, or (at your option) any later version.
+#
+# TALER is distributed in the hope that it will be useful, 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.
+#
+# You should have received a copy of the GNU General Public License along with
+# TALER; see the file COPYING. If not, see <http://www.gnu.org/licenses/>.
+
+"""Check secmod key preservation across section capitalization changes."""
+
+import hashlib
+import os
+from pathlib import Path
+import socket
+import stat
+import struct
+import subprocess
+import sys
+import tempfile
+import time
+
+
+# 2026-01-01 00:30 UTC, safely between hourly key rotations.
+HELPER_TIME_US = 1767227400000000
+
+
+def receive(sock, size):
+ """Read a complete IPC header or payload."""
+ result = b""
+ while len(result) < size:
+ part = sock.recv(size - len(result))
+ assert part, "Helper disconnected before sending its key set"
+ result += part
+ return result
+
+
+def run_helper(binary, cipher, root, section, error=None):
+ """Start the helper, read its initial key set, and shut it down cleanly."""
+ config = root / "secmod.conf"
+ # Recompute the clock offset for every start so the helper always runs near
+ # HELPER_TIME_US, even when restarts straddle a real UTC hour boundary.
+ config.write_text(
+ f"""[{section}]
+CIPHER = {cipher.upper()}
+RSA_KEYSIZE = 1024
+DURATION_WITHDRAW = 1 hour
+ANCHOR_ROUND = 1 hour
+
+[taler-exchange-secmod-{cipher}]
+KEY_DIR = {root}/Keys
+SM_PRIV_KEY = {root}/secmod-private-key
+UNIXPATH = {root}/server.sock
+OVERLAP_DURATION = 0 s
+LOOKAHEAD_SIGN = 2 hours
+
+[testing]
+SKEW_OFFSET = {HELPER_TIME_US}
+SKEW_VARIANCE = {time.time_ns() // 1000}
+"""
+ )
+ # Supply all settings explicitly, independently of installed configuration.
+ env = dict(os.environ, TALER_BASE_CONFIG=str(root / "empty-config"))
+ with tempfile.TemporaryFile(mode="w+t") as log:
+ proc = subprocess.Popen(
+ [binary, "-c", str(config), "-w", "1", "-L", "INFO"],
+ stdout=log,
+ stderr=log,
+ env=env,
+ )
+ try:
+ if error is not None:
+ assert proc.wait(timeout=10) == 1, "Expected migration failure"
+ log.seek(0)
+ assert error in log.read(), "Missing migration failure diagnostic"
+ return None
+ with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
+ sock.settimeout(10)
+ deadline = time.monotonic() + 20
+ while True:
+ assert proc.poll() is None, "Helper exited during startup"
+ try:
+ sock.connect(str(root / "server.sock"))
+ break
+ except (FileNotFoundError, ConnectionRefusedError):
+ assert time.monotonic() < deadline, "Helper startup timed out"
+ time.sleep(0.02)
+ messages = set()
+ # Message types are defined in secmod_rsa.h and secmod_cs.h.
+ synced = 10 if cipher == "rsa" else 15
+ while True:
+ size, msg_type = struct.unpack("!HH", receive(sock, 4))
+ assert size >= 4
+ payload = receive(sock, size - 4)
+ if msg_type == synced:
+ break
+ assert msg_type == 2, "Expected a key announcement"
+ assert payload.endswith(b"coin_chf_n1\0"), "Section is not lowercase"
+ messages.add(payload)
+ assert messages, "Helper did not announce any keys"
+ proc.terminate()
+ assert proc.wait(timeout=10) == 0, "Helper did not shut down cleanly"
+ return messages
+ except BaseException:
+ log.seek(0)
+ sys.stderr.write(log.read())
+ raise
+ finally:
+ if proc.poll() is None:
+ proc.kill()
+ proc.wait()
+
+
+def snapshot(directory):
+ """Record metadata, link targets and key hashes without exposing keys."""
+ result = {}
+ for entry in directory.rglob("*"):
+ info = entry.lstat()
+ digest = None
+ if stat.S_ISREG(info.st_mode):
+ digest = hashlib.sha256(entry.read_bytes()).digest()
+ target = os.readlink(entry) if stat.S_ISLNK(info.st_mode) else None
+ result[str(entry.relative_to(directory))] = (
+ info.st_ino, info.st_mode, digest, target
+ )
+ return result
+
+
+def main():
+ binary, cipher = sys.argv[1:]
+ with tempfile.TemporaryDirectory(prefix=f"secmod-{cipher}-") as tmp:
+ root = Path(tmp)
+ (root / "empty-config").mkdir()
+ keydir = root / "Keys"
+ canonical = keydir / "coin_chf_n1"
+ legacy = keydir / "cOiN_CHF_n1"
+
+ # Mixed-case configuration must create a lowercase directory immediately.
+ announcements = run_helper(binary, cipher, root, "cOiN_CHF_n1")
+ assert {p.name for p in keydir.iterdir()} == {canonical.name}
+ keys = snapshot(canonical)
+ assert keys
+ assert min(keys).startswith("1767225600-"), "Helper clock was not controlled"
+
+ # A case-only configuration change must reload exactly the same keys.
+ assert run_helper(binary, cipher, root, "COIN_CHF_N1") == announcements
+ assert snapshot(canonical) == keys, "Restart changed the keys"
+
+ # Reject symlinks before renaming their targets, including links whose
+ # own names are already lowercase or do not have the coin_ prefix.
+ stored = keydir / "CoIn_Stored"
+ canonical.rename(stored)
+ for link, target in (
+ (canonical, stored.name),
+ (legacy, stored.name),
+ (canonical, str(stored)),
+ (canonical, "missing-directory"),
+ (canonical, canonical.name),
+ (keydir / "key-alias", stored.name),
+ ):
+ link.symlink_to(target, target_is_directory=True)
+ before = snapshot(keydir)
+ run_helper(binary, cipher, root, "coin_chf_n1",
+ error="symbolic links are not allowed")
+ assert snapshot(keydir) == before, "Symlink rejection changed key storage"
+ link.unlink()
+ stored.rename(canonical)
+
+ # Slash-containing section names must be rejected before migrating any
+ # directories or loading keys, including an old nested key layout.
+ nested = keydir / "coin_chf" / "N1"
+ nested.parent.mkdir()
+ canonical.rename(nested)
+ pending = keydir / "cOiN_Pending"
+ pending.mkdir()
+ before = snapshot(keydir)
+ for section in ("coin_chf/N1", "CoIn_CHF/N1", "coin_chf/", "coin_../escape"):
+ run_helper(binary, cipher, root, section, error="must not contain '/'")
+ assert snapshot(keydir) == before, "Invalid section changed key storage"
+ nested.rename(canonical)
+ nested.parent.rmdir()
+ pending.rmdir()
+
+ # Simulate an old installation, with a different spelling in the config.
+ canonical.rename(legacy)
+ archive = keydir / "CoIn_Archived"
+ archive.mkdir()
+ (archive / "saved-key").write_bytes(b"archived key material")
+ archived = snapshot(archive)
+ unrelated = keydir / "Keep_Mixed"
+ unrelated.mkdir()
+ assert run_helper(binary, cipher, root, "Coin_Chf_N1") == announcements
+ assert legacy.name not in {p.name for p in keydir.iterdir()}
+ assert snapshot(canonical) == keys, "Migration changed the keys"
+ assert snapshot(keydir / "coin_archived") == archived
+ assert unrelated.is_dir()
+
+ # Older timestamp-only key filenames must still migrate after the directory.
+ canonical.rename(legacy)
+ for key in legacy.iterdir():
+ key.rename(key.with_name(key.name.split("-")[0]))
+ assert run_helper(binary, cipher, root, "coin_chf_n1") == announcements
+ assert snapshot(canonical) == keys, "Legacy filename migration changed keys"
+
+ if legacy.exists():
+ print("Skipping name collision cases on a case-insensitive filesystem")
+ return
+
+ # Even an empty destination directory must not be silently overwritten.
+ canonical.rename(legacy)
+ canonical.mkdir()
+ before = snapshot(keydir)
+ run_helper(binary, cipher, root, "coin_chf_n1", error="before restarting")
+ assert snapshot(keydir) == before, "Conflict modified key storage"
+ canonical.rmdir()
+ legacy.rename(canonical)
+
+ # Detect two mixed-case directories even when the lowercase name is absent.
+ canonical.rename(legacy)
+ other = keydir / "COIN_CHF_N1"
+ other.mkdir()
+ (keydir / "coin_archived").rename(archive)
+ before = snapshot(keydir)
+ run_helper(binary, cipher, root, "Coin_Chf_N1", error="before restarting")
+ assert snapshot(keydir) == before, "Conflict partially migrated directories"
+ other.rmdir()
+
+ # A file occupying the destination must also survive a failed migration.
+ canonical.write_bytes(b"do not overwrite")
+ before = snapshot(keydir)
+ run_helper(binary, cipher, root, "coin_chf_n1", error="before restarting")
+ assert snapshot(keydir) == before, "Conflict overwrote a file"
+ canonical.unlink()
+
+ # A failed rename must stop startup instead of generating replacement keys.
+ if os.geteuid() != 0:
+ keydir.chmod(0o500)
+ try:
+ before = snapshot(keydir)
+ run_helper(binary, cipher, root, "coin_chf_n1", error="rename")
+ assert snapshot(keydir) == before, "Failed migration changed keys"
+ finally:
+ keydir.chmod(0o700)
+
+
+if __name__ == "__main__":
+ main()
