Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rely on the authority ID instead of the issued time when updating the status of keys in the journal #5622

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Rely on the authority ID instead of the issued time when updating the status of keys in the journal #5622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bf71e0
Rely on the authority ID instead of the issued time when updating the…
amartinezfayo Nov 1, 2024
296b127
Merge branch 'main' into journal-update-with-authority-id
amartinezfayo Nov 5, 2024
95f0589
Merge branch 'main' into journal-update-with-authority-id
MarcosDY Nov 7, 2024
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
29 changes: 11 additions & 18 deletions pkg/server/ca/manager/journal.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading