Skip to content

fix: add usedforsecurity=False to hashlib.sha1 calls for FIPS compliance - #77391

Open
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/hashlib-sha1-fips-tui-gateway
Open

fix: add usedforsecurity=False to hashlib.sha1 calls for FIPS compliance#77391
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/hashlib-sha1-fips-tui-gateway

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Summary

hashlib.sha1() without usedforsecurity=False crashes on FIPS-enabled systems (RHEL 8/9 with FIPS mode) with ValueError: EVP_DigestInit_ex disabled for FIPS.

Two uncovered sites fixed:

  • tui_gateway/server.py:11671 — config revision hash for MCP reload detection
  • optional-skills/security/unbroker/scripts/dossier.py:25 — opaque subject ID generation

Both are non-security uses (cache keys / dedup) where SHA-1 is used purely for content hashing, not for cryptographic security.

Context

This pattern has been fixed across the codebase in prior PRs:

These two sites were missed in the earlier sweeps.

Test Plan

  • python3 -c "import ast; ast.parse(open('tui_gateway/server.py').read())"
  • python3 -c "import ast; ast.parse(open('optional-skills/security/unbroker/scripts/dossier.py').read())"

hashlib.sha1() without usedforsecurity=False crashes on FIPS-enabled
systems (RHEL 8/9 with FIPS mode) with ValueError: EVP_DigestInit_ex
disabled for FIPS.

Two uncovered sites fixed:
- tui_gateway/server.py:11671 - config revision hash for MCP reload
- optional-skills/security/unbroker/scripts/dossier.py:25 - opaque
  subject ID generation

Both are non-security uses (cache keys / dedup) where SHA-1 is used
purely for hashing, not for cryptographic security.
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The two edited SHA-1 calls correctly opt out of FIPS security restrictions without changing their digest outputs. However, the same non-security use remains unpatched in fallback function-call IDs, webhook deduplication, QQBot upload checksums, and dashboard action names, so FIPS-restricted deployments can still fail on those sibling paths.

  • [P2] Non-security SHA-1 call sites still fail in FIPS mode (agent/codex_responses_adapter.py:333)
    The patch fixes the MCP revision token and unbroker subject ID, but default-mode SHA-1 constructors remain in agent/codex_responses_adapter.py:333 (fallback Responses function-call IDs), gateway/platforms/msgraph_webhook.py:390 (fallback webhook receipt IDs), gateway/platforms/qqbot/chunked_upload.py:562 (transport checksum), and hermes_cli/web_server.py:12694 and :13664 (action names). These are identifiers, deduplication keys, checksums, or names rather than cryptographic decisions; under the same FIPS restriction they can raise before producing their outputs. Protocol-signature uses in the SMS, WeCom, Tencent COS, and Yuanbao adapters are security uses and should not be changed by this finding.
    Remediation: Pass usedforsecurity=False to every non-security SHA-1 constructor in the listed identifier, deduplication, checksum, and action-name paths. For the QQBot helper, audit its adjacent non-security MD5 constructors as part of the same FIPS compatibility change; retain the default security mode for Twilio, WeCom, Tencent COS, and Yuanbao protocol-signature primitives.

Security evidence:

  • trust boundary: The patched inputs are local OS randomness and locally loaded MCP configuration. Their SHA-1 outputs become an opaque subject identifier and an in-process/configuration revision token; neither output authenticates a caller, authorizes an action, derives a secret, or verifies integrity.
  • source/sink/invariant: Both changed sinks preserve the existing SHA-1 digest bytes and truncation lengths while explicitly passing usedforsecurity=False. The flag only declares the already non-security purpose to the hash provider; it does not alter the input, digest, or consuming comparison.
  • current-main reproduction: The current-main MCP revision and unbroker subject-ID constructors use default security-mode SHA-1 and are rejected by a FIPS-restricted policy before producing their outputs.
  • PR-head or patch-replay validation: The reviewed patch changes only the MCP revision token and unbroker subject ID calls, each with usedforsecurity=False.
  • positive/negative cases: Focused MCP reload and unbroker tests pass; a FIPS-mode simulation rejects each pre-patch expression, accepts each patched expression, and preserves the 10- and 12-character truncations.
  • residual bypass search: Default-mode SHA-1 calls remain in the fallback function-call ID, Graph webhook deduplication, QQBot checksum, and two dashboard action-name paths listed above. SMS, WeCom, Tencent COS, and Yuanbao SHA-1 operations are protocol signatures and retain security-mode semantics.
  • reviewer validation: The changed functions and sibling SHA-1 call sites were reviewed against the Python >=3.11 contract, and the proposed remediation preserves security-mode protocol signatures.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

  • Real OpenSSL FIPS-provider validation
  • Ruff lint validation

Signed: GPT-5.6-luna-max in Codex

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

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants