Modify the proto message to work with Google CA.

istio · Jan 27, 2020 · c5f214b · c5f214b
1 parent 005dc98
commit c5f214b
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 62 deletions.
diff --git a/security/pkg/nodeagent/cache/mock/secretcache_mock.go b/security/pkg/nodeagent/cache/mock/secretcache_mock.go
@@ -49,7 +49,7 @@ func NewMockCAClient(mockCertChain1st, mockCertChainRemain []string, failureRate
 	return &cl
 }
 
-func (c *CAClient) CSRSign(ctx context.Context, csrPEM []byte, exchangedToken string,
+func (c *CAClient) CSRSign(ctx context.Context, reqID string, csrPEM []byte, exchangedToken string,
 	certValidTTLInSec int64) ([]string /*PEM-encoded certificate chain*/, error) {
 	// Mock CSRSign failure errors to force Citadel agent to retry.
 	// 50% chance of failure.

diff --git a/security/pkg/nodeagent/cache/secretcache.go b/security/pkg/nodeagent/cache/secretcache.go
@@ -27,6 +27,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/google/uuid"
 	"istio.io/istio/pkg/mcp/status"
 	"istio.io/istio/security/pkg/nodeagent/model"
 	"istio.io/istio/security/pkg/nodeagent/plugin"
@@ -830,13 +831,16 @@ func (sc *SecretCache) sendRetriableRequest(ctx context.Context, csrPEM []byte,
 	var requestErrorString string
 	var err error
 
+	// Assign a unique request ID for all the retries.
+	reqID := uuid.New().String()
+
 	// Keep trying until no error or timeout.
 	for {
 		var httpRespCode int
 		if isCSR {
 			requestErrorString = fmt.Sprintf("%s CSR", conIDresourceNamePrefix)
 			certChainPEM, err = sc.fetcher.CaClient.CSRSign(
-				ctx, csrPEM, exchangedToken, int64(sc.configOptions.SecretTTL.Seconds()))
+				ctx, reqID, csrPEM, exchangedToken, int64(sc.configOptions.SecretTTL.Seconds()))
 		} else {
 			requestErrorString = fmt.Sprintf("%s token exchange", conIDresourceNamePrefix)
 			p := sc.configOptions.Plugins[0]

diff --git a/security/pkg/nodeagent/caclient/interface/iclient.go b/security/pkg/nodeagent/caclient/interface/iclient.go
@@ -20,6 +20,6 @@ import (
 
 // Client interface defines the clients need to implement to talk to CA for CSR.
 type Client interface {
-	CSRSign(ctx context.Context, csrPEM []byte, subjectID string,
+	CSRSign(ctx context.Context, reqID string, csrPEM []byte, subjectID string,
 		certValidTTLInSec int64) ([]string /*PEM-encoded certificate chain*/, error)
 }
diff --git a/security/pkg/nodeagent/caclient/providers/citadel/client.go b/security/pkg/nodeagent/caclient/providers/citadel/client.go
@@ -79,7 +79,7 @@ func NewCitadelClient(endpoint string, tls bool, rootCert []byte) (caClientInter
 }
 
 // CSR Sign calls Citadel to sign a CSR.
-func (c *citadelClient) CSRSign(ctx context.Context, csrPEM []byte, token string,
+func (c *citadelClient) CSRSign(ctx context.Context, reqID string, csrPEM []byte, token string,
 	certValidTTLInSec int64) ([]string /*PEM-encoded certificate chain*/, error) {
 	req := &pb.IstioCertificateRequest{
 		Csr:              string(csrPEM),

diff --git a/security/pkg/nodeagent/caclient/providers/citadel/client_test.go b/security/pkg/nodeagent/caclient/providers/citadel/client_test.go
@@ -93,7 +93,7 @@ func TestCitadelClient(t *testing.T) {
 			t.Errorf("Test case [%s]: failed to create ca client: %v", id, err)
 		}
 
-		resp, err := cli.CSRSign(context.Background(), []byte{01}, fakeToken, 1)
+		resp, err := cli.CSRSign(context.Background(), "12345678-1234-1234-1234-123456789012", []byte{01}, fakeToken, 1)
 		if err != nil {
 			if err.Error() != tc.expectedErr {
 				t.Errorf("Test case [%s]: error (%s) does not match expected error (%s)", id, err.Error(), tc.expectedErr)

diff --git a/security/pkg/nodeagent/caclient/providers/google/client.go b/security/pkg/nodeagent/caclient/providers/google/client.go
@@ -22,6 +22,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/golang/protobuf/ptypes/duration"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/metadata"
@@ -76,11 +77,12 @@ func NewGoogleCAClient(endpoint string, tls bool) (caClientInterface.Client, err
 }
 
 // CSR Sign calls Google CA to sign a CSR.
-func (cl *googleCAClient) CSRSign(ctx context.Context, csrPEM []byte, token string,
+func (cl *googleCAClient) CSRSign(ctx context.Context, reqID string, csrPEM []byte, token string,
 	certValidTTLInSec int64) ([]string /*PEM-encoded certificate chain*/, error) {
 	req := &gcapb.MeshCertificateRequest{
-		Csr:              string(csrPEM),
-		ValidityDuration: certValidTTLInSec,
+		RequestId: reqID,
+		Csr:       string(csrPEM),
+		Validity:  &duration.Duration{Seconds: certValidTTLInSec},
 	}
 
 	// If the token doesn't have "Bearer " prefix, add it.

diff --git a/security/pkg/nodeagent/caclient/providers/google/client_test.go b/security/pkg/nodeagent/caclient/providers/google/client_test.go
@@ -72,7 +72,7 @@ func TestGoogleCAClient(t *testing.T) {
 			t.Errorf("Test case [%s]: failed to create ca client: %v", id, err)
 		}
 
-		resp, err := cli.CSRSign(context.Background(), []byte{01}, fakeToken, 1)
+		resp, err := cli.CSRSign(context.Background(), "12345678-1234-1234-1234-123456789012", []byte{01}, fakeToken, 1)
 		if err != nil {
 			if err.Error() != tc.expectedErr {
 				t.Errorf("Test case [%s]: error (%s) does not match expected error (%s)", id, err.Error(), tc.expectedErr)

diff --git a/security/pkg/nodeagent/caclient/providers/vault/client.go b/security/pkg/nodeagent/caclient/providers/vault/client.go
@@ -73,7 +73,7 @@ func NewVaultClient(tls bool, tlsRootCert []byte,
 }
 
 // CSR Sign calls Vault to sign a CSR.
-func (c *vaultClient) CSRSign(ctx context.Context, csrPEM []byte, saToken string,
+func (c *vaultClient) CSRSign(ctx context.Context, reqID string, csrPEM []byte, saToken string,
 	certValidTTLInSec int64) ([]string /*PEM-encoded certificate chain*/, error) {
 	token, err := loginVaultK8sAuthMethod(c.client, c.vaultLoginPath, c.vaultLoginRole, saToken)
 	if err != nil {

diff --git a/security/pkg/nodeagent/caclient/providers/vault/client_test.go b/security/pkg/nodeagent/caclient/providers/vault/client_test.go
@@ -160,7 +160,8 @@ func TestClientOnMockVaultCA(t *testing.T) {
 			t.Errorf("Test case [%s]: failed to create ca client: %v", id, err)
 		}
 
-		resp, err := cli.CSRSign(context.Background(), tc.cliConfig.csr, tc.cliConfig.clientToken, 1)
+		resp, err := cli.CSRSign(context.Background(), "12345678-1234-1234-1234-123456789012",
+			tc.cliConfig.csr, tc.cliConfig.clientToken, 1)
 		if err != nil {
 			match, _ := regexp.MatchString(tc.expectedErr+".+", err.Error())
 			if !match {

diff --git a/security/proto/providers/google/meshca.pb.go b/security/proto/providers/google/meshca.pb.go
diff --git a/security/proto/providers/google/meshca.proto b/security/proto/providers/google/meshca.proto
@@ -14,16 +14,22 @@
 
 syntax = "proto3";
 
-package google.security.meshca.v1beta1;
+package google.security.meshca.v1;
+
+import "google/protobuf/duration.proto";
 
 // Certificate request message.
 message MeshCertificateRequest {
+  // The request ID must be a valid UUID with the exception that zero UUID is
+  // not supported (00000000-0000-0000-0000-000000000000).
+  string request_id = 1;
   // PEM-encoded certificate request.
-  string csr = 1;
-  // Optional subject ID field.
-  string subject_id = 2;
-  // Optional: requested certificate validity period, in seconds.
-  int64 validity_duration = 3;
+  string csr = 2;
+  // Optional: requested certificate validity period.
+  google.protobuf.Duration validity = 3;
+  // A path to a Private CA CertificateAuthority resource, in the format
+  // `projects/*/locations/*/certificateAuthorities/*`.
+  string certificate_authority = 4;
 }
 
 // Certificate response message.