Skip to content

fix: verify PKCS7 signature on Azure instance identity tokens (backport 2.32)#25303

Merged
spikecurtis merged 1 commit into
release/2.32from
spike/azure-pkcs7-verify-2.32
May 13, 2026
Merged

fix: verify PKCS7 signature on Azure instance identity tokens (backport 2.32)#25303
spikecurtis merged 1 commit into
release/2.32from
spike/azure-pkcs7-verify-2.32

Conversation

@spikecurtis
Copy link
Copy Markdown
Contributor

The Azure instance-identity authentication endpoint parsed the PKCS7 envelope and verified the certificate chain, but never verified the PKCS7 signature itself. An attacker could forge a PKCS7 envelope with a legitimate, publicly obtainable Azure certificate and arbitrary vmId content to obtain any agent auth token.

Add verifyPKCS7Signature(), a custom PKCS7 signature verification that handles Azure non-standard use of sha256WithRSAEncryption (OID 1.2.840.113549.1.1.11) as the DigestAlgorithm. The upstream go.mozilla.org/pkcs7 library Verify() rejects this combination.

The verification checks:

  1. Content digest matches the signed message-digest attribute
  2. Signature over the authenticated attributes is valid

Tests added:

  • TestValidate_TamperedContent: forges a PKCS7 with modified vmId, confirms rejection
  • TestValidate_UntrustedCertWithValidSignature: valid PKCS7 signature with untrusted cert chain, confirms rejection

Migrates Azure instance identity verification from
`go.mozilla.org/pkcs7` and `github.com/fullsailor/pkcs7` to
`github.com/smallstep/pkcs7`, using `VerifyWithChainAtTime` to validate
both the PKCS7 signature and the certificate chain in one call. The
previous code only verified the signer certificate against a set of
intermediates/roots but did not verify that the PKCS7 signature itself
covered the content, meaning tampered payloads could be accepted.

The `Options` struct is restructured to accept `Roots`, `Intermediates`,
and `CurrentTime` as explicit fields instead of embedding
`x509.VerifyOptions`. The test helper `NewAzureInstanceIdentity` now
builds a realistic 3-level certificate chain (Root CA -> Intermediate CA
-> Signing Cert) matching real Azure trust hierarchy. New tests
(`TestValidate_TamperedContent`,
`TestValidate_UntrustedCertWithValidSignature`) confirm tampered and
untrusted envelopes are rejected.

Addresses GHSA-6x44-w3xg-hqqf.

> [!NOTE]
> This PR was authored by Coder Agents.

<details>
<summary>Implementation Plan</summary>

| File | Summary |
|------|---------|
| `coderd/azureidentity/azureidentity.go` | Replace `signer.Verify()`
with `VerifyWithChainAtTime`; restructure `Options` struct; add
`ParseCertificates()` helper |
| `coderd/azureidentity/azureidentity_test.go` | Add `testCertChain`
builder, tampered-content and untrusted-cert tests; update existing
tests for new `Options` API |
| `coderd/coderd.go` | Change `AzureCertificates` field from
`x509.VerifyOptions` to `azureidentity.Options` |
| `coderd/workspaceresourceauth.go` | Pass `api.AzureCertificates`
directly instead of wrapping |
| `coderd/coderdtest/coderdtest.go` | Migrate to `smallstep/pkcs7`;
build 3-level cert chain in test helper |
| `go.mod` / `go.sum` | Add `github.com/smallstep/pkcs7`; remove
`fullsailor/pkcs7` and `go.mozilla.org/pkcs7` |

</details>
@spikecurtis spikecurtis force-pushed the spike/azure-pkcs7-verify-2.32 branch from 060e335 to e460fa5 Compare May 13, 2026 17:22
@spikecurtis spikecurtis changed the title fix(coderd/azureidentity): verify PKCS7 signature on Azure instance identity tokens (backport 2.32) fix: verify PKCS7 signature on Azure instance identity tokens (backport 2.32) May 13, 2026
@spikecurtis spikecurtis merged commit d6e9344 into release/2.32 May 13, 2026
35 of 37 checks passed
@spikecurtis spikecurtis deleted the spike/azure-pkcs7-verify-2.32 branch May 13, 2026 17:45
@github-actions github-actions Bot locked and limited conversation to collaborators May 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants