Skip to content

3008.x: RequestRouter._extract_command allocates fresh MasterKeys per RSA request, leaking OpenSSL EVP_PKEY handles #69927

Description

@dwoz

Description

On 3008.x the pooled master request path routes every incoming payload through RequestRouter._extract_command (salt/master.py:1650-1703). The RSA branch of that method constructs a fresh salt.crypt.MasterKeys per call:

elif enc == "pub":
    # RSA encryption
    import salt.crypt

    mkey = salt.crypt.MasterKeys(self.opts)   # every RSA payload
    load = mkey.priv_decrypt(load)

MasterKeys.__init__ (salt/crypt.py:530-584) is heavy:

  • Constructs a salt.cache.Cache(opts, driver=self.opts["keys.cache_driver"]) object graph.
  • Calls _setup_keys() -> find_or_create_keys(...) which reads master.pem from disk and loads the private RSA key into an OpenSSL EVP_PKEY handle via cryptography.

RSA-encrypted payloads on this path are _auth requests. With open_mode: True + auto_accept: True, or under any workload where minions churn re-authentications (test suites, unstable networks, mass minion restarts), every _auth allocates a fresh MasterKeys on the Python heap and an EVP_PKEY handle that OpenSSL retains in its own arena (not tracked by pymalloc, so it appears as smem "system" growth rather than Python allocator growth).

Impact

This is fix #2 in the sequence identified by the memray-backed audit at agents/reports/zmq-master-app-leak-audit.md in the internal stress rig (tests/monitoring/). Suspect #1 (per-request Crypticle) is tracked in #69922; the ZMQ identity slot-cap is #69920; the warn_until memoize is #69924.

Under RSA-heavy auth churn the per-call cost is ~100x higher than the AES path because the RSA key load is unavoidably a disk read plus an OpenSSL EVP_PKEY parse. Even at low request rates (a handful of _auth/s), the leaked EVP_PKEY handles accumulate in OpenSSL's own arena and show up as monotonic RSS growth in the MWorkerQueue process.

Proposed fix

Hoist the MasterKeys(self.opts) construction out of _extract_command and into RequestRouter.__init__, exposed via a lazy @property so routers that only ever see AES traffic don't pay for the allocation.

class RequestRouter:
    def __init__(self, opts, secrets=None):
        ...
        self._master_keys = None

    @property
    def master_keys(self):
        if self._master_keys is None:
            import salt.crypt
            self._master_keys = salt.crypt.MasterKeys(self.opts)
        return self._master_keys

    def _extract_command(self, payload):
        ...
        elif enc == "pub":
            load = self.master_keys.priv_decrypt(load)

Safety

MasterKeys is immutable once initialized. _setup_keys runs exactly once inside __init__, and no method mutates self.key, self.master_key, self.pub_signature, self.pubkey_signature, or the cache handle after construction. Every other MasterKeys(self.opts) call site in the codebase is inside an __init__ (the master process, AuthFuncs, ReqServerChannel, etc.), i.e. one-shot at process startup -- no rotation flow re-instantiates MasterKeys at runtime.

Runtime key rotation goes through SMaster.secrets shared memory (the AES symmetric key), which is orthogonal to the RSA private key. The rotate_aes_key opt drives SMaster.rotate_secrets on secrets["aes"], not MasterKeys.

Reproduction

Stress rig at tests/monitoring/ (already checked in). See agents/reports/zmq-master-app-leak-audit.md for the memray methodology.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions