Skip to content

Commit

Permalink
server: check NotAfter when loading journal (#5836)
Browse files Browse the repository at this point in the history 
Otherwise we can end up with expired CAs which leads to issues

Signed-off-by: Sorin Dumitru <[email protected]>
  • Loading branch information
@sorindumitru
sorindumitru authored Feb 6, 2025
1 parent 433a9b2 commit 9e09851
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 0 deletions.
8 changes: 8 additions & 0 deletions pkg/server/ca/manager/slot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ func (s *SlotLoader) loadX509CASlotFromEntry(ctx context.Context, entry *journal
return nil, "no slot id", nil
}

if entry.GetNotAfter() < time.Now().Unix() {
return nil, "slot expired", nil
}

cert, err := x509.ParseCertificate(entry.Certificate)
if err != nil {
return nil, "", fmt.Errorf("unable to parse CA certificate: %w", err)
Expand Down Expand Up @@ -417,6 +421,10 @@ func (s *SlotLoader) loadJWTKeySlotFromEntry(ctx context.Context, entry *journal
return nil, "no slot id", nil
}

if entry.GetNotAfter() < time.Now().Unix() {
return nil, "slot expired", nil
}

publicKey, err := x509.ParsePKIXPublicKey(entry.PublicKey)
if err != nil {
return nil, "", err
Expand Down
99 changes: 99 additions & 0 deletions pkg/server/ca/manager/slot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func TestJournalLoad(t *testing.T) {
X509CAs: []*journal.X509CAEntry{
{
SlotId: "B",
NotAfter: notAfterUnix,
IssuedAt: secondIssuedAtUnix,
Certificate: x509RootB.Raw,
},
Expand Down Expand Up @@ -288,18 +289,21 @@ func TestJournalLoad(t *testing.T) {
{
SlotId: "A",
IssuedAt: firstIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
AuthorityId: "3",
},
{
SlotId: "B",
IssuedAt: secondIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootB.Raw,
AuthorityId: "2",
},
{
SlotId: "A",
IssuedAt: thirdIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
AuthorityId: "1",
},
Expand Down Expand Up @@ -399,6 +403,7 @@ func TestJournalLoad(t *testing.T) {
{
SlotId: "A",
IssuedAt: thirdIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
Status: journal.Status_PREPARED,
AuthorityId: "1",
Expand Down Expand Up @@ -466,20 +471,23 @@ func TestJournalLoad(t *testing.T) {
{
SlotId: "A",
IssuedAt: firstIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
Status: journal.Status_OLD,
AuthorityId: "3",
},
{
SlotId: "B",
IssuedAt: secondIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootB.Raw,
Status: journal.Status_OLD,
AuthorityId: "2",
},
{
SlotId: "A",
IssuedAt: thirdIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
Status: journal.Status_ACTIVE,
AuthorityId: "1",
Expand Down Expand Up @@ -584,20 +592,23 @@ func TestJournalLoad(t *testing.T) {
{
SlotId: "A",
IssuedAt: firstIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootA.Raw,
Status: journal.Status_ACTIVE,
AuthorityId: "3",
},
{
SlotId: "B",
IssuedAt: secondIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootB.Raw,
Status: journal.Status_OLD,
AuthorityId: "2",
},
{
SlotId: "B",
IssuedAt: thirdIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: x509RootB.Raw,
Status: journal.Status_PREPARED,
AuthorityId: "1",
Expand Down Expand Up @@ -702,6 +713,7 @@ func TestJournalLoad(t *testing.T) {
{
SlotId: "A",
IssuedAt: firstIssuedAtUnix,
NotAfter: notAfterUnix,
Certificate: []byte("foo"),
Status: journal.Status_ACTIVE,
AuthorityId: "1",
Expand Down Expand Up @@ -733,6 +745,50 @@ func TestJournalLoad(t *testing.T) {
},
},
},
{
name: "Expired X.509 entry",
entries: &journal.Entries{
X509CAs: []*journal.X509CAEntry{
{
SlotId: "A",
IssuedAt: firstIssuedAtUnix,
NotAfter: time.Now().Add(-time.Minute).Unix(),
Certificate: x509RootA.Raw,
Status: journal.Status_ACTIVE,
AuthorityId: "1",
UpstreamAuthorityId: "2",
},
},
},
expectSlots: map[SlotPosition]Slot{
CurrentX509CASlot: newX509CASlot("A"),
NextX509CASlot: newX509CASlot("B"),
CurrentJWTKeySlot: newJWTKeySlot("A"),
NextJWTKeySlot: newJWTKeySlot("B"),
},
expectLogs: []spiretest.LogEntry{
{
Level: logrus.InfoLevel,
Message: "Journal loaded",
Data: logrus.Fields{
telemetry.X509CAs: "1",
telemetry.JWTKeys: "0",
},
},
{
Level: logrus.WarnLevel,
Message: "X509CA slot unusable",
Data: logrus.Fields{
logrus.ErrorKey: "slot expired",
telemetry.IssuedAt: firstIssuedAt.String(),
telemetry.Slot: "A",
telemetry.Status: "ACTIVE",
telemetry.LocalAuthorityID: "1",
telemetry.UpstreamAuthorityID: "2",
},
},
},
},
{
name: "Invalid JWTKey entry",
entries: &journal.Entries{
Expand Down Expand Up @@ -771,6 +827,49 @@ func TestJournalLoad(t *testing.T) {
},
},
},
{
name: "Expired JWTKey entry",
entries: &journal.Entries{
JwtKeys: []*journal.JWTKeyEntry{
{
SlotId: "B",
IssuedAt: thirdIssuedAtUnix,
Kid: "kid3",
NotAfter: time.Now().Add(-time.Minute).Unix(),
PublicKey: jwtKeyAPKIX,
Status: journal.Status_ACTIVE,
AuthorityId: "a",
},
},
},
expectSlots: map[SlotPosition]Slot{
CurrentX509CASlot: newX509CASlot("A"),
NextX509CASlot: newX509CASlot("B"),
CurrentJWTKeySlot: newJWTKeySlot("A"),
NextJWTKeySlot: newJWTKeySlot("B"),
},
expectLogs: []spiretest.LogEntry{
{
Level: logrus.InfoLevel,
Message: "Journal loaded",
Data: logrus.Fields{
telemetry.X509CAs: "0",
telemetry.JWTKeys: "1",
},
},
{
Level: logrus.WarnLevel,
Message: "JWT key slot unusable",
Data: logrus.Fields{
logrus.ErrorKey: "slot expired",
telemetry.IssuedAt: thirdIssuedAt.String(),
telemetry.Slot: "B",
telemetry.Status: "ACTIVE",
telemetry.LocalAuthorityID: "a",
},
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
loghook.Reset()
Expand Down

0 comments on commit 9e09851

Please sign in to comment.