Skip to content

Add use_os_truststore config option and ca_truststore grain#69148

Merged
dwoz merged 6 commits into
saltstack:3008.xfrom
twangboy:fix/65439/3008.x
May 16, 2026
Merged

Add use_os_truststore config option and ca_truststore grain#69148
dwoz merged 6 commits into
saltstack:3008.xfrom
twangboy:fix/65439/3008.x

Conversation

@twangboy
Copy link
Copy Markdown
Contributor

@twangboy twangboy commented May 14, 2026

What does this PR do?

Introduces opt-in support for verifying outbound TLS connections against the native OS certificate store (Windows CryptoAPI, macOS Keychain, Linux system CA bundle) instead of the bundled certifi CA bundle.

  • New config option use_os_truststore (default False) on both master and minion; when True, calls truststore.inject_into_ssl() once at daemon startup via salt.utils.ostruststore.apply_if_enabled()
  • Injection is gated behind the config option and is one-shot per process, preserving existing certifi behavior for all deployments that do not opt in
  • Explicit ca_bundle: in config always takes precedence over the OS store
  • New ca_truststore grain reports which store is active (certifi or os)
  • get_ca_bundle() in salt.utils.http short-circuits to None when use_os_truststore is True and no explicit ca_bundle is configured
  • Requires the truststore package (Python 3.10+); Salt logs a warning and falls back to certifi if the package is absent
  • Documents that pip-system-certs must not be installed in the Salt environment as its .pth file bypasses the config option entirely
  • Does not affect the master/minion PKI authentication layer

What issues does this PR fix or reference?

Fixes #65439

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@twangboy twangboy added this to the Argon v3008.0 milestone May 14, 2026
@twangboy twangboy self-assigned this May 14, 2026
@twangboy twangboy requested a review from a team as a code owner May 14, 2026 21:58
@twangboy twangboy added the test:full Run the full test suite label May 14, 2026
@twangboy twangboy force-pushed the fix/65439/3008.x branch from 567e618 to 44c5bd9 Compare May 14, 2026 21:58
dwoz
dwoz previously approved these changes May 14, 2026
@twangboy twangboy force-pushed the fix/65439/3008.x branch from 44c5bd9 to 1c87543 Compare May 14, 2026 23:58
Copy link
Copy Markdown
Collaborator

@sujitdb sujitdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

twangboy added 6 commits May 15, 2026 16:30
…ltstack#65439)

Introduces opt-in support for verifying outbound TLS connections against the
native OS certificate store (Windows CryptoAPI, macOS Keychain, Linux system
CA bundle) instead of the bundled certifi CA bundle.
- New config option use_os_truststore (default False) on both master and
  minion; when True, calls truststore.inject_into_ssl() once at daemon
  startup via salt.utils.ostruststore.apply_if_enabled()
- Injection is gated behind the config option and is one-shot per process,
  preserving existing certifi behavior for all deployments that do not opt in
- Explicit ca_bundle: in config always takes precedence over the OS store
- New ca_truststore grain reports which store is active (certifi or os)
- get_ca_bundle() in salt.utils.http short-circuits to None when
  use_os_truststore is True and no explicit ca_bundle is configured
- Requires the truststore package (Python 3.10+); Salt logs a warning and
  falls back to certifi if the package is absent
- Documents that pip-system-certs must not be installed in the Salt
  environment as its .pth file bypasses the config option entirely
- Does not affect the master/minion PKI authentication layer
Python's compiler treats any name that appears in an import statement
anywhere in a function as a local variable for the entire function scope.
Each daemon prepare() method contains a late "import salt.master" or
"import salt.minion", which caused the compiler to mark "salt" as local
throughout the function. The "salt.utils.ostruststore.apply_if_enabled()"
call at the top of each prepare() then raised UnboundLocalError because the
local "salt" had not yet been assigned.
Fix by importing ostruststore as "_ostruststore" at module level so the
call sites reference "_ostruststore" directly, which is never shadowed by
the late salt imports inside the function bodies.
@twangboy twangboy force-pushed the fix/65439/3008.x branch from 3f1d04c to b50ea1a Compare May 15, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants