Skip to content

Commit

Permalink
Rely on the authority ID instead of the issued time when updating the…
Browse files Browse the repository at this point in the history 
… journal (#5622)

Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
  • Loading branch information
@amartinezfayo
amartinezfayo authored Nov 19, 2024
1 parent a8857ba commit a746e98
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 35 deletions.
29 changes: 11 additions & 18 deletions pkg/server/ca/manager/journal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type journalConfig struct {
}

// Journal stores X509 CAs and JWT keys on disk as they are rotated by the
// manager. The data format on disk is a PEM encoded protocol buffer.
// manager. The data format is a PEM encoded protocol buffer.
type Journal struct {
config *journalConfig

Expand Down Expand Up @@ -87,21 +87,17 @@ func (j *Journal) AppendX509CA(ctx context.Context, slotID string, issuedAt time
return nil
}

// UpdateX509CAStatus updates a stored X509CA entry to have the given status, updating the journal file.
func (j *Journal) UpdateX509CAStatus(ctx context.Context, issuedAt time.Time, status journal.Status) error {
// UpdateX509CAStatus updates a stored X509CA entry to have the given status,
// updating the CA journal.
func (j *Journal) UpdateX509CAStatus(ctx context.Context, authorityID string, status journal.Status) error {
j.mu.Lock()
defer j.mu.Unlock()

backup := j.entries.X509CAs

// Once we have the authorityID, we can use it to search for an entry,
// but for now, we depend on issuedAt.
issuedAtUnix := issuedAt.Unix()

var found bool
for i := len(j.entries.X509CAs) - 1; i >= 0; i-- {
entry := j.entries.X509CAs[i]
if issuedAtUnix == entry.IssuedAt {
if authorityID == entry.AuthorityId {
found = true
entry.Status = status
if status == journal.Status_ACTIVE {
Expand All @@ -112,7 +108,7 @@ func (j *Journal) UpdateX509CAStatus(ctx context.Context, issuedAt time.Time, st
}

if !found {
return fmt.Errorf("no journal entry found issued at: %v", issuedAtUnix)
return fmt.Errorf("no journal entry found with authority ID %q", authorityID)
}

if err := j.save(ctx); err != nil {
Expand Down Expand Up @@ -159,29 +155,26 @@ func (j *Journal) AppendJWTKey(ctx context.Context, slotID string, issuedAt time
return nil
}

// UpdateJWTKeyStatus updates a stored JWTKey entry to have the given status, updating the journal file.
func (j *Journal) UpdateJWTKeyStatus(ctx context.Context, issuedAt time.Time, status journal.Status) error {
// UpdateJWTKeyStatus updates a stored JWTKey entry to have the given status,
// updating the CA journal.
func (j *Journal) UpdateJWTKeyStatus(ctx context.Context, authorityID string, status journal.Status) error {
j.mu.Lock()
defer j.mu.Unlock()

backup := j.entries.JwtKeys

// Once we have the authorityID, we can use it to search for an entry,
// but for now we depend on issuedAt.
issuedAtUnix := issuedAt.Unix()

var found bool
for i := len(j.entries.JwtKeys) - 1; i >= 0; i-- {
entry := j.entries.JwtKeys[i]
if issuedAtUnix == entry.IssuedAt {
if authorityID == entry.AuthorityId {
found = true
entry.Status = status
break
}
}

if !found {
return fmt.Errorf("no journal entry found issued at: %v", issuedAtUnix)
return fmt.Errorf("no journal entry found with authority ID %q", authorityID)
}

if err := j.save(ctx); err != nil {
Expand Down
28 changes: 15 additions & 13 deletions pkg/server/ca/manager/journal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"errors"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -32,9 +33,10 @@ var (
{Raw: []byte("C")},
}

km keymanager.KeyManager
kmKeys = map[string]keymanager.Key{}
rootCerts = map[string]*x509.Certificate{}
km keymanager.KeyManager
kmKeys = map[string]keymanager.Key{}
rootCerts = map[string]*x509.Certificate{}
nonExistingAuthorityID = "non-existing-authority-id"
)

func setupJournalTest(t *testing.T) *journalTest {
Expand Down Expand Up @@ -118,7 +120,8 @@ func TestJournalPersistence(t *testing.T) {
})
require.NoError(t, err)

require.NoError(t, j.UpdateX509CAStatus(ctx, now, journal.Status_ACTIVE))
authorityIDA := x509util.SubjectKeyIDToString(rootCerts["X509-Root-A"].SubjectKeyId)
require.NoError(t, j.UpdateX509CAStatus(ctx, authorityIDA, journal.Status_ACTIVE))

// Check that the CA journal was properly stored in the datastore.
journalDS := test.loadJournal(t)
Expand All @@ -134,7 +137,7 @@ func TestJournalPersistence(t *testing.T) {
UpstreamChain: testChain,
})
require.NoError(t, err)
require.NoError(t, j.UpdateX509CAStatus(ctx, now, journal.Status_ACTIVE))
require.NoError(t, j.UpdateX509CAStatus(ctx, authorityIDA, journal.Status_ACTIVE))

journalDS = test.loadJournal(t)
require.NotNil(t, journalDS)
Expand Down Expand Up @@ -237,7 +240,8 @@ func TestUpdateX509CAStatus(t *testing.T) {
require.Equal(t, journal.Status_PREPARED, ca.Status)
}

err = testJournal.UpdateX509CAStatus(ctx, secondIssuedAt, journal.Status_ACTIVE)
authorityIDB := x509util.SubjectKeyIDToString(rootCerts["X509-Root-B"].SubjectKeyId)
err = testJournal.UpdateX509CAStatus(ctx, authorityIDB, journal.Status_ACTIVE)
require.NoError(t, err)

for _, ca := range testJournal.getEntries().X509CAs {
Expand All @@ -249,9 +253,8 @@ func TestUpdateX509CAStatus(t *testing.T) {
require.Equal(t, expectedStatus, ca.Status)
}

unusedTime := test.now().Add(time.Hour)
err = testJournal.UpdateX509CAStatus(ctx, unusedTime, journal.Status_OLD)
require.ErrorContains(t, err, "no journal entry found issued at:")
err = testJournal.UpdateX509CAStatus(ctx, nonExistingAuthorityID, journal.Status_OLD)
require.ErrorContains(t, err, fmt.Sprintf("no journal entry found with authority ID %q", nonExistingAuthorityID))
}

func TestUpdateJWTKeyStatus(t *testing.T) {
Expand Down Expand Up @@ -287,7 +290,7 @@ func TestUpdateJWTKeyStatus(t *testing.T) {
require.Equal(t, journal.Status_PREPARED, key.Status)
}

err = testJournal.UpdateJWTKeyStatus(ctx, secondIssuedAt, journal.Status_ACTIVE)
err = testJournal.UpdateJWTKeyStatus(ctx, "kid2", journal.Status_ACTIVE)
require.NoError(t, err)

for _, key := range testJournal.getEntries().JwtKeys {
Expand All @@ -299,9 +302,8 @@ func TestUpdateJWTKeyStatus(t *testing.T) {
require.Equal(t, expectedStatus, key.Status)
}

unusedTime := test.now().Add(time.Hour)
err = testJournal.UpdateJWTKeyStatus(ctx, unusedTime, journal.Status_OLD)
require.ErrorContains(t, err, "no journal entry found issued at:")
err = testJournal.UpdateJWTKeyStatus(ctx, nonExistingAuthorityID, journal.Status_OLD)
require.ErrorContains(t, err, fmt.Sprintf("no journal entry found with authority ID %q", nonExistingAuthorityID))
}

func TestJWTKeyOverflow(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/server/ca/manager/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (m *Manager) RotateX509CA(ctx context.Context) {

m.currentX509CA, m.nextX509CA = m.nextX509CA, m.currentX509CA
m.nextX509CA.Reset()
if err := m.journal.UpdateX509CAStatus(ctx, m.nextX509CA.issuedAt, journal.Status_OLD); err != nil {
if err := m.journal.UpdateX509CAStatus(ctx, m.nextX509CA.AuthorityID(), journal.Status_OLD); err != nil {
m.c.Log.WithError(err).Error("Failed to update status on X509CA journal entry")
}

Expand Down Expand Up @@ -396,7 +396,7 @@ func (m *Manager) RotateJWTKey(ctx context.Context) {
m.currentJWTKey, m.nextJWTKey = m.nextJWTKey, m.currentJWTKey
m.nextJWTKey.Reset()

if err := m.journal.UpdateJWTKeyStatus(ctx, m.nextJWTKey.issuedAt, journal.Status_OLD); err != nil {
if err := m.journal.UpdateJWTKeyStatus(ctx, m.nextJWTKey.AuthorityID(), journal.Status_OLD); err != nil {
m.c.Log.WithError(err).Error("Failed to update status on JWTKey journal entry")
}

Expand Down Expand Up @@ -534,7 +534,7 @@ func (m *Manager) activateJWTKey(ctx context.Context) {
telemetry_server.IncrActivateJWTKeyManagerCounter(m.c.Metrics)

m.currentJWTKey.status = journal.Status_ACTIVE
if err := m.journal.UpdateJWTKeyStatus(ctx, m.currentJWTKey.issuedAt, journal.Status_ACTIVE); err != nil {
if err := m.journal.UpdateJWTKeyStatus(ctx, m.currentJWTKey.AuthorityID(), journal.Status_ACTIVE); err != nil {
log.WithError(err).Error("Failed to update to activated status on JWTKey journal entry")
}

Expand All @@ -553,7 +553,7 @@ func (m *Manager) activateX509CA(ctx context.Context) {
telemetry_server.IncrActivateX509CAManagerCounter(m.c.Metrics)

m.currentX509CA.status = journal.Status_ACTIVE
if err := m.journal.UpdateX509CAStatus(ctx, m.currentX509CA.issuedAt, journal.Status_ACTIVE); err != nil {
if err := m.journal.UpdateX509CAStatus(ctx, m.currentX509CA.AuthorityID(), journal.Status_ACTIVE); err != nil {
log.WithError(err).Error("Failed to update to activated status on X509CA journal entry")
}

Expand Down

0 comments on commit a746e98

Please sign in to comment.