Skip to content

PublicKey.decrypt rebuilds cryptography + libcrypto RSA state on every call #69940

Description

@dwoz

Description

Under load, salt/crypt.py rebuilds cryptography + libcrypto RSA state on
every public-key operation. Loaded PEM bytes are parsed once per call, and
the libcrypto-backed X9.31 verifier / signer objects that back
PublicKey.decrypt and PrivateKey.encrypt are constructed once per call.
There is no per-instance cache of these bridge objects and no path-level
cache of the parsed public key.

memray counts (stressed 3008.x MWorker, 60 s window)

Rig: 3008.x master, salt-api + highstate + software install/remove loops
against a stressed minion pool.

  • PublicKey.__init__ calls / 60 s: 5,922
  • RSAX931Verifier.__init__ calls / 60 s: ~5,000
  • PublicKey.decrypt(data) calls / 60 s: 4,494

Ratio of decrypts per PublicKey instance is ~0.76: most instances are
one-shot
. Per-instance verifier caching alone therefore only helps if
instances get reused via a path-level cache.

Same pattern on the sign side in PrivateKey.encrypt(data).

Root cause

Three separate absent caches:

  1. PublicKey.decrypt re-serializes self.key to PEM and constructs a fresh
    RSAX931Verifier (libcrypto BIO_new_mem_buf + RSA_new +
    PEM_read_bio_RSA_PUBKEY) on every call.
  2. PrivateKey.encrypt mirrors (1) with RSAX931Signer.
  3. PublicKey.from_file(path) reads + parses the PEM every call. Under the
    MWorker workload the master public key is loaded thousands of times per
    minute.

Proposed fix (three layers)

Layer 1 - per-instance lazy _verifier / _signer cache on
PublicKey.decrypt and PrivateKey.encrypt. self.key is immutable after
__init__, so the derived bridge objects can be reused for the lifetime of
the instance.

Layer 2 - mtime-keyed path cache on PublicKey.from_file(path), mirroring
the _get_key_with_evict(path, timestamp, passphrase) idiom used on 3006.x
for the private-key side. Rotation on disk bumps mtime and invalidates the
cache automatically.

Layer 3 - retry-on-verify-fail in PublicKey.verify / .decrypt
preserves the pre-cache "always fresh" semantics for edge cases where a
rotation preserves mtime (cp -p, NFS mtime cache, atomic rename with
preserved timestamps). On the first failure the cache entry is evicted, the
key is reloaded from disk, and one retry is attempted. Genuine bad
signatures still return False and cost only one extra file read + PEM
parse per forged attempt.

Empirical validation

Standalone repro instantiates one PublicKey and calls .decrypt(signed)
1000 times, instrumenting RSAX931Verifier.__init__ to count constructions:

  • Baseline (3008.x HEAD): RSAX931Verifier.__init__ calls: 1000
  • Patched: RSAX931Verifier.__init__ calls: 1

Scope

Change is confined to salt/crypt.py. It does not touch:

  • get_rsa_key / private-key side (handled by a separate mtime-regression PR)
  • AsyncAuth._auth_singleton_key (different cache, unrelated)

Setup

3008.x HEAD, standard master + minion config. Reproducible with any master
under sustained auth + payload-verification load.

Versions Report

salt --versions-report (3008.x head, tip of origin/3008.x).

Metadata

Metadata

Assignees

No one assigned

    Labels

    Performancebugbroken, incorrect, or confusing behaviorseverity-medium3rd level, incorrect or bad functionality, confusing and lacks a work around

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions