Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions internal/material/x509/keypair.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ import (
// certificate request. The name is fixed by RFC 7468.
const pemTypeCertificateRequest = "CERTIFICATE REQUEST"

// csrSubjectCommonName is a placeholder subject. STS does not parse the CSR, so an empty
// one reaches the signer, which rejects it. It names the agent, not the workload, because
// the agent does not choose the identity.
//
// TEMPORARY: delete once STS overwrites the subject and marks the SAN critical itself.
const csrSubjectCommonName = "eks-workload-identity"

// generateKey creates a new key pair of the configured kind for one certificate.
//
// It returns a crypto.Signer rather than the key, because a Signer does not expose the
Expand Down Expand Up @@ -64,11 +71,12 @@ func rsaKey(bits int, kind workloadidentity.KeyType) (crypto.Signer, error) {
// buildCSR creates the certificate request we send to STS. It carries the public half of
// the key pair and a signature made with the private half, which proves possession.
//
// It carries no name. STS takes the subject from the attested workload identity in the
// session, and ignores any subject or SAN in the request.
// It names no identity. The SPIFFE ID comes from the attested workload identity in the
// session, never from the request, so the request carries no SAN and no extensions. Its
// subject is the fixed csrSubjectCommonName placeholder, for the reason given there.
func buildCSR(key crypto.Signer) (string, error) {
der, err := cryptox509.CreateCertificateRequest(rand.Reader, &cryptox509.CertificateRequest{
Subject: pkix.Name{},
Subject: pkix.Name{CommonName: csrSubjectCommonName},
}, key)
if err != nil {
return "", fmt.Errorf("building the certificate signing request: %w", err)
Expand Down
16 changes: 11 additions & 5 deletions internal/material/x509/keypair_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ func TestGenerateKeyRejectsAnUnknownType(t *testing.T) {
"the error has to name the types an operator can choose instead")
}

// TestBuildCSRCarriesNothingButTheKey is the definition-of-done assertion on the
// request: it carries a public key and a proof of possession, and nothing that
// could be read as the agent naming an identity.
func TestBuildCSRCarriesNothingButTheKey(t *testing.T) {
// TestBuildCSRNamesNoIdentity is the definition-of-done assertion on the request: it
// carries a public key, a proof of possession, and a fixed placeholder subject, and
// nothing that could be read as the agent naming an identity.
func TestBuildCSRNamesNoIdentity(t *testing.T) {
key, err := generateKey(workloadidentity.DefaultKeyType)
require.NoError(t, err)

Expand All @@ -114,7 +114,13 @@ func TestBuildCSRCarriesNothingButTheKey(t *testing.T) {
assert.NoError(t, csr.CheckSignature(), "the request has to prove possession of the key")
assert.Equal(t, key.Public(), csr.PublicKey, "the request has to carry this issuance's public key")

assert.Empty(t, csr.Subject.String(), "a populated subject would be discarded by STS and would mislead a reader into thinking the agent picks the identity")
// The subject is the fixed placeholder and nothing else: a blank one is currently
// rejected by the signer, and a workload-specific one would say the agent picks the
// identity. Asserting the exact value keeps this pinned to the one string that has
// to be deleted when STS starts overwriting the subject itself.
assert.Equal(t, csrSubjectCommonName, csr.Subject.CommonName)
assert.Equal(t, "CN="+csrSubjectCommonName, csr.Subject.String(),
"the subject carries the placeholder common name and no other attribute")
assert.Empty(t, csr.Extensions, "the request carries no extensions")
assert.Empty(t, csr.ExtraExtensions, "the request carries no extensions")
assert.Empty(t, csr.DNSNames)
Expand Down
Loading