Skip to content

Diagnostics: prove which secret value was selected, without logging it #14

Description

@spbsoluble

Motivation

A support escalation (F5 store returning HTTP 401 after a successful PAM resolution) exposed a
gap in what our logs can prove. With Debug enabled on the orchestrator, the log showed:

Fetching secret '<path>' (type: static_kv)
Secret '<path>' value is JSON-formatted
KV field name set to 'Username'
Successfully extracted field 'Username' from JSON secret
Successfully retrieved secret '<path>' (type: static_kv) from Akeyless

That confirms which secret, which field name was requested, that the field existed, and which
parse branch ran. It cannot confirm that the value handed to Command is the value expected to be
in Akeyless. The 46/46 successful retrievals in that log were indistinguishable from
46 retrievals of an empty string, a value with a trailing newline, or the wrong field.

This issue proposes closing that gap. It is scoped to logging and diagnostics only. No change
to what the provider returns.

Part A: safe diagnostics (no value exposure)

These expose no secret material and are independently useful. Listed in rough value order.

A1. Log the available field names on a JSON-parsed secret.
ParseKvSecret already logs every key it walks (Evaluating KV key '{Key}' at line {LineIndex},
AkeylessPam.cs:355), so key names are established as loggable. ParseJsonSecret logs nothing
equivalent; on the missing-field path (AkeylessPam.cs:322) it reports only the field that was
not found. Because content sniffing sends a static_kv-typed secret holding JSON down the JSON
path (AkeylessPam.cs:402-403), the escalation above got zero key visibility. Log the deserialized
key set (names only) at Debug.

A2. Warn when the extracted field value is empty or whitespace.
AkeylessPam.cs:390 checks that the whole secret is non-empty, but nothing checks the value
after field extraction:

  • ParseJsonSecret (AkeylessPam.cs:319) returns "" for {"Password": ""}.
  • ParseKvSecret (AkeylessPam.cs:359) returns "" for a Password= line.

Either silently yields an empty credential, which downstream presents as an auth failure at the
target device. This is a plausible root cause for the escalation above and there is currently no
signal for it. Log a warning naming the field (not the value).

A3. Warn when the extracted value has leading or trailing whitespace.
The two parsers disagree: ParseKvSecret trims (AkeylessPam.cs:359), ParseJsonSecret does not
(AkeylessPam.cs:319). So a JSON value with a trailing newline is returned verbatim and will fail
authentication at the target, and thanks to content sniffing that is the path a static_kv
secret containing JSON actually takes. Detecting and warning is in scope here; changing the trim
behavior is a behavior change and is not.

A4. On a missing field, hint at a case-insensitive match.
Both lookups are case-sensitive (jsonObj.TryGetValue at AkeylessPam.cs:316 with the default
comparer; k != fieldName at AkeylessPam.cs:356). Username vs username is a common
misconfiguration that currently produces only "does not contain the specified field". If a
case-insensitive match exists, say so in the error log.

Part B: gated value fingerprint

The actual ask: let an operator verify the value we returned matches the value in Akeyless,
without the log ever containing the value.

What. SHA-256 over the UTF-8 bytes of the string GetStaticSecret is about to return, plus
the byte length. Rendered as sha256:<first 12 hex chars>.

Where. A single statement in GetStaticSecret immediately before return result
(AkeylessPam.cs:420). One insertion point covers all three branches (JSON field, KV field, raw
blob / plain text), and because one GetPassword call resolves exactly one field, one call
produces exactly one fingerprint. Do not put this in the parse methods.

Gating. Must require an explicit opt-in, not just a log level. Proposed:
AKEYLESS_LOG_VALUE_FINGERPRINT=true, default off, AND emitted at Trace. Rationale: this would
be the only log line in the provider derived from secret material, and it must not start
appearing merely because someone raised a log level for an unrelated reason. Follow the existing
ResolveEnvOverride pattern for reading the flag, and audit-log the fact that fingerprinting is
enabled.

Verification the flag is meant to enable:

akeyless get-secret-value --name /path/to/secret --token <TOKEN> \
  | jq -rj '.Username' | sha256sum

Compare the first 12 hex characters against the log line.

Security tradeoffs to document

  • The hash must be unsalted for the cross-check above to work. That is the whole point of the
    feature and it is also its main risk.
  • An unsalted hash of a low-entropy value (a short or guessable password) is confirmable offline:
    someone holding the log and a candidate list can test guesses. This is why the feature is
    opt-in and default-off, and the docs must tell operators to enable it for a diagnostic window
    and disable it afterward.
  • Truncating to 12 hex characters is for log readability only. It is not a security control;
    a guesser simply compares prefixes. Do not describe it as mitigation.
  • Logging the length narrows a password-length search slightly. It rides the same flag.
  • Part A carries none of this and should not be gated behind the Part B flag.

Out of scope / adjacent findings

Not part of this issue, flagged while reading the code. Happy to file separately:

  • Possible NullReferenceException at AkeylessPam.cs:319. fieldValue.ToString() ?? string.Empty
    guards ToString() returning null, not fieldValue itself being null. For {"Password": null},
    TryGetValue succeeds with a null fieldValue and .ToString() throws. Looks like it wants to
    be fieldValue?.ToString() ?? string.Empty.
  • The trim asymmetry in A3 is arguably a bug in its own right, not just a thing to warn about.

Acceptance criteria

  • Part A emits no secret material. Verifiable by unit test asserting no logged message contains
    the value under test.
  • Part B emits nothing at all unless the opt-in flag is set, at any log level. Unit test for the
    flag-off case.
  • Fingerprint of a known input matches printf %s '<value>' | sha256sum (guards against a
    trailing-newline or encoding mismatch making the verification check useless).
  • Existing logging discipline holds: no parameter-dictionary values, no AccessKey,
    no ApiException.Message.
  • docsource/ updated (README and docs are generated, so not those directly), including the
    enable-then-disable guidance and the verification command.

No implementation on this issue yet; filed for triage and design review.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions