Skip to content

add_ssh_key / update_ssh_key private keys reach the operations log: key is missing from the processLocalTransaction strip list #2199

Description

@dawsontoth

processLocalTransaction logs every operation body at info and strips a list of secret-bearing fields — but not key, which is where add_ssh_key and update_ssh_key carry an SSH private key. So the private key is written to the operations log verbatim.

core/server/serverHelpers/serverUtilities.ts:98-111:

const {
    hdb_user, hdbAuthHeader, password, payload, credentials,
    registryAuth, value, values, envelope,
    ...cleanBody
} = req.body;
operationLog.info(cleanBody);   // :111 — `key` is not stripped

The lists already disagree with each other

harper-pro's replication path strips this exact field, deliberately. harper-pro/replication/logRedaction.ts:13:

const SENSITIVE_OPERATION_FIELDS = ['token', 'key', 'password', 'hdbAuthHeader'];

with a header comment naming the reason: "Some replicated/forwarded operations carry secrets — SSH private keys (add_ssh_key / update_ssh_key) …". So key was already identified as private-key material that must not reach logs, the replication send/receive paths were hardened for it, and the operations log never got the same treatment.

Two confirmed exposure paths

  1. Origin, client-supplied key. The log line runs before the handler, so any sealing the handler does cannot help — the plaintext key as posted is already in the operations log.
  2. Any peer receiving a replicated add_ssh_key/update_ssh_key. harper-pro/replication/replicationConnection.ts:2952 carefully calls redactOperationForLog for its own debug line, then passes the unredacted object to server.operation(...) on the next line. That reaches operation() at serverUtilities.ts:311processLocalTransaction({ body: operation }, …) → the log line at :111, with key already populated on the received body. Where the cluster has secret custody this logs the enc:v1: envelope (not plaintext, but still the stored credential); on a node with no custody, sealSSHKey passes the key through unchanged, so peers log the private key in the clear.

Worth noting what is not affected, since it is easy to assume otherwise: on the origin, add_ssh_key generate=true avoids path 1 entirely, because req.key is still undefined when the log line runs. The generate path is exposed only via path 2, on peers.

Reachability

Gated on harperLogger.logLevel ∈ {info, debug, trace}; the default is warn (core/static/defaultConfig.yaml, logging.level). So it is off by default — but info/debug is a routine troubleshooting setting, and operations logs are commonly shipped off-host, where a private key then lands in a log aggregator with a different retention and access model than <rootPath>/ssh/.

Fix

Add key to the destructured strip list at serverUtilities.ts:98-109. One word, and it covers add_ssh_key and update_ssh_key together. Worth a comment tying it to the logRedaction.ts list so the two do not drift again — and it may be worth reconciling the two lists outright, since token is stripped by one and not the other.

Provenance

Found and confirmed by code reading during a deep-review of HarperFast/harper-pro#594 (server-side SSH keygen). Pre-existing and unrelated to that PR's diff; filed here rather than there because the defect and the fix are both in core. Cross-ref: HarperFast/harper-pro#594.

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

    area:securitySecurity, TLS/certs, authentication, authorizationbugSomething isn't working

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions