diff --git a/design-doc/3.1-advanced-overrides.md b/design-doc/3.1-advanced-overrides.md new file mode 100644 index 00000000..2105011c --- /dev/null +++ b/design-doc/3.1-advanced-overrides.md @@ -0,0 +1,124 @@ +# 3.1 — CLI overrides for advanced.properties (`-o key=value`) + +## Goal + +Let the CLI set any advanced-config key (the `engine..*` namespace and other `advanced.properties` +keys) without editing the file or juggling `JSIGNPDF_CONFIG_DIR`. Scope of this doc is **Option 1 only**: a +generic, repeatable `-o key=value` flag. (A file-based `--advanced-properties` overlay was considered and +deferred.) + +Motivating case: making the DSS engine's LT/LTA trust knobs settable inline, e.g. + +``` +jsignpdf doc.pdf -eng dss -pl LT -ts \ + -o engine.dss.online.enabled=true \ + -o engine.dss.trust.certFiles=/path/ca.pem +``` + +## Why generic (not typed per-engine flags) + +The `engine..*` namespace + `EngineConfig` abstraction exist so the core CLI never names engine +internals. A generic passthrough preserves that boundary: it works for every current and future engine key +(and non-engine keys like `font.path`) with zero per-key plumbing. Typed flags like `--dss-trust-certs` +would re-introduce exactly the coupling the engine-api split removed, so they are explicitly rejected here. + +## CLI surface + +- Short/long: **`-o` / `--option`** (both free today — verified no collision in `Constants` / `OPTS`). +- Repeatable; argument form `key=value`. +- commons-cli wiring (matches the existing `OptionBuilder` style in `SignerOptionsFromCmdLine`): + `OptionBuilder.withLongOpt(ARG_OPTION_LONG).hasArgs(2).withValueSeparator('=') + .withDescription(RES.get("hlp.option")).create(ARG_OPTION)` + then collect with `line.getOptionProperties(ARG_OPTION)` → a `Properties` of all pairs. +- Malformed entry (no `=`) → fail fast with a usage message rather than silently dropping it. +- Empty value (`-o key=`) sets the key to the empty string (distinct from "unset"; lets a user blank out a + bundled default). + +## Precedence / data model + +Highest wins: + +``` +-o overrides > user advanced.properties (config dir) > bundled defaults +``` + +`AdvancedConfig` today is `userLayer` (persisted `PropertyProvider`) over `bundledDefaults` (`Properties`), +and **every** typed accessor (`getAsBool/Int/Float`, `getNotEmptyProperty`) reads through the single +`getProperty(String)`. So the change is one transient layer checked first there: + +```java +// new, non-persisted top layer +private final Properties overrides = new Properties(); + +public synchronized String getProperty(String key) { + String v = overrides.getProperty(key); + if (v != null) return v; + v = userLayer.getProperty(key); + return v != null ? v : bundledDefaults.getProperty(key); +} + +/** CLI-supplied, in-memory only; never written back. */ +public synchronized void applyOverrides(Map kv) { overrides.putAll(kv); } +``` + +**Non-persistence is a hard requirement.** Overrides must never reach disk: they live in a separate +`Properties`, so `save()` / `userLayerSnapshot()` / the changed-key diff are untouched by construction. The +GUI never calls `applyOverrides`, so it is unaffected. + +## Wiring / timing + +1. `Constants`: add `ARG_OPTION = "o"`, `ARG_OPTION_LONG = "option"`. +2. `SignerOptionsFromCmdLine`: register the option in `loadOptions()`; in the parse path collect + `getOptionProperties(ARG_OPTION)` into a map. +3. Apply to the shared advanced store **after parse, before signing**: + `AppConfig.cfg()` → `PropertyStoreFactory.getInstance().advancedConfig()` is a singleton, and + `SignerLogic` reads `AppConfig.engineConfigFor(engine.id())` at sign time — so pushing overrides into that + instance once, right after CLI parsing, is sufficient. Expose via `AppConfig.applyAdvancedOverrides(map)` + (thin delegate to `advancedConfig().applyOverrides`). + +No change needed in `AdvancedEngineConfig` / `EngineConfig`: they already resolve through `AdvancedConfig`, +so engine-relative keys (`online.enabled`, `trust.certFiles`, …) inherit the override transparently. + +## Observability & security + +- **Log applied overrides at INFO** so a typo'd key (silently ignored, same as a typo in the file) is at + least visible: `Applied advanced override: =`. +- **Mask secrets in the log**: redact the value when the key contains `password`/`pwd` (e.g. + `engine.dss.trust.truststorePassword`). Print `=***`. +- **Document the argv caveat**: values passed via `-o` are visible in `ps` / shell history. Steer secrets to + the config file (or env) — consistent with existing guidance for `-ksp`. This is a doc note, not a code + guard. + +## Validation + +- Unknown keys are accepted (parity with hand-editing the file) but logged, so nothing fails mysteriously. +- Optional, low priority: warn when a key matches no known prefix (`engine..` or a known + app-global key). Deferred unless it proves useful. + +## Testing + +- `AdvancedConfigTest` (exists): override beats user layer beats bundled default across all typed accessors; + empty value sets empty string; `save()` and the changed-key diff ignore overrides (not persisted). +- `SignerOptionsFromCmdLineTest` (exists): multiple `-o` pairs collected; `key=val=with=eq` splits only on + the first `=` (value may contain `=`); malformed `-o keyonly` errors with usage. +- A focused test that the masking logic redacts `*password*` keys in the emitted log message. + +## Docs + +- `--help` text (`hlp.option`) in the translations bundle. +- Update the DSS engine guide (`jsignpdf.eu/docs/#pades-the-dss-engine`) and `sample.properties` with the + inline-override form; pairs naturally with the `#432` LOTL fix as a one-line repro. + +## Touch list + +- `engines/api/.../Constants.java` — new ARG constants. +- `jsignpdf/.../SignerOptionsFromCmdLine.java` — register + parse + apply. +- `engines/api/.../utils/AdvancedConfig.java` — override layer + `applyOverrides`. +- `engines/api/.../utils/AppConfig.java` — `applyAdvancedOverrides` delegate. +- translations (`hlp.option`), `sample.properties`, guide. +- tests as above. + +## Out of scope + +`--advanced-properties ` overlay (Option 2), typed per-engine flags (Option 3), and reusing +`-lp`/`-lpf` (those target the separate *basic* signer-options store, deliberately kept distinct). diff --git a/engines/api/src/main/java/net/sf/jsignpdf/Constants.java b/engines/api/src/main/java/net/sf/jsignpdf/Constants.java index da3a0348..6f7df75f 100644 --- a/engines/api/src/main/java/net/sf/jsignpdf/Constants.java +++ b/engines/api/src/main/java/net/sf/jsignpdf/Constants.java @@ -241,6 +241,9 @@ public class Constants { public static final String ARG_LOADPROPS_FILE_LONG = "load-properties-file"; public static final String ARG_LOADPROPS_FILE = "lpf"; + public static final String ARG_OPTION_LONG = "option"; + public static final String ARG_OPTION = "o"; + public static final String ARG_LIST_KS_TYPES = "lkt"; public static final String ARG_LIST_KS_TYPES_LONG = "list-keystore-types"; diff --git a/engines/api/src/main/java/net/sf/jsignpdf/utils/AdvancedConfig.java b/engines/api/src/main/java/net/sf/jsignpdf/utils/AdvancedConfig.java index 18e28ee6..f3d15a17 100644 --- a/engines/api/src/main/java/net/sf/jsignpdf/utils/AdvancedConfig.java +++ b/engines/api/src/main/java/net/sf/jsignpdf/utils/AdvancedConfig.java @@ -5,6 +5,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashSet; +import java.util.Map; import java.util.Objects; import java.util.Properties; import java.util.Set; @@ -23,6 +24,7 @@ public final class AdvancedConfig { private final PropertyProvider userLayer; private final Properties bundledDefaults; + private final Properties overrides = new Properties(); private Properties baseline; /** @@ -75,10 +77,30 @@ public synchronized Set save() throws ProperyProviderException { } public synchronized String getProperty(String key) { - String v = userLayer.getProperty(key); + String v = overrides.getProperty(key); + if (v != null) { + return v; + } + v = userLayer.getProperty(key); return v != null ? v : bundledDefaults.getProperty(key); } + /** + * Installs the given key/value pairs as the highest-priority, in-memory-only layer. Overrides win over the user file + * and the bundled defaults, and are never persisted by {@link #save()} nor reported as user overrides. Intended for + * transient CLI-supplied configuration. Entries with a {@code null} key or value are ignored. + */ + public synchronized void applyOverrides(Map kv) { + if (kv == null) { + return; + } + kv.forEach((k, v) -> { + if (k != null && v != null) { + overrides.setProperty(k, v); + } + }); + } + public synchronized String getProperty(String key, String def) { String v = getProperty(key); return v != null ? v : def; diff --git a/engines/api/src/main/java/net/sf/jsignpdf/utils/AppConfig.java b/engines/api/src/main/java/net/sf/jsignpdf/utils/AppConfig.java index d999a541..cfd10c5c 100644 --- a/engines/api/src/main/java/net/sf/jsignpdf/utils/AppConfig.java +++ b/engines/api/src/main/java/net/sf/jsignpdf/utils/AppConfig.java @@ -1,5 +1,8 @@ package net.sf.jsignpdf.utils; +import java.util.Locale; +import java.util.Map; + import net.sf.jsignpdf.Constants; import net.sf.jsignpdf.engine.AdvancedEngineConfig; import net.sf.jsignpdf.engine.EngineConfig; @@ -71,6 +74,24 @@ public static String fontEncoding() { return cfg().getNotEmptyProperty("font.encoding", null); } + /** + * Installs CLI-supplied {@code advanced.properties} overrides into the shared advanced configuration as a transient, + * highest-priority layer. Applied entries are logged at INFO with secret values masked. No-op for a {@code null} or + * empty map. + */ + public static void applyAdvancedOverrides(Map overrides) { + if (overrides == null || overrides.isEmpty()) { + return; + } + overrides.forEach((k, v) -> Constants.LOGGER.info("Applied advanced override: " + k + "=" + mask(k, v))); + cfg().applyOverrides(overrides); + } + + private static String mask(String key, String value) { + String lower = key == null ? "" : key.toLowerCase(Locale.ENGLISH); + return lower.contains("password") || lower.contains("pwd") ? "***" : value; + } + private static AdvancedConfig cfg() { return PropertyStoreFactory.getInstance().advancedConfig(); } diff --git a/engines/api/src/main/resources/net/sf/jsignpdf/conf/advanced.default.properties b/engines/api/src/main/resources/net/sf/jsignpdf/conf/advanced.default.properties index 2f9a5e12..2a37cef5 100644 --- a/engines/api/src/main/resources/net/sf/jsignpdf/conf/advanced.default.properties +++ b/engines/api/src/main/resources/net/sf/jsignpdf/conf/advanced.default.properties @@ -12,8 +12,7 @@ engine=openpdf # Engine-specific configuration. Each engine module reads its own keys under the # engine..* prefix (via EngineConfig). No keys are defined for the bundled -# OpenPDF engine; future engines (PDFBox, DSS) document their keys in their own -# design docs. +# OpenPDF engine # Path to a TTF/OTF font used to render the visible signature L2 text. # Empty value -> JSignPdf uses the bundled DejaVuSans font. diff --git a/engines/api/src/main/resources/net/sf/jsignpdf/translations/messages.properties b/engines/api/src/main/resources/net/sf/jsignpdf/translations/messages.properties index 103be235..86873ff4 100644 --- a/engines/api/src/main/resources/net/sf/jsignpdf/translations/messages.properties +++ b/engines/api/src/main/resources/net/sf/jsignpdf/translations/messages.properties @@ -231,6 +231,7 @@ hlp.listKeys=lists keys in chosen keystore hlp.listKsTypes=lists keystore types, which can be used as values -kst option hlp.loadProperties=Loads properties from a default file (created by GUI application). hlp.loadPropertiesFile=Loads properties from the given file. The file can be create by copying the default property file .JSignPdf created by the GUI in the user home directory. +hlp.option=sets an advanced.properties key for this invocation (overrides the config file). Repeatable. hlp.location=location of a signature (e.g. Washington DC). Empty by default. hlp.ocsp=enable OCSP certificate validation hlp.ocspServerUrl=default OCSP server URL, which will be used in case the signing certificate doesn't contain this information diff --git a/jsignpdf/src/main/java/net/sf/jsignpdf/SignerOptionsFromCmdLine.java b/jsignpdf/src/main/java/net/sf/jsignpdf/SignerOptionsFromCmdLine.java index 398ea131..d1d0b8fc 100644 --- a/jsignpdf/src/main/java/net/sf/jsignpdf/SignerOptionsFromCmdLine.java +++ b/jsignpdf/src/main/java/net/sf/jsignpdf/SignerOptionsFromCmdLine.java @@ -5,7 +5,9 @@ import java.io.IOException; import java.io.PrintStream; import java.net.Proxy; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; @@ -113,6 +115,8 @@ public void loadCmdLine() throws ParseException { setListKeys(line.hasOption(ARG_LIST_KEYS)); setListEngines(line.hasOption(ARG_LIST_ENGINES)); + AppConfig.applyAdvancedOverrides(parseAdvancedOverrides(line)); + // signing engine selection (CLI override of advanced.properties) if (line.hasOption(ARG_ENGINE)) setEngine(line.getOptionValue(ARG_ENGINE)); @@ -324,6 +328,22 @@ private float getFloat(Object aVal, float aDefVal) { return aDefVal; } + private Map parseAdvancedOverrides(CommandLine line) throws ParseException { + Map result = new LinkedHashMap<>(); + String[] values = line.getOptionValues(ARG_OPTION); + if (values == null) { + return result; + } + for (String raw : values) { + int eq = raw.indexOf('='); + if (eq < 1) { + throw new ParseException("Invalid -" + ARG_OPTION + " value '" + raw + "', expected key=value"); + } + result.put(raw.substring(0, eq), raw.substring(eq + 1)); + } + return result; + } + static { // reset option builder OptionBuilder.withLongOpt(ARG_HELP_LONG).create(); @@ -334,6 +354,8 @@ private float getFloat(Object aVal, float aDefVal) { .create(ARG_LOADPROPS)); OPTS.addOption(OptionBuilder.withLongOpt(ARG_LOADPROPS_FILE_LONG).withDescription(RES.get("hlp.loadPropertiesFile")) .hasArg().withArgName("file").create(ARG_LOADPROPS_FILE)); + OPTS.addOption(OptionBuilder.withLongOpt(ARG_OPTION_LONG).withDescription(RES.get("hlp.option")) + .hasArg().withArgName("key=value").create(ARG_OPTION)); OPTS.addOption(OptionBuilder.withLongOpt(ARG_LIST_KS_TYPES_LONG).withDescription(RES.get("hlp.listKsTypes")) .create(ARG_LIST_KS_TYPES)); OPTS.addOption( diff --git a/jsignpdf/src/test/java/net/sf/jsignpdf/SignerOptionsFromCmdLineTest.java b/jsignpdf/src/test/java/net/sf/jsignpdf/SignerOptionsFromCmdLineTest.java index fd6d1091..9187f210 100644 --- a/jsignpdf/src/test/java/net/sf/jsignpdf/SignerOptionsFromCmdLineTest.java +++ b/jsignpdf/src/test/java/net/sf/jsignpdf/SignerOptionsFromCmdLineTest.java @@ -332,6 +332,52 @@ public void legacyAppendFlag_stillKeepsAppendOn() throws Exception { assertTrue(f.opts.isAppend()); } + @Test + public void optionOverride_multiplePairsFlowToAdvancedConfig() throws Exception { + Fixture f = new Fixture(""); + f.opts.setCmdLine(new String[] { + "-o", "engine.cliovr.online.enabled=true", + "-o", "engine.cliovr.trust.certFiles=/path/ca.pem", + }); + f.opts.loadCmdLine(); + net.sf.jsignpdf.engine.EngineConfig cfg = net.sf.jsignpdf.utils.AppConfig.engineConfigFor("cliovr"); + assertTrue(cfg.getBoolean("online.enabled", false)); + assertEquals("/path/ca.pem", cfg.getString("trust.certFiles")); + } + + @Test + public void optionOverride_valueMayContainEquals() throws Exception { + Fixture f = new Fixture(""); + f.opts.setCmdLine(new String[] { "-o", "engine.cliovr.eq.value=a=b=c" }); + f.opts.loadCmdLine(); + assertEquals("a=b=c", net.sf.jsignpdf.utils.AppConfig.engineConfigFor("cliovr").getString("eq.value")); + } + + @Test + public void optionOverride_missingEqualsIsRejected() { + Fixture f = new Fixture(""); + f.opts.setCmdLine(new String[] { "-o", "engine.cliovr.no-separator" }); + try { + f.opts.loadCmdLine(); + fail("expected ParseException for a value without '='"); + } catch (ParseException e) { + assertTrue("message should echo the bad value, was: " + e.getMessage(), + e.getMessage().contains("engine.cliovr.no-separator")); + } + } + + @Test + public void optionOverride_emptyKeyIsRejected() { + Fixture f = new Fixture(""); + f.opts.setCmdLine(new String[] { "-o", "=orphan" }); + try { + f.opts.loadCmdLine(); + fail("expected ParseException for an empty key"); + } catch (ParseException expected) { + // ok + } + } + /** Convenience wiring: captures warnings and feeds a canned stdin reader with no Console. */ private static final class Fixture { final SignerOptionsFromCmdLine opts = new SignerOptionsFromCmdLine(); diff --git a/jsignpdf/src/test/java/net/sf/jsignpdf/utils/AdvancedConfigTest.java b/jsignpdf/src/test/java/net/sf/jsignpdf/utils/AdvancedConfigTest.java index db8442e6..a7c02ed7 100644 --- a/jsignpdf/src/test/java/net/sf/jsignpdf/utils/AdvancedConfigTest.java +++ b/jsignpdf/src/test/java/net/sf/jsignpdf/utils/AdvancedConfigTest.java @@ -9,6 +9,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; import java.util.Properties; import java.util.Set; @@ -138,4 +139,46 @@ public void getProperty_unknownKey_returnsNull() throws Exception { AdvancedConfig cfg = new AdvancedConfig(file, bundledDefaults); assertNull(cfg.getProperty("does.not.exist")); } + + @Test + public void applyOverrides_winsOverUserLayerAndBundledDefaults() throws Exception { + Path file = tmp.newFolder().toPath().resolve("advanced.properties"); + Files.writeString(file, "relax.ssl.security=false\ntsa.hashAlgorithm=SHA-256\n"); + AdvancedConfig cfg = new AdvancedConfig(file, bundledDefaults); + cfg.applyOverrides(Map.of( + "relax.ssl.security", "true", + "tsa.hashAlgorithm", "SHA-512", + "pdf2image.libraries", "pdfbox")); + assertTrue("override beats user layer", cfg.getAsBool("relax.ssl.security", false)); + assertEquals("override beats user layer", "SHA-512", cfg.getNotEmptyProperty("tsa.hashAlgorithm", null)); + assertEquals("override beats bundled default", "pdfbox", cfg.getNotEmptyProperty("pdf2image.libraries", null)); + } + + @Test + public void applyOverrides_emptyValueSetsEmptyString() throws Exception { + Path file = tmp.newFolder().toPath().resolve("advanced.properties"); + AdvancedConfig cfg = new AdvancedConfig(file, bundledDefaults); + cfg.applyOverrides(Map.of("tsa.hashAlgorithm", "")); + assertEquals("", cfg.getProperty("tsa.hashAlgorithm")); + } + + @Test + public void applyOverrides_isNotPersistedNorReportedAsUserOverride() throws Exception { + Path file = tmp.newFolder().toPath().resolve("advanced.properties"); + AdvancedConfig cfg = new AdvancedConfig(file, bundledDefaults); + cfg.applyOverrides(Map.of("relax.ssl.security", "true")); + assertFalse("transient override is not a user override", cfg.hasUserOverride("relax.ssl.security")); + Set changed = cfg.save(); + assertTrue("overrides never reach the change set", changed.isEmpty()); + assertFalse("overrides are never written to disk", + Files.exists(file) && Files.readString(file).contains("relax.ssl.security")); + } + + @Test + public void applyOverrides_nullMapIsNoOp() throws Exception { + Path file = tmp.newFolder().toPath().resolve("advanced.properties"); + AdvancedConfig cfg = new AdvancedConfig(file, bundledDefaults); + cfg.applyOverrides(null); + assertFalse("falls through to the bundled default", cfg.getAsBool("relax.ssl.security", true)); + } } diff --git a/website/docs/JSignPdf.adoc b/website/docs/JSignPdf.adoc index 028ec9ac..8a7a9f99 100644 --- a/website/docs/JSignPdf.adoc +++ b/website/docs/JSignPdf.adoc @@ -321,7 +321,8 @@ usage: jsignpdf [file1.pdf [file2.pdf ...]] [-a] [--bg-path [-ha ] [--img-path ] [-ka ] [-ki ] [-kp ] [-ksf ] [-ksp ] [-kst ] [-l ] [--l2-text ] [--l4-text ] [-le] [-lk] - [-lkt] [-llx ] [-lly ] [-lp] [-lpf ] [--ocsp] + [-lkt] [-llx ] [-lly ] [-lp] [-lpf ] [-o + ] [--ocsp] [--ocsp-server-url ] [-op ] [-opwd ] [-os ] [--overwrite] [-pe ] [-pg ] [-pr ] [--proxy-host ] [--proxy-port ] [-pl ] [--proxy-type ] [-q] [-r @@ -367,6 +368,9 @@ usage: jsignpdf [file1.pdf [file2.pdf ...]] [-a] [--bg-path | `-lpf, --load-properties-file ` | Loads properties from the given file. The file can be created by copying the default `config.properties` from the <>. +| `-o, --option ` +| Sets a single <> key for this invocation, overriding the value from the configuration file (and the bundled default). Repeatable. The value may contain `=` (only the first one is the separator). Useful for the `engine.dss.*` trust knobs -- see <>. Note: values are visible in the process list and shell history, so prefer the configuration file for secrets such as truststore passwords. + | `--enable-stdin-passwords` | Allow reading password values from standard input. When set, a password option value of `-` means "read one line from stdin" (or prompt, on an interactive console). See <>. |=== @@ -1014,12 +1018,23 @@ DSS requires a PAdES digest, so the hash algorithm must be `SHA256`, `SHA384` or If `LT`/`LTA` is requested but revocation data cannot be reached (for example, with online fetching disabled), the signing fails with a logged error rather than silently emitting a weaker level. +On the command line these keys can be set for a single run with `-o` (see <>), without editing `advanced.properties`: + +[source,shell] +---- +jsignpdf doc.pdf -eng dss -pl LT -ts \ + -o engine.dss.online.enabled=true \ + -o engine.dss.trust.certFiles=/path/to/ca.pem +---- + == Advanced application configuration Application-wide tweaks beyond the per-document signing options live in `/advanced.properties` and -- for hardware tokens -- `/pkcs11.cfg`. Both are plain text files; the easiest way to edit them is the JavaFX <>, which writes them on _OK_. Power users can also hand-edit the files directly while JSignPdf is closed; the next launch picks up the changes. `advanced.properties` covers the same six topics as the Preferences dialog (visible-signature font, certificate-validation toggles, relaxed SSL, PDF preview backend order, default TSA hash algorithm, and one or two miscellaneous knobs). When a key is missing from `advanced.properties`, JSignPdf falls back to defaults bundled inside the application jar, so a fresh install needs no config file at all. +In batch mode, any of these keys can be overridden for a single run with the `-o key=value` command-line option (see <>), without changing the file. CLI overrides take precedence over `advanced.properties` and the bundled defaults, and are not persisted. + === Migration from earlier versions In JSignPdf 2.x the same options lived in `/conf/conf.properties` and the PKCS#11 provider was pointed to by a `pkcs11config.path` key in that file. Starting with 3.0.0 those files move under your per-user <>: