Skip to content

Carry the tenant SPIFFE SAN in enrollment CSRs; stop the TLS downgrade (WDY-2498, WDY-2799) - #1855

Open
justsem wants to merge 1 commit into
mainfrom
wdy-2498-tenant-spiffe-san-and-tls-hardening
Open

Carry the tenant SPIFFE SAN in enrollment CSRs; stop the TLS downgrade (WDY-2498, WDY-2799)#1855
justsem wants to merge 1 commit into
mainfrom
wdy-2498-tenant-spiffe-san-and-tls-hardening

Conversation

@justsem

@justsem justsem commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Agent + CLI half of the enrollment chain. Cloud's half (the tenant_uuid claim) is being built in parallel — the wire contract is pinned on WDY-2584.

WDY-2498 — tenant SPIFFE SAN

Cloud relays issuance to pki-core under the service-identity profile with a signed grant, and refuses to sign that grant unless the CSR carries exactly one tenant SPIFFE URI SAN equal to the grant principal (FabricRelayCertBackend.swift:119-125). Our CSRs carry only urn:wendy, so every one is rejected.

The agent and CLI now read the tenant_uuid claim from cloud's enrollment token and add spiffe://wendy.sh/tenant/<uuid>/service/asset-<id> (or /service/user-<id>).

Two constraints from pki-core shaped this:

  • The SPIFFE SAN goes alongside the urn:wendy SAN, not instead of it. A SPIFFE principal names a tenant UUID, not the int32 org that certs.IdentityFromCert compares. Replacing the urn would leave device certs with no parseable org identity, and main.go:623-632 then degrades the mTLS gate to OrgModeOff — silently disarming org-equality enforcement fleet-wide.
  • URI-SAN-only. service-identity is the one profile that consults the tenant domain allow-list, so any dNSName fails the mint with ErrDNSSANNotAuthorized. There is a test asserting the CSR carries none.

tenant_uuid is optional — cloud omits it for orgs with no pki tenant (normal for the local and GCP CAS backends). Absence means "build the CSR as before" and is never an error, so those deployments are unaffected.

GenerateCSR now takes []string of identity URIs instead of a single URN, which is what lets one CSR carry both. All four token-derived call sites route through it: the agent's StartProvisioning, wendy auth login, wendy auth login --api-key, and wendy os provision. The certificate refresh path is deliberately untouched — it derives identity from the stored cert, not a token, so there is no tenant claim to read.

WDY-2799 — enrollment TLS downgrade (security)

Folded in per dispatch; it is the same function. The dial picked its transport by port heuristic — :443 meant TLS, anything else meant plaintext — so a cloudHost written without a port became <host>:50051 and shipped the enrollment token, a bearer credential, in cleartext to a public host.

TLS is now the default for every address, a port-less host resolves to :443, and a downgrade requires WENDY_CLOUD_INSECURE. Local plaintext pki-core still works by naming its port and setting that variable; the agent logs a warning naming the host when it does.

Testing

go build ./..., go vet ./go/..., go test ./go/... all clean except internal/agent/oci GPU entitlement failures, which fail identically on clean main (verified by stashing) — environment-dependent, unrelated to this change.

Both new guards were confirmed to fail against the pre-fix code, not just pass against the new:

  • The CSR test parses the CSR the fake cloud actually received and asserts both SAN sets plus zero DNS SANs.
  • The transport test points the real DefaultCloudDialer at a plaintext server on a non-443 port and asserts the RPC only succeeds once the opt-out is set.

Notes for review

  • Merge conflict warning: this touches go/internal/cli/commands/auth.go, which feat/cli-oidc-login also modifies heavily. enrollmentTokenIdentity changed signature there. Worth sequencing with that branch.
  • Nothing here is observable until cloud's half ships. WDY-2797 (the dev broker's missing ALPN h2) is now fixed and re-verified liveapi.dev.wendy.sh:443 negotiates h2, so both the CLI and the device can reach IssueCertificate. The remaining legs are cloud's tenant_uuid PR and WDY-2804's dev-data repair (no organizations row maps the acme realm's tenant, so the claim would be omitted on dev regardless).

WDY-2498: cloud relays certificate issuance to pki-core under the
"service-identity" profile with a signed grant, and refuses to sign that
grant unless the CSR carries exactly one tenant SPIFFE URI SAN equal to
the grant principal. Today's CSRs carry only urn:wendy, so grant-relayed
issuance rejects every one of them.

Cloud now puts the org's pki tenant in its own enrollment-token JWT as a
"tenant_uuid" claim (WDY-2584). The agent and CLI read it and add
spiffe://wendy.sh/tenant/<uuid>/service/asset-<id> (or /service/user-<id>)
to the CSR. Two constraints from pki-core shape this:

  - The SPIFFE SAN is carried *alongside* the urn:wendy SAN, not instead
    of it. The urn is what the agent's own org gate reads out of a peer
    certificate; a SPIFFE principal names a tenant UUID, not the int32
    org the gate compares, so replacing it would leave device certs with
    no parseable org identity and silently disarm org-equality
    enforcement across the fleet.
  - The CSR stays URI-SAN-only. "service-identity" is the one profile
    that consults the tenant domain allow-list, so any dNSName fails the
    mint with ErrDNSSANNotAuthorized.

tenant_uuid is optional: cloud omits it for organizations with no pki
tenant, the normal state for the local and GCP CAS backends. Absence
means "build the CSR as before" and is never an error, so those
deployments keep enrolling unchanged.

GenerateCSR now takes a slice of identity URIs rather than a single URN,
which is what lets one CSR carry both SANs. All four token-derived call
sites go through it: the agent's StartProvisioning, `wendy auth login`,
`wendy auth login --api-key`, and `wendy os provision`. The certificate
*refresh* path is untouched — it derives identity from the stored cert
rather than a token, so it has no tenant claim to read.

WDY-2799, folded in as it is the same code: the enrollment dial no longer
picks its transport by port heuristic. ":443 means TLS, anything else
means plaintext" meant a cloudHost written without a port became
<host>:50051 and shipped the enrollment token -- a bearer credential --
in cleartext to a public host. TLS is now the default for every address,
a port-less host resolves to :443 rather than the plaintext provisioning
port, and a downgrade requires WENDY_CLOUD_INSECURE to be explicitly set.
Enrolling against a local plaintext pki-core still works by naming its
port and setting that variable, and the agent logs a warning naming the
host whenever it does.

Tests: the CSR assertions parse the CSR the fake cloud actually received
and check both SAN sets plus the absence of DNS SANs; the transport guard
points the real dialer at a plaintext server on a non-443 port and
asserts the RPC only succeeds once the opt-out is set. Both were
confirmed to fail against the pre-fix code.
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

AI Docs Review

Note

Automated docs coverage suggestions from Claude. Apply, adapt, or dismiss as needed.

Claude found docs coverage suggestions for this PR.

🛑 Error clients/wendy-cli/commands/device/enroll.md and provision.md document only the urn:wendy SAN

docs/clients/wendy-cli/commands/device/enroll.md: Both enroll.md and provision.md state the CSR carries a single urn:wendy URI SAN; they now carry an additional tenant SPIFFE URI SAN when the token includes a tenant_uuid.

Details
## enroll.md — suggested diff

```diff
-› **Certificate identity:** The CSR submitted during provisioning includes the
-› device's authoritative Wendy identity as a URI Subject Alternative Name
-› (`urn:wendy:org:‹org›:asset:‹assetID›`). The cloud certificate service
-› validates this SAN against the enrollment token at issuance time.
+› **Certificate identity:** The CSR submitted during provisioning always
+› includes the device's authoritative Wendy identity as a URI Subject
+› Alternative Name (`urn:wendy:org:‹org›:asset:‹assetID›`). When the
+› enrollment token includes a `tenant_uuid` claim, a second URI SAN
+› (`spiffe://wendy.sh/tenant/‹uuid›/service/asset-‹assetID›`) is added;
+› cloud requires this SAN to sign relay grants. Orgs without a pki tenant
+› receive only the urn:wendy SAN, as before.
 ```

## provision.md — same change

Apply the identical replacement in the callout at the top of `clients/wendy-cli/commands/device/provision.md`.

🛑 Error pki/README.md does not document the WENDY_CLOUD_INSECURE opt-out or the TLS-default change

docs/pki/README.md: The local-dev pki-core instructions describe connecting to port 50051 but say nothing about the new TLS-by-default enrollment dial or the WENDY_CLOUD_INSECURE variable required to use plaintext.

Details
## Suggested addition to `pki/README.md`

In the **Local development with pki-core****Provision a device** sub-section, add a note after the `wendy device provision` example:

```diff
+› **Plaintext dial (local pki-core only):** The enrollment dial is TLS by
+› default for every address. When pointing at a local pki-core that serves
+› plaintext gRPC (the default `pkicore serve wendy --dev` configuration),
+› set `WENDY_CLOUD_INSECURE=1` before running `wendy device provision` or
+› `wendy auth login-local`. The agent logs a warning naming the host whenever
+› this variable is set, because the enrollment token — a bearer credential —
+› is then sent in cleartext. Never set this variable on a real device.
+
+› A port-less hostname now resolves to `:443` (previously `:50051`). Local
+› pki-core addresses must include an explicit port, e.g. `192.168.0.102:50051`.
 ```

Also update the existing note about the end-to-end test helper to reflect the `nil` rather than `""` signature change (minor, but keeps the note accurate):

```diff
-› **Note:** The end-to-end test helper `go run ./cmd/local-pki-test` passes an
-› empty identity URN, so the CSR it generates has no URI SAN.
+› **Note:** The end-to-end test helper `go run ./cmd/local-pki-test` passes
+› `nil` for identity URIs, so the CSR it generates has no URI SAN.
 ```

🛑 Error pki/README.md omits tenant SPIFFE URI SAN added to enrollment CSRs

docs/pki/README.md: The CSR identity table documents only the urn:wendy SAN; enrollment CSRs now also carry a tenant SPIFFE URI SAN when the token includes a tenant_uuid claim.

Details
## Suggested diff for `pki/README.md`

In the **Certificate identity and URI SAN** section, extend the table and the surrounding explanation:

```diff
 ## Certificate identity and URI SAN
 
 Every CSR generated by the CLI and agent now carries the authoritative Wendy
 identity as a **URI Subject Alternative Name** in addition to the legacy
 Subject CommonName:
 
 | Entity | URI SAN format |
 |--------|---------------|
 | User (CLI) | `urn:wendy:org:‹orgID›:user:‹userID›` |
 | Device / agent | `urn:wendy:org:‹orgID›:asset:‹assetID›` |
 
+When the enrollment token includes a `tenant_uuid` claim, a second URI SAN is
+added alongside the urn:wendy SAN:
+
+| Entity | Tenant SPIFFE URI SAN format |
+|--------|------------------------------|
+| Device / agent | `spiffe://wendy.sh/tenant/‹uuid›/service/asset-‹assetID›` |
+| User (CLI) | `spiffe://wendy.sh/tenant/‹uuid›/service/user-‹userID›` |
+
+Cloud refuses to sign a relay grant unless the CSR carries exactly this
+SPIFFE principal. The urn:wendy SAN is retained — it is what the agent's
+org-equality gate reads from peer certificates, and removing it would
+silently disable mTLS org enforcement. Only URI SANs are emitted; pki-core's
+`service-identity` profile rejects CSRs that carry any DNS SAN.
+
+Orgs with no pki tenant receive no `tenant_uuid` claim and enroll exactly as
+before — the absence of the claim is never an error.
 ```

⚠️ Concern provision.md still implies port 50051 is the default cloud port for bare hostnames

docs/clients/wendy-cli/commands/device/provision.md: The provisioning command documentation does not mention that a bare hostname now resolves to :443 instead of :50051, which could mislead users configuring --cloud without an explicit port.

Details
## Suggested addition to `clients/wendy-cli/commands/device/provision.md`

In the Flags table, update the `--cloud` description:

```diff
 | `--cloud` | yes | pki-core gRPC address (`host:port`) |
 ```

Change to:

```diff
 | `--cloud` | yes | pki-core gRPC address (`host:port`). A port is required for local pki-core — a bare hostname resolves to `:443` and the dial is TLS by default. Set `WENDY_CLOUD_INSECURE=1` for plaintext pki-core endpoints. |
 ```

⚠️ Concern Certificate refresh path intentionally excluded from SPIFFE SAN — not documented

docs/pki/README.md: The certificate refresh path does not add the tenant SPIFFE SAN because it derives identity from the stored cert rather than a token; this deliberate asymmetry is not described anywhere in the docs.

Details
## Suggested addition to `pki/README.md`

Add a note in the **Certificate identity and URI SAN** section after the tenant SPIFFE URI explanation:

```markdown**Certificate refresh:** The refresh path derives identity from the already-stored
› provisioning certificate rather than an enrollment token, so it carries no
› `tenant_uuid` claim and does not add the SPIFFE URI SAN. Only the four
› initial-enrollment paths (agent `StartProvisioning`, `wendy auth login`,
› `wendy auth login --api-key`, and `wendy os provision`) include the SPIFFE SAN.
```

💡 Info security/index.mdx or security/verification.mdx may benefit from noting the TLS-default enrollment fix

The fix for WDY-2799 (enrollment token previously sent in cleartext when cloudHost had no port) is a security improvement worth noting in the security documentation.

Details
## Suggested addition

In whichever security doc discusses enrollment or transport security, add:

```markdown
## Enrollment transport security

The agent's enrollment dial is TLS by default for every address. A bare
hostname (no explicit port) resolves to `:443`. Previously, a hostname
without a port resolved to `:50051` and the old port heuristic dialled that
in plaintext, sending the enrollment token — a bearer credential — in
cleartext to a public host (WDY-2799).

A plaintext dial now requires `WENDY_CLOUD_INSECURE=1` and is intended only
for local pki-core development. The agent logs a warning naming the host
whenever this variable is active.
```

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

AI Security Review

Note

Automated security review from Claude. Apply, adapt, silence with // SECURITY: <reason>, or dismiss as needed.

Input coverage: 11/11 changed files; 36,225/36,225 bytes reviewed; diff SHA-256 7276c7f61dda43c09d4bd12747846f19e462ac073461e4e60a51b598d160e7b1; truncation: none.

Claude found security review findings for this PR.

⚠️ Concern — Open MEDIUM: Unvalidated tenant_uuid and user_id concatenated into SPIFFE SAN URIs

go/internal/shared/certs/orgident.go:39-63: AssetSPIFFEURI/UserSPIFFEURI build SPIFFE principal URIs by raw string concatenation of untrusted token claims, allowing path/URI injection into the CSR SAN.

Details
**Status:** Open
**Severity:** MEDIUM
**Standards:** SOC2-CC6, ISO27001-A.8, NIST-800-53-IA-5, NIST-800-53-SI-10
**Location:** `go/internal/shared/certs/orgident.go:39-63`

`AssetSPIFFEURI(tenantUUID, assetID)` and `UserSPIFFEURI(tenantUUID, userID)` do `tenantSPIFFEPrefix + tenantUUID + "/service/user-" + userID` with no validation. These values come from `enrolltoken.Parse`, which decodes the JWT payload **without verifying the signature** (`enrolltoken.go` Parse only base64-decodes the middle segment; header/sig are placeholders in tests and are not checked). A `tenant_uuid` or `user_id` containing `/`, `?`, `#`, or `..` would inject extra path segments into the SPIFFE URI (e.g. `user_id="a/service/asset-1"` reshapes the principal), potentially causing the CSR to claim a different tenant/service principal than intended. Note the existing `enrollmentTokenIdentity` code already guards `user_id` against `:` for the urn path, but no equivalent guard exists for the SPIFFE path, and there is zero validation that `tenant_uuid` is a canonical UUID. Remediation: validate `tenantUUID` matches a strict UUID regex and reject/escape `userID` (disallow `/`, control chars, and non-canonical characters) before building the URI; consider using `url.PathEscape` on each path segment and failing closed on malformed input rather than silently emitting a malformed principal.

💡 Info — Open LOW: Plaintext enrollment opt-out gated only by environment variable

go/internal/agent/services/provisioning_service.go:45-75: WENDY_CLOUD_INSECURE allows shipping the enrollment bearer token in cleartext if the variable is set on a production device.

Details
**Status:** Open
**Severity:** LOW
**Standards:** SOC2-CC6.6, ISO27001-A.8.24, NIST-800-53-SC-8
**Location:** `go/internal/agent/services/provisioning_service.go:45-75`

The downgrade is a genuine improvement over the prior port heuristic, and the code logs a warning naming the host when insecure mode is active — good defense-in-depth. Residual risk: a stray/injected `WENDY_CLOUD_INSECURE=1` in a device environment silently permits cleartext transmission of the enrollment token (a bearer credential). The warning is the only compensating control and logs may not be monitored on edge devices. Remediation (hardening): consider additionally restricting the opt-out to non-production builds or to loopback/private-range addresses only, so plaintext can never target a public host even when the variable is set. This is non-blocking given the added warning.

💡 Info — Open INFORMATIONAL: Enrollment token parsed without signature verification before use

go/internal/shared/enrolltoken/enrolltoken.go:19-70: Claims (including tenant_uuid) are read from the token payload without verifying the token signature client-side.

Details
**Status:** Open
**Severity:** INFORMATIONAL
**Standards:** SOC2-CC6.1, ISO27001-A.8.24
**Location:** `go/internal/shared/enrolltoken/enrolltoken.go:19-70`

`Parse` decodes only the base64url payload segment; the signature is never checked on the agent/CLI side. This appears to be by design — actual authorization is enforced server-side by cloud and pki-core when the grant is signed and the CSR is minted — so a forged claim only yields a CSR that the server rejects. This is acceptable if and only if the server-side binding (grant principal == CSR SPIFFE SAN, org-equality on the urn) is authoritative. Documenting here so reviewers confirm no downstream code trusts these client-parsed claims for an authorization decision. No change required if server-side enforcement holds.
Compliance summary
**SOC 2 / ISO 27001 / NIST 800-53:** The TLS-by-default change materially improves protection of the enrollment bearer credential in transit (SC-8 / A.8.24) and is a positive. One medium finding concerns input validation (SI-10 / A.8.24): untrusted, unverified token claims are concatenated into SPIFFE SAN URIs without sanitization, which should be fixed by strict UUID/userID validation before merge or explicitly risk-accepted. The plaintext opt-out and unverified client-side token parsing are low/informational hardening items. No PCI DSS or HIPAA data domains are touched by this diff.

@github-actions github-actions Bot added the risk: high High estimated risk; thoroughly test compatibility and affected workflows label Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high High estimated risk; thoroughly test compatibility and affected workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant