Fix empty map panic in networkservice authorize.Close (#1386)

* fix empty map panic in networkservice authorize.Close Signed-off-by: Nikita Skrynnik <[email protected]> * fix linter issues Signed-off-by: Nikita Skrynnik <[email protected]> * rerun CI Signed-off-by: Nikita Skrynnik <[email protected]> * apply review comments Signed-off-by: Nikita Skrynnik <[email protected]> Signed-off-by: Nikita Skrynnik <[email protected]>
networkservicemesh · Nov 24, 2022 · d83bdbd · d83bdbd
1 parent 14f2490
commit d83bdbd
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 6 deletions.
diff --git a/pkg/networkservice/common/authorize/server.go b/pkg/networkservice/common/authorize/server.go
@@ -94,16 +94,18 @@ func (a *authorizeServer) Close(ctx context.Context, conn *networkservice.Connec
 	if spiffeID, err := spire.SpiffeIDFromContext(ctx); err == nil {
 		connID := conn.GetPath().GetPathSegments()[index-1].GetId()
 		ids, ok := a.spiffeIDConnectionMap.Load(spiffeID)
+		idsEmpty := true
 		if ok {
-			if _, ok := ids.Load(connID); ok {
+			if _, loaded := ids.Load(connID); loaded {
 				ids.Delete(connID)
 			}
+
+			ids.Range(func(_ string, _ struct{}) bool {
+				idsEmpty = false
+				return false
+			})
 		}
-		idsEmpty := true
-		ids.Range(func(_ string, _ struct{}) bool {
-			idsEmpty = false
-			return true
-		})
+
 		if idsEmpty {
 			a.spiffeIDConnectionMap.Delete(spiffeID)
 		} else {

diff --git a/pkg/networkservice/common/authorize/server_test.go b/pkg/networkservice/common/authorize/server_test.go
@@ -1,5 +1,7 @@
 // Copyright (c) 2020-2021 Doc.ai and/or its affiliates.
 //
+// Copyright (c) 2022 Cisco and/or its affiliates.
+//
 // SPDX-License-Identifier: Apache-2.0
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -18,20 +20,58 @@ package authorize_test
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"math/big"
+	"net/url"
 	"testing"
+	"time"
 
 	"github.com/networkservicemesh/sdk/pkg/tools/opa"
 
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/peer"
 	"google.golang.org/grpc/status"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/authorize"
 )
 
+func generateCert(u *url.URL) []byte {
+	ca := &x509.Certificate{
+		SerialNumber: big.NewInt(1653),
+		NotBefore:    time.Now(),
+		NotAfter:     time.Now().AddDate(10, 0, 0),
+		URIs:         []*url.URL{u},
+	}
+
+	priv, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	pub := &priv.PublicKey
+
+	certBytes, _ := x509.CreateCertificate(rand.Reader, ca, ca, pub, priv)
+	return certBytes
+}
+
+func withPeer(ctx context.Context, certBytes []byte) (context.Context, error) {
+	x509cert, err := x509.ParseCertificate(certBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	authInfo := &credentials.TLSInfo{
+		State: tls.ConnectionState{
+			PeerCertificates: []*x509.Certificate{x509cert},
+		},
+	}
+	return peer.NewContext(ctx, &peer.Peer{AuthInfo: authInfo}), nil
+}
+
 func testPolicy() authorize.Policy {
 	return opa.WithPolicyFromSource(`
 		package test
@@ -129,3 +169,27 @@ func TestAuthzEndpoint(t *testing.T) {
 		})
 	}
 }
+
+func TestAuthorize_EmptySpiffeIDConnectionMapOnClose(t *testing.T) {
+	t.Cleanup(func() { goleak.VerifyNone(t) })
+
+	conn := &networkservice.Connection{
+		Id: "conn",
+		Path: &networkservice.Path{
+			Index: 1,
+			PathSegments: []*networkservice.PathSegment{
+				{Id: "id-1"},
+				{Id: "id-2"},
+			},
+		},
+	}
+
+	server := authorize.NewServer(authorize.Any())
+	certBytes := generateCert(&url.URL{Scheme: "spiffe", Host: "test.com", Path: "test"})
+
+	ctx, err := withPeer(context.Background(), certBytes)
+	require.NoError(t, err)
+
+	_, err = server.Close(ctx, conn)
+	require.NoError(t, err)
+}