Enforce reattest to renew on server

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

@@ -13,6 +13,7 @@ import (
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	agentv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/agent/v1"
 	"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
+	"github.com/spiffe/spire/pkg/common/errorutil"
 	"github.com/spiffe/spire/pkg/common/idutil"
 	"github.com/spiffe/spire/pkg/common/nodeutil"
 	"github.com/spiffe/spire/pkg/common/selector"
@@ -433,10 +434,9 @@ func (s *Service) RenewAgent(ctx context.Context, req *agentv1.RenewAgentRequest
 	}
 
 	// Agent attempted to renew when it should've been reattesting
-	// TODO: Uncomment in 1.10.0
-	/*if attestedNode.CanReattest {
-		return nil, errorutil.PermissionDenied(types.PermissionDeniedDetails_AGENT_MUST_REATTEST, "agent can't renew SVID, must reattest")
-	}*/
+	if attestedNode.CanReattest {
+		return nil, errorutil.PermissionDenied(types.PermissionDeniedDetails_AGENT_MUST_REATTEST, "agent must reattest instead of renew")
+	}
 
 	log.Info("Renewing agent SVID")
 

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

@@ -1668,6 +1668,9 @@ func TestRenewAgent(t *testing.T) {
 		CertSerialNumber:    "6789",
 	}
 
+	reattestableNode := cloneAttestedNode(defaultNode)
+	reattestableNode.CanReattest = true
+
 	// Create a test CSR with empty template
 	csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{}, testKey)
 	require.NoError(t, err)
@@ -1695,6 +1698,7 @@ func TestRenewAgent(t *testing.T) {
 		req            *agentv1.RenewAgentRequest
 		expectCode     codes.Code
 		expectMsg      string
+		expectDetail   proto.Message
 		rateLimiterErr error
 	}{
 		{
@@ -1951,6 +1955,31 @@ func TestRenewAgent(t *testing.T) {
 			expectCode: codes.Internal,
 			expectMsg:  "failed to fetch agent: some error",
 		},
+		{
+			name:       "can reattest instead",
+			createNode: reattestableNode,
+			expectLogs: []spiretest.LogEntry{
+				{
+					Level:   logrus.InfoLevel,
+					Message: "API accessed",
+					Data: logrus.Fields{
+						telemetry.Status:        "error",
+						telemetry.Type:          "audit",
+						telemetry.Csr:           csrHash,
+						telemetry.StatusCode:    "PermissionDenied",
+						telemetry.StatusMessage: "agent must reattest instead of renew",
+					},
+				},
+			},
+			req: &agentv1.RenewAgentRequest{
+				Params: &agentv1.AgentX509SVIDParams{
+					Csr: csr,
+				},
+			},
+			expectCode:   codes.PermissionDenied,
+			expectMsg:    "agent must reattest instead of renew",
+			expectDetail: &types.PermissionDeniedDetails{Reason: types.PermissionDeniedDetails_AGENT_MUST_REATTEST},
+		},
 	} {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
@@ -1983,6 +2012,13 @@ func TestRenewAgent(t *testing.T) {
 			// Send param message
 			resp, err := test.client.RenewAgent(ctx, tt.req)
 			spiretest.RequireGRPCStatus(t, err, tt.expectCode, tt.expectMsg)
+			st := status.Convert(err)
+			if tt.expectDetail == nil {
+				require.Empty(t, st.Details())
+			} else {
+				require.Len(t, st.Details(), 1)
+				spiretest.RequireProtoEqual(t, tt.expectDetail, st.Details()[0].(proto.Message))
+			}
 
 			if tt.expectCode != codes.OK {
 				require.Nil(t, resp)

test/integration/setup/node-attestation/client.go

@@ -22,7 +22,6 @@ import (
 	"github.com/spiffe/spire/test/integration/setup/itclient"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	"google.golang.org/protobuf/proto"
 )
 
 var (
@@ -208,14 +207,14 @@ func doX509popStep(ctx context.Context) error {
 	client := c.AgentClient()
 
 	// Attest agent
-	svidResp, err := x509popAttest(ctx)
-	if err != nil {
+	if _, err := x509popAttest(ctx); err != nil {
 		return fmt.Errorf("failed to attest: %w", err)
 	}
 
-	// Renew agent
-	if err := x509popRenew(ctx, svidResp); err != nil {
-		return fmt.Errorf("failed to renew agent: %w", err)
+	// Reattest agent to "renew"
+	svidResp, err := x509popAttest(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to re-attest agent for renewal: %w", err)
 	}
 
 	// Delete agent
@@ -347,40 +346,6 @@ func x509popAttest(ctx context.Context) (*types.X509SVID, error) {
 	return resp.GetResult().Svid, nil
 }
 
-// x509popRenew creates a connection using provided svid and renew it
-func x509popRenew(ctx context.Context, x509Svid *types.X509SVID) error {
-	log.Println("Renewing agent...")
-
-	csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{}, key)
-	if err != nil {
-		return fmt.Errorf("failed to create CSR: %w", err)
-	}
-
-	cert, err := x509.ParseCertificate(x509Svid.CertChain[0])
-	if err != nil {
-		return fmt.Errorf("failed to parse cert: %w", err)
-	}
-
-	conn := itclient.NewWithCert(cert, key)
-	defer conn.Release()
-	client := conn.AgentClient()
-
-	resp, err := client.RenewAgent(ctx, &agent.RenewAgentRequest{
-		Params: &agent.AgentX509SVIDParams{
-			Csr: csr,
-		},
-	})
-	if err != nil {
-		return fmt.Errorf("failed to renew agent: %w", err)
-	}
-
-	if !proto.Equal(resp.Svid.Id, x509Svid.Id) {
-		return fmt.Errorf("uxexpected ID: %q, expected: %q", resp.Svid.Id.String(), x509Svid.Id.String())
-	}
-
-	return nil
-}
-
 // deleteAgent delete agent using "admin" connection
 func deleteAgent(ctx context.Context, client agent.AgentClient, id *types.SPIFFEID) error {
 	log.Println("Deleting agent...")