fix: manager generates mTLS certificates for arbitrary IP addresses

Signed-off-by: Gaius <[email protected]>
dragonflyoss · Aug 9, 2023 · 750b57f · 750b57f
1 parent 77c91cb
commit 750b57f
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 41 deletions.
diff --git a/manager/rpcserver/security_server_v1.go b/manager/rpcserver/security_server_v1.go
@@ -22,11 +22,9 @@ import (
 	"crypto/x509"
 	"fmt"
 	"math/big"
-	"net"
 	"time"
 
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/peer"
 	"google.golang.org/grpc/status"
 
 	securityv1 "d7y.io/api/v2/pkg/apis/security/v1"
@@ -53,57 +51,36 @@ func (s *securityServerV1) IssueCertificate(ctx context.Context, req *securityv1
 		return nil, status.Errorf(codes.Unavailable, "ca is missing for this manager instance")
 	}
 
-	var (
-		ip  string
-		err error
-	)
-	p, ok := peer.FromContext(ctx)
-	if !ok {
-		return nil, status.Errorf(codes.InvalidArgument, "invalid grpc peer info")
-	}
-
-	if addr, ok := p.Addr.(*net.TCPAddr); ok {
-		ip = addr.IP.String()
-	} else {
-		ip, _, err = net.SplitHostPort(p.Addr.String())
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	// Parse csr.
 	csr, err := x509.ParseCertificateRequest(req.Csr)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "invalid csr format: %s", err.Error())
 	}
 
 	// Check csr signature.
-	// TODO check csr common name and so on.
 	if err = csr.CheckSignature(); err != nil {
 		return nil, err
 	}
 	logger.Infof("valid csr: %#v", csr.Subject)
 
+	// Check csr ip address.
+	if len(csr.IPAddresses) == 0 {
+		return nil, status.Errorf(codes.InvalidArgument, "invalid csr ip address")
+	}
+
+	// Generate serial number.
 	serial, err := rand.Int(rand.Reader, (&big.Int{}).Exp(big.NewInt(2), big.NewInt(159), nil))
 	if err != nil {
 		return nil, err
 	}
 
-	// TODO only valid for peer ip
-	// BTW we need support both of ipv4 and ipv6.
-	ips := csr.IPAddresses
-	if len(ips) == 0 {
-		// Add default connected ip.
-		ips = []net.IP{net.ParseIP(ip)}
-	}
-
 	now := time.Now()
 	template := x509.Certificate{
 		SerialNumber:          serial,
 		Subject:               csr.Subject,
 		DNSNames:              csr.DNSNames,
 		EmailAddresses:        csr.EmailAddresses,
-		IPAddresses:           ips,
+		IPAddresses:           csr.IPAddresses,
 		URIs:                  csr.URIs,
 		NotBefore:             now.Add(-10 * time.Minute).UTC(),
 		NotAfter:              now.Add(req.ValidityPeriod.AsDuration()).UTC(),

diff --git a/manager/rpcserver/security_server_v1_test.go b/manager/rpcserver/security_server_v1_test.go
@@ -32,7 +32,6 @@ import (
 
 	testifyassert "github.com/stretchr/testify/assert"
 	testifyrequire "github.com/stretchr/testify/require"
-	"google.golang.org/grpc/peer"
 	"google.golang.org/protobuf/types/known/durationpb"
 
 	securityv1 "d7y.io/api/v2/pkg/apis/security/v1"
@@ -60,6 +59,7 @@ func TestIssueCertificate(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			template := &x509.CertificateRequest{
+				IPAddresses: []net.IP{net.ParseIP(tc.peerIP)},
 				Subject: pkix.Name{
 					Country:            []string{"China"},
 					Organization:       []string{"Dragonfly"},
@@ -88,17 +88,8 @@ func TestIssueCertificate(t *testing.T) {
 			})
 			require.Nilf(err, "newServer should be ok")
 
-			ctx := peer.NewContext(
-				context.Background(),
-				&peer.Peer{
-					Addr: &net.TCPAddr{
-						IP:   net.ParseIP(tc.peerIP),
-						Port: 65008,
-					},
-				})
-
 			resp, err := securityServerV1.IssueCertificate(
-				ctx,
+				context.Background(),
 				&securityv1.CertificateRequest{
 					Csr:            csr,
 					ValidityPeriod: durationpb.New(time.Hour),