Skip to content

Add build-time FIPS crypto profile - #765

Draft
Marc-André Moreau (mamoreau-devolutions) wants to merge 5 commits into
masterfrom
copilot/fips-mode-support
Draft

Marc-André Moreau (mamoreau-devolutions) wants to merge 5 commits into
masterfrom
copilot/fips-mode-support

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

Summary

Adds an explicit build-time fips profile that restricts the crate to an AWS-LC FIPS-backed provider boundary, while leaving the normal default build unchanged.

Base is tag sspi-v0.21.3 and the package version stays 0.21.3 so downstream [patch.crates-io] overrides resolve for consumers on ^0.21 (IronRDP, Devolutions Gateway).

Feature graph

Normal defaults are unchanged:

default   = ["aws-lc-rs", "all-ssps"]
all-ssps  = ["credssp", "negotiate"]
credssp   = ["negotiate"]
negotiate = ["ntlm", "kerberos", "pku2u"]
pku2u     = ["kerberos"]

Supporting features select their SSPs positively (network_client → negotiate, dns_resolver → kerberos, scard → kerberos, pku2u, tsssp → credssp).

Because Cargo features are additive, fips cannot turn defaults off. FIPS consumers must build with:

cargo build --no-default-features --features fips

fips selects picky/fips-aws-lc, rustls/fips, rustls/aws-lc-rs and the internal rustls wiring — nothing else. NTLM, Kerberos, PKU2U, smartcard/winscard, and their direct legacy crypto dependencies are optional and absent from the FIPS graph. compile_error! guards reject conflicting combinations (fips + ring, fips + any SSP feature, etc.).

The excluded crates are asserted in CI: crypto-bigint, crypto-mac, hmac, md-5, md4, picky-krb, rsa, sha1.

TLS

  • rustls::crypto::default_fips_provider() is used under fips.
  • Built configs are verified to report fips(); a preinstalled non-FIPS process default fails closed rather than silently downgrading.
  • SHA-1 signature schemes are removed from the custom verification path.
  • Channel bindings are not silently omitted.

Validation

  • Default profile: 249 unit tests, 2 integration tests, doctests, clippy, rustfmt.
  • FIPS profile built and tested against the real AWS-LC FIPS module (aws-lc-fips-sys 0.14.2): 66 tests pass. Graph-only checking was not sufficient — building all targets initially pulled ungated examples and a legacy integration target into the FIPS build, now fixed with required-features.
  • cargo tree proves the banned crates are absent from the FIPS graph.
  • CI adds a fips job covering the full package test command, per-SSP feature checks, the banned-crate tree rejection, provider feature assertions, and 12 conflict combinations.

Known blocker

picky is temporarily pinned to a git revision (4de0135c528b60500d21451111b5f35d72671395) because the release containing fips-aws-lc is not yet published to crates.io. crates/winscard Picky ASN.1 refs are pinned to the same revision to avoid duplicate-type mismatches. This pin must be replaced with a published version before release.

Scope note

Selecting the fips feature is not a compliance certification. It constrains the build to an AWS-LC FIPS-backed provider boundary. Module, platform, build, environment, and certificate coverage remain deployment requirements.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Git-only dependencies currently block crate publication, and the FIPS fail-closed path lacks regression coverage.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Adds a build-time FIPS profile backed by AWS-LC FIPS while preserving the standard SSP feature set.

Changes:

  • Introduces positive SSP feature gating and optional legacy crypto dependencies.
  • Adds FIPS provider installation, validation, and conflict guards.
  • Expands CI coverage and documents feature usage.
File Description
Cargo.toml Defines FIPS and SSP feature graphs.
Cargo.lock Locks FIPS and updated dependencies.
crates/​winscard/​Cargo.toml Aligns Picky ASN.1 revisions.
.github/​workflows/​ci.yml Adds FIPS boundary validation.
README.md Documents profiles and features.
src/​lib.rs Gates SSP APIs and enforces conflicts.
src/​rustls.rs Installs and validates the FIPS provider.
src/​utils.rs Gates protocol-specific utilities.
src/​auth_identity.rs Gates protocol-specific credentials.
src/​crypto/​mod.rs Gates NTLM cryptography.
src/​generator.rs Gates CredSSP generator support.
src/​ntlm/​config.rs Gates Negotiate integration.
src/​kerberos/​config.rs Gates Negotiate integration.
src/​kerberos/​pa_datas.rs Gates smart-card imports.
src/​pku2u/​config.rs Gates Negotiate integration.
src/​pku2u/​cert_utils/​win_extraction.rs Handles errors without CredSSP.
src/​credssp/​sspi_cred_ssp/​tls_connection.rs Removes SHA-1 verification schemes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Cargo.toml
num-traits = { version = "0.2", default-features = false }

picky = { version = "=7.0.0-rc.25", default-features = false }
picky = { git = "https://github.com/Devolutions/picky-rs", rev = "4de0135c528b60500d21451111b5f35d72671395", default-features = false }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed. The currently published Picky releases do not expose fips-aws-lc, and adding the Git crate's 7.0.0-rc.25 as a registry companion also makes Cargo resolve an incompatible RustCrypto prerelease graph. I documented the publication blocker beside the pin and in the FIPS README. This thread should remain open until a synchronized FIPS-enabled Picky release is published and the Git pin can be replaced safely.

Auto-replied by the GitHub Copilot app

iso7816-tlv = "0.4"
picky = { workspace = true, features = ["x509"] }
picky-asn1-x509.workspace = true
picky-asn1-x509 = { git = "https://github.com/Devolutions/picky-rs", rev = "4de0135c528b60500d21451111b5f35d72671395" }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed. These ASN.1 Git sources must stay synchronized with the root Picky FIPS revision to avoid duplicate-type mismatches. I documented the blocker beside the pins and in the README. This thread should remain open until the matching Picky ASN.1 releases are available, at which point both Git dependencies must be replaced with registry versions.

Auto-replied by the GitHub Copilot app

Comment thread src/rustls.rs
Comment thread README.md Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Repin Picky to the validated FIPS-compatible revision and keep the default SSP surface unchanged through the all-ssps aggregate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Restore master PKU2U APIs after the feature split, synchronize the workspace on the audited Picky source, add fail-closed provider regression coverage, and clarify the package boundary and publication blocker.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants