fix: verify PKCS7 signature on Azure instance identity tokens (backport 2.31) (#25304)

Backport of: #25286

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>

<!--

If you have used AI to produce some or all of this PR, please ensure you
have read our [AI Contribution
guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING)
before submitting.

-->

Co-authored-by: Jakub Domeracki <jakub@coder.com>
This commit is contained in:
Spike Curtis
2026-05-13 13:56:43 -04:00
committed by GitHub
parent eb461163c7
commit 6ff657f090
7 changed files with 333 additions and 71 deletions
+58 -21
View File
@@ -32,11 +32,11 @@ import (
"time"
"cloud.google.com/go/compute/metadata"
"github.com/fullsailor/pkcs7"
"github.com/go-chi/chi/v5"
"github.com/golang-jwt/jwt/v4"
"github.com/google/uuid"
"github.com/prometheus/client_golang/prometheus"
"github.com/smallstep/pkcs7"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/text/cases"
@@ -59,6 +59,7 @@ import (
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/autobuild"
"github.com/coder/coder/v2/coderd/awsidentity"
"github.com/coder/coder/v2/coderd/azureidentity"
"github.com/coder/coder/v2/coderd/connectionlog"
"github.com/coder/coder/v2/coderd/cryptokeys"
"github.com/coder/coder/v2/coderd/database"
@@ -118,7 +119,7 @@ type Options struct {
AppHostname string
AWSCertificates awsidentity.Certificates
Authorizer rbac.Authorizer
AzureCertificates x509.VerifyOptions
AzureCertificates azureidentity.Options
GithubOAuth2Config *coderd.GithubOAuth2Config
RealIPConfig *httpmw.RealIPConfig
OIDCConfig *coderd.OIDCConfig
@@ -1571,27 +1572,63 @@ func NewAWSInstanceIdentity(t testing.TB, instanceID string) (awsidentity.Certif
}
}
// NewAzureInstanceIdentity returns a metadata client and ID token validator for faking
// instance authentication for Azure.
func NewAzureInstanceIdentity(t testing.TB, instanceID string) (x509.VerifyOptions, *http.Client) {
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
// NewAzureInstanceIdentity returns a metadata client and ID token
// validator for faking instance authentication for Azure. It builds
// a realistic 3-level certificate chain (Root CA -> Intermediate ->
// Signing Cert) to match the real Azure trust hierarchy.
func NewAzureInstanceIdentity(t testing.TB, instanceID string) (azureidentity.Options, *http.Client) {
// Root CA (self-signed, trusted).
rootKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
rootTmpl := &x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{CommonName: "Test Root CA"},
NotBefore: time.Now().Add(-time.Hour),
NotAfter: time.Now().AddDate(10, 0, 0),
IsCA: true,
BasicConstraintsValid: true,
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
}
rootDER, err := x509.CreateCertificate(rand.Reader, rootTmpl, rootTmpl, &rootKey.PublicKey, rootKey)
require.NoError(t, err)
rootCert, err := x509.ParseCertificate(rootDER)
require.NoError(t, err)
rawCertificate, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{
SerialNumber: big.NewInt(2022),
// Intermediate CA (signed by root).
interKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
interTmpl := &x509.Certificate{
SerialNumber: big.NewInt(2),
Subject: pkix.Name{CommonName: "Test Intermediate CA"},
NotBefore: time.Now().Add(-time.Hour),
NotAfter: time.Now().AddDate(5, 0, 0),
IsCA: true,
BasicConstraintsValid: true,
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
}
interDER, err := x509.CreateCertificate(rand.Reader, interTmpl, rootCert, &interKey.PublicKey, rootKey)
require.NoError(t, err)
interCert, err := x509.ParseCertificate(interDER)
require.NoError(t, err)
// Signing cert (leaf, signed by intermediate).
signKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
signTmpl := &x509.Certificate{
SerialNumber: big.NewInt(3),
Subject: pkix.Name{CommonName: "metadata.azure.com"},
NotBefore: time.Now().Add(-time.Hour),
NotAfter: time.Now().AddDate(1, 0, 0),
Subject: pkix.Name{
CommonName: "metadata.azure.com",
},
}, &x509.Certificate{}, &privateKey.PublicKey, privateKey)
require.NoError(t, err)
certificate, err := x509.ParseCertificate(rawCertificate)
}
signDER, err := x509.CreateCertificate(rand.Reader, signTmpl, interCert, &signKey.PublicKey, interKey)
require.NoError(t, err)
signCert, err := x509.ParseCertificate(signDER)
require.NoError(t, err)
// Build PKCS7 signed data with only the signing cert.
signed, err := pkcs7.NewSignedData([]byte(`{"vmId":"` + instanceID + `"}`))
require.NoError(t, err)
err = signed.AddSigner(certificate, privateKey, pkcs7.SignerInfoConfig{})
err = signed.AddSigner(signCert, signKey, pkcs7.SignerInfoConfig{})
require.NoError(t, err)
signatureRaw, err := signed.Finish()
require.NoError(t, err)
@@ -1604,12 +1641,12 @@ func NewAzureInstanceIdentity(t testing.TB, instanceID string) (x509.VerifyOptio
})
require.NoError(t, err)
certPool := x509.NewCertPool()
certPool.AddCert(certificate)
roots := x509.NewCertPool()
roots.AddCert(rootCert)
return x509.VerifyOptions{
Intermediates: certPool,
Roots: certPool,
return azureidentity.Options{
Roots: roots,
Intermediates: []*x509.Certificate{interCert},
}, &http.Client{
Transport: roundTripper(func(r *http.Request) (*http.Response, error) {
// Only handle metadata server requests.