Skip to content

Commit

Permalink
Log SPIFFE ID for X.509-SVIDs signed in BatchNewX509SVID (spiffe#4902)
Browse files Browse the repository at this point in the history 
The audit log emitted on calls to BatchNewX509SVID doesn't include the
SPIFFE ID of the X.509-SVIDs that are signed during the API handler
execution. It's valuable to include the SPIFFE ID in this log message
for traceability and auditing purposes. The SPIFFE ID in signed
X.509-SVIDs is currently only included in a DEBUG level log in the
server CA.

Signed-off-by: Ryan Turner <turner@uber.com>
  • Loading branch information
@rturner3
rturner3 authored Feb 27, 2024
1 parent 49f8857 commit 31b3cb1
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/server/api/svid/v1/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/spiffe/go-spiffe/v2/spiffeid"
svidv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/svid/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
"github.com/spiffe/spire/pkg/common/idutil"
"github.com/spiffe/spire/pkg/common/jwtsvid"
"github.com/spiffe/spire/pkg/common/telemetry"
"github.com/spiffe/spire/pkg/common/x509util"
Expand Down Expand Up @@ -175,10 +176,19 @@ func (s *Service) BatchNewX509SVID(ctx context.Context, req *svidv1.BatchNewX509
// Create new SVID
r := s.newX509SVID(ctx, svidParam, entriesMap)
results = append(results, r)
spiffeID := ""
if r.Svid != nil {
id, err := idutil.IDProtoString(r.Svid.Id)
if err == nil {
spiffeID = id
}
}

rpccontext.AuditRPCWithTypesStatus(ctx, r.Status, func() logrus.Fields {
fields := logrus.Fields{
telemetry.Csr: api.HashByte(svidParam.Csr),
telemetry.RegistrationID: svidParam.EntryId,
telemetry.SPIFFEID: spiffeID,
}

if r.Svid != nil {
Expand Down
14 changes: 14 additions & 0 deletions pkg/server/api/svid/v1/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "workload",
telemetry.Csr: api.HashByte(m["workload"]),
telemetry.ExpiresAt: expiresAtFromCAStr,
telemetry.SPIFFEID: "spiffe://example.org/workload1",
},
},
}
Expand All @@ -1236,6 +1237,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "ttl",
telemetry.Csr: api.HashByte(m["ttl"]),
telemetry.ExpiresAt: expiresAtFromTTLEntryStr,
telemetry.SPIFFEID: "spiffe://example.org/ttl",
},
},
}
Expand All @@ -1259,6 +1261,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "x509ttl",
telemetry.Csr: api.HashByte(m["x509ttl"]),
telemetry.ExpiresAt: expiresAtFromX509TTLEntryStr,
telemetry.SPIFFEID: "spiffe://example.org/ttl",
},
},
}
Expand All @@ -1282,6 +1285,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "dns",
telemetry.Csr: api.HashByte(m["dns"]),
telemetry.ExpiresAt: expiresAtFromCAStr,
telemetry.SPIFFEID: "spiffe://example.org/dns",
},
},
}
Expand Down Expand Up @@ -1314,6 +1318,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "workload",
telemetry.Csr: api.HashByte(m["workload"]),
telemetry.ExpiresAt: expiresAtFromCAStr,
telemetry.SPIFFEID: "spiffe://example.org/workload1",
},
},
{
Expand All @@ -1334,6 +1339,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["invalid"]),
telemetry.StatusCode: "Internal",
telemetry.StatusMessage: "entry has malformed SPIFFE ID: request must specify SPIFFE ID",
telemetry.SPIFFEID: "",
},
},
{
Expand All @@ -1345,6 +1351,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.RegistrationID: "dns",
telemetry.Csr: api.HashByte(m["dns"]),
telemetry.ExpiresAt: expiresAtFromCAStr,
telemetry.SPIFFEID: "spiffe://example.org/dns",
},
},
}
Expand Down Expand Up @@ -1477,6 +1484,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m[""]),
telemetry.StatusCode: "InvalidArgument",
telemetry.StatusMessage: "missing entry ID",
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1511,6 +1519,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: "",
telemetry.StatusCode: "InvalidArgument",
telemetry.StatusMessage: "missing CSR",
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1545,6 +1554,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["invalid entry"]),
telemetry.StatusCode: "NotFound",
telemetry.StatusMessage: "entry not found or not authorized",
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1583,6 +1593,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["workload"]),
telemetry.StatusCode: "InvalidArgument",
telemetry.StatusMessage: fmt.Sprintf("malformed CSR: %v", invalidCsrErr),
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1624,6 +1635,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["workload"]),
telemetry.StatusCode: "InvalidArgument",
telemetry.StatusMessage: "invalid CSR signature: x509: ECDSA verification failure",
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1659,6 +1671,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["invalid"]),
telemetry.StatusCode: "Internal",
telemetry.StatusMessage: "entry has malformed SPIFFE ID: request must specify SPIFFE ID",
telemetry.SPIFFEID: "",
},
},
}
Expand Down Expand Up @@ -1696,6 +1709,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
telemetry.Csr: api.HashByte(m["workload"]),
telemetry.StatusCode: "Internal",
telemetry.StatusMessage: "failed to sign X509-SVID: oh no",
telemetry.SPIFFEID: "",
},
},
}
Expand Down

0 comments on commit 31b3cb1

Please sign in to comment.