PR changes

Signed-off-by: Marcos Yacob <[email protected]>

pkg/common/coretypes/jwtkey/jwtkey.go

@@ -15,20 +15,19 @@ type JWTKey struct {
 	Tainted   bool
 }
 
-func toProtoFields(jwtKey JWTKey) (string, []byte, int64, bool, error) {
+func toProtoFields(jwtKey JWTKey) (id string, publicKey []byte, expiresAt int64, tainted bool, err error) {
 	if jwtKey.ID == "" {
 		return "", nil, 0, false, errors.New("missing key ID for JWT key")
 	}
 
 	if jwtKey.PublicKey == nil {
 		return "", nil, 0, false, fmt.Errorf("missing public key for JWT key %q", jwtKey.ID)
 	}
-	publicKey, err := x509.MarshalPKIXPublicKey(jwtKey.PublicKey)
+	publicKey, err = x509.MarshalPKIXPublicKey(jwtKey.PublicKey)
 	if err != nil {
 		return "", nil, 0, false, fmt.Errorf("failed to marshal public key for JWT key %q: %w", jwtKey.ID, err)
 	}
 
-	var expiresAt int64
 	if !jwtKey.ExpiresAt.IsZero() {
 		expiresAt = jwtKey.ExpiresAt.Unix()
 	}

pkg/common/telemetry/names.go

@@ -541,7 +541,7 @@ const (
 	// with other tags to add clarity
 	Subject = "subject"
 
-	// Subject tags a certificate subject key ID
+	// SubjectKeyID tags a certificate subject key ID
 	SubjectKeyID = "subject_key_id"
 
 	// SVIDMapSize is the gauge key for the size of the LRU cache SVID map

pkg/server/api/localauthority/v1/service.go

@@ -461,7 +461,7 @@ func (s *Service) RevokeX509UpstreamAuthority(ctx context.Context, req *localaut
 	}
 
 	rpccontext.AuditRPC(ctx)
-	log.Info("X.509 upstream authority revoked successfully")
+	log.Info("X.509 upstream authority successfully revoked")
 
 	return &localauthorityv1.RevokeX509UpstreamAuthorityResponse{}, nil
 }
@@ -495,7 +495,7 @@ func (s *Service) validateUpstreamAuthoritySubjectKey(subjectKeyIDRequest string
 
 	nextSlot := s.ca.GetNextX509CASlot()
 	if subjectKeyIDRequest != nextSlot.UpstreamAuthorityID() {
-		return errors.New("upstream authority is not signing Old local authority")
+		return errors.New("upstream authority didn't sign the old local authority")
 	}
 
 	if nextSlot.Status() == journal.Status_PREPARED {

pkg/server/api/localauthority/v1/service_test.go

@@ -1298,8 +1298,6 @@ func TestTaintX509Authority(t *testing.T) {
 
 	nextCA, nextKey, err := testutil.SelfSign(template)
 	require.NoError(t, err)
-	// nextPublicKeyRaw, err := x509.MarshalPKIXPublicKey(nextKey.Public())
-	// require.NoError(t, err)
 	nextKeySKI, err := x509util.GetSubjectKeyID(nextKey.Public())
 	require.NoError(t, err)
 	nextAuthorityID := x509util.SubjectKeyIDToString(nextKeySKI)
@@ -1572,7 +1570,7 @@ func TestTaintX509Authority(t *testing.T) {
 
 func TestTaintX509UpstreamAuthority(t *testing.T) {
 	getUpstreamCertAndSubjectID := func(ca *testca.CA) (*x509.Certificate, string) {
-		// Selfsigned CA will return itself
+		// Self signed CA will return itself
 		cert := ca.X509Authorities()[0]
 		return cert, x509util.SubjectKeyIDToString(cert.SubjectKeyId)
 	}
@@ -1729,13 +1727,13 @@ func TestTaintX509UpstreamAuthority(t *testing.T) {
 			nextSlot:            createSlotWithUpstream(journal.Status_OLD, nextIntermediateCA, notAfterNext),
 			subjectKeyIDToTaint: "invalidID",
 			expectCode:          codes.InvalidArgument,
-			expectMsg:           "provided subject key id is not valid: upstream authority is not signing Old local authority",
+			expectMsg:           "provided subject key id is not valid: upstream authority didn't sign the old local authority",
 			expectLogs: []spiretest.LogEntry{
 				{
 					Level:   logrus.ErrorLevel,
 					Message: "Invalid argument: provided subject key id is not valid",
 					Data: logrus.Fields{
-						logrus.ErrorKey:        "upstream authority is not signing Old local authority",
+						logrus.ErrorKey:        "upstream authority didn't sign the old local authority",
 						telemetry.SubjectKeyID: "invalidID",
 					},
 				},
@@ -1745,7 +1743,7 @@ func TestTaintX509UpstreamAuthority(t *testing.T) {
 					Data: logrus.Fields{
 						telemetry.Status:        "error",
 						telemetry.StatusCode:    "InvalidArgument",
-						telemetry.StatusMessage: "provided subject key id is not valid: upstream authority is not signing Old local authority",
+						telemetry.StatusMessage: "provided subject key id is not valid: upstream authority didn't sign the old local authority",
 						telemetry.Type:          "audit",
 						telemetry.SubjectKeyID:  "invalidID",
 					},
@@ -2120,7 +2118,7 @@ func TestRevokeX509Authority(t *testing.T) {
 
 func TestRevokeX509UpstreamAuthority(t *testing.T) {
 	getUpstreamCertAndSubjectID := func(ca *testca.CA) (*x509.Certificate, string) {
-		// Selfsigned CA will return itself
+		// Self signed CA will return itself
 		cert := ca.X509Authorities()[0]
 		return cert, x509util.SubjectKeyIDToString(cert.SubjectKeyId)
 	}
@@ -2171,7 +2169,7 @@ func TestRevokeX509UpstreamAuthority(t *testing.T) {
 				},
 				{
 					Level:   logrus.InfoLevel,
-					Message: "X.509 upstream authority revoked successfully",
+					Message: "X.509 upstream authority successfully revoked",
 					Data: logrus.Fields{
 						telemetry.SubjectKeyID: deactivatedUpstreamAuthorityID,
 					},
@@ -2268,13 +2266,13 @@ func TestRevokeX509UpstreamAuthority(t *testing.T) {
 			nextSlot:             createSlotWithUpstream(journal.Status_OLD, nextIntermediateCA, notAfterNext),
 			subjectKeyIDToRevoke: "invalidID",
 			expectCode:           codes.InvalidArgument,
-			expectMsg:            "invalid subject key ID: upstream authority is not signing Old local authority",
+			expectMsg:            "invalid subject key ID: upstream authority didn't sign the old local authority",
 			expectLogs: []spiretest.LogEntry{
 				{
 					Level:   logrus.ErrorLevel,
 					Message: "Invalid argument: invalid subject key ID",
 					Data: logrus.Fields{
-						logrus.ErrorKey:        "upstream authority is not signing Old local authority",
+						logrus.ErrorKey:        "upstream authority didn't sign the old local authority",
 						telemetry.SubjectKeyID: "invalidID",
 					},
 				},
@@ -2284,7 +2282,7 @@ func TestRevokeX509UpstreamAuthority(t *testing.T) {
 					Data: logrus.Fields{
 						telemetry.Status:        "error",
 						telemetry.StatusCode:    "InvalidArgument",
-						telemetry.StatusMessage: "invalid subject key ID: upstream authority is not signing Old local authority",
+						telemetry.StatusMessage: "invalid subject key ID: upstream authority didn't sign the old local authority",
 						telemetry.Type:          "audit",
 						telemetry.SubjectKeyID:  "invalidID",
 					},

pkg/server/ca/manager/manager_test.go

@@ -303,12 +303,12 @@ func TestUpstreamSigned(t *testing.T) {
 	err := fakeUA.TaintAuthority(0)
 	require.NoError(t, err)
 
-	// Get root again an verify firt item is tainted
+	// Get the roots again and verify that the first X.509 authority is tainted
 	x509Roots := fakeUA.X509Roots()
 	require.True(t, x509Roots[0].Tainted)
 
 	commonCertificates := x509certificate.RequireToCommonProtos(x509Roots)
-	// Retry until tainted is propagated to database
+	// Retry until the Tainted attribute is propagated to the database
 	require.Eventually(t, func() bool {
 		bundle := test.fetchBundle(ctx)
 		return spiretest.AssertProtoListEqual(t, commonCertificates, bundle.RootCas)

pkg/server/ca/manager/slot.go

@@ -42,7 +42,7 @@ type Slot interface {
 	Status() journal.Status
 	UpstreamAuthorityID() string
 	AuthorityID() string
-	// TODO: Should we remove public key?
+	// TODO: This will be removed as part of #5390
 	PublicKey() crypto.PublicKey
 	NotAfter() time.Time
 }

pkg/server/ca/upstream_client.go

@@ -140,7 +140,7 @@ func (u *UpstreamClient) runMintX509CAStream(ctx context.Context, csr []byte, tt
 	}
 	defer x509RootsStream.Close()
 
-	// Extract all roots certificates
+	// Extract all root certificates
 	var x509RootCerts []*x509.Certificate
 	for _, eachRoot := range x509Roots {
 		x509RootCerts = append(x509RootCerts, eachRoot.Certificate)

pkg/server/datastore/sqlstore/sqlstore.go

@@ -1398,9 +1398,11 @@ func taintX509CA(tx *gorm.DB, trustDomainID string, subjectKeyIDToTaint string)
 	}
 
 	if !found {
-		return status.Errorf(codes.NotFound, "no ca found with provided subject key ID")
+		return status.Error(codes.NotFound, "no ca found with provided subject key ID")
 	}
 
+	bundle.SequenceNumber++
+
 	_, err = updateBundle(tx, bundle, nil)
 	if err != nil {
 		return err
@@ -1440,6 +1442,7 @@ func revokeX509CA(tx *gorm.DB, trustDomainID string, subjectKeyIDToRevoke string
 	}
 
 	bundle.RootCas = rootCAs
+	bundle.SequenceNumber++
 
 	if _, err := updateBundle(tx, bundle, nil); err != nil {
 		return status.Errorf(codes.Internal, "failed to update bundle: %v", err)
@@ -1478,6 +1481,7 @@ func taintJWTKey(tx *gorm.DB, trustDomainID string, authorityID string) (*common
 		return nil, status.Error(codes.NotFound, "no JWT Key found with provided key ID")
 	}
 
+	bundle.SequenceNumber++
 	if _, err := updateBundle(tx, bundle, nil); err != nil {
 		return nil, err
 	}
@@ -1517,6 +1521,7 @@ func revokeJWTKey(tx *gorm.DB, trustDomainID string, authorityID string) (*commo
 		return nil, status.Error(codes.NotFound, "no JWT Key found with provided key ID")
 	}
 
+	bundle.SequenceNumber++
 	if _, err := updateBundle(tx, bundle, nil); err != nil {
 		return nil, err
 	}