From 08571e33fb897d047040d74a9df8545f3c928a7b Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Tue, 28 May 2024 07:06:46 +0000 Subject: [PATCH 01/16] Add cert authorization with common name support --- gnmi_server/clientCertAuth.go | 44 ++++++++++++++++- gnmi_server/server_test.go | 93 +++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 1c44d9c5..278b2764 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -2,12 +2,15 @@ package gnmi import ( "github.com/sonic-net/sonic-gnmi/common_utils" + "github.com/sonic-net/sonic-gnmi/sonic_data_client" + "github.com/sonic-net/sonic-gnmi/swsscommon" "github.com/golang/glog" "golang.org/x/net/context" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/peer" "google.golang.org/grpc/status" + "strings" ) func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { @@ -32,10 +35,49 @@ func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { return ctx, status.Error(codes.Unauthenticated, "invalid username in certificate common name.") } - if err := PopulateAuthStruct(username, &rc.Auth, nil); err != nil { + err := CommonNameMatch(username) + if err != nil { + return ctx, err + } + + if err = PopulateAuthStruct(username, &rc.Auth, nil); err != nil { glog.Infof("[%s] Failed to retrieve authentication information; %v", rc.ID, err) return ctx, status.Errorf(codes.Unauthenticated, "") } return ctx, nil } + +func CommonNameMatch(certCommonName string) error { + var trustedCertCommonNames, err = getTrustedCertCommonNames() + if err != nil { + return err + } + + if len(trustedCertCommonNames) == 0 { + // ignore further check because not config trusted cert common names + return nil + } + + for _, trustedCertCommonName := range trustedCertCommonNames { + if certCommonName == trustedCertCommonName { + return nil; + } + } + + return status.Errorf(codes.Unauthenticated, "Invalid cert cname:'%s', not a trusted cert common name.", certCommonName) +} + +func getTrustedCertCommonNames() ([]string, error) { + var configDbConnector = swsscommon.NewConfigDBConnector() + defer swsscommon.DeleteConfigDBConnector_Native(configDbConnector.ConfigDBConnector_Native) + configDbConnector.Connect(false) + + clientCrtCommonNames, err := client.Hget(configDbConnector, "GNMI", "certs", "client_crt_cname"); + if err != nil { + return nil, err + } + + var clientCrtCommonNameArray = strings.Split(clientCrtCommonNames, ",") + return clientCrtCommonNameArray, nil +} diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 27e75741..c681913f 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -55,6 +55,7 @@ import ( gnmipb "github.com/openconfig/gnmi/proto/gnmi" gnoi_system_pb "github.com/openconfig/gnoi/system" "github.com/sonic-net/sonic-gnmi/common_utils" + "github.com/sonic-net/sonic-gnmi/swsscommon" ) var clientTypes = []string{gclient.Type} @@ -4187,6 +4188,98 @@ func TestSaveOnSet(t *testing.T) { } } +func TestGetTrustedCertCommonNames(t *testing.T) { + if !swsscommon.SonicDBConfigIsInit() { + swsscommon.SonicDBConfigInitialize() + } + + var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) + configDb.Flushdb() + + // check get nil cert name + trustedCertCommonNames, err := getTrustedCertCommonNames() + if err == nil { + t.Errorf("get trusted cert common names should failed: %v, but get %s", err, trustedCertCommonNames) + } + + // check get 1 cert name + var gnmiTable = swsscommon.NewTable(configDb, "GNMI") + gnmiTable.Hset("certs", "client_crt_cname", "certname1") + trustedCertCommonNames, err = getTrustedCertCommonNames() + if err != nil { + t.Errorf("get DPU address should success: %v", err) + } + + if len(trustedCertCommonNames) != 1 { + t.Errorf("get DPU address should get 1 result, but get %s", trustedCertCommonNames) + } + + if trustedCertCommonNames[0] != "certname1" { + t.Errorf("get DPU address should get 'certname1', but get %s", trustedCertCommonNames[0]) + } + + // check get multiple cert names + gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") + trustedCertCommonNames, err = getTrustedCertCommonNames() + if err != nil { + t.Errorf("get DPU address should success: %v", err) + } + + if len(trustedCertCommonNames) != 2 { + t.Errorf("get DPU address should get 2 result, but get %s", trustedCertCommonNames) + } + + if trustedCertCommonNames[0] != "certname1" { + t.Errorf("get DPU address should get 'certname1', but get %s", trustedCertCommonNames[0]) + } + + if trustedCertCommonNames[1] != "certname2" { + t.Errorf("get DPU address should get 'certname2', but get %s", trustedCertCommonNames[1]) + } + + swsscommon.DeleteTable(gnmiTable) + swsscommon.DeleteDBConnector(configDb) +} + +func TestCommonNameMatch(t *testing.T) { + if !swsscommon.SonicDBConfigIsInit() { + swsscommon.SonicDBConfigInitialize() + } + + var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) + configDb.Flushdb() + + // check auth with nil cert name + err := CommonNameMatch("certname1") + if err != nil { + t.Errorf("CommonNameMatch with empty config should success: %v", err) + } + + // check get 1 cert name + var gnmiTable = swsscommon.NewTable(configDb, "GNMI") + gnmiTable.Hset("certs", "client_crt_cname", "certname1") + err = CommonNameMatch("certname1") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + + // check get multiple cert names + gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") + err = CommonNameMatch("certname1") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + + gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") + err = CommonNameMatch("certname2") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + + swsscommon.DeleteTable(gnmiTable) + swsscommon.DeleteDBConnector(configDb) +} + func init() { // Enable logs at UT setup flag.Lookup("v").Value.Set("10") From 795a52bb5f16c780c58a25f73db542c6ad66541d Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Tue, 28 May 2024 07:21:14 +0000 Subject: [PATCH 02/16] Improve code --- gnmi_server/clientCertAuth.go | 3 ++- gnmi_server/server_test.go | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 278b2764..d3f564e5 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -75,7 +75,8 @@ func getTrustedCertCommonNames() ([]string, error) { clientCrtCommonNames, err := client.Hget(configDbConnector, "GNMI", "certs", "client_crt_cname"); if err != nil { - return nil, err + // config item does not exist, return empty array + return []string{}, nil } var clientCrtCommonNameArray = strings.Split(clientCrtCommonNames, ",") diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index c681913f..2f2754cc 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4198,8 +4198,12 @@ func TestGetTrustedCertCommonNames(t *testing.T) { // check get nil cert name trustedCertCommonNames, err := getTrustedCertCommonNames() - if err == nil { - t.Errorf("get trusted cert common names should failed: %v, but get %s", err, trustedCertCommonNames) + if err != nil { + t.Errorf("get DPU address should success: %v", err) + } + + if len(trustedCertCommonNames) != 0 { + t.Errorf("get DPU address should get 0 result, but get %s", trustedCertCommonNames) } // check get 1 cert name From b998f4ec02b6304b771ee97eeef459af871b26b8 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Tue, 28 May 2024 09:16:26 +0000 Subject: [PATCH 03/16] Improve code --- gnmi_server/clientCertAuth.go | 12 ++++-------- gnmi_server/server_test.go | 6 ++++++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index d3f564e5..911c9dd3 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -49,11 +49,7 @@ func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { } func CommonNameMatch(certCommonName string) error { - var trustedCertCommonNames, err = getTrustedCertCommonNames() - if err != nil { - return err - } - + var trustedCertCommonNames = getTrustedCertCommonNames() if len(trustedCertCommonNames) == 0 { // ignore further check because not config trusted cert common names return nil @@ -68,7 +64,7 @@ func CommonNameMatch(certCommonName string) error { return status.Errorf(codes.Unauthenticated, "Invalid cert cname:'%s', not a trusted cert common name.", certCommonName) } -func getTrustedCertCommonNames() ([]string, error) { +func getTrustedCertCommonNames() []string { var configDbConnector = swsscommon.NewConfigDBConnector() defer swsscommon.DeleteConfigDBConnector_Native(configDbConnector.ConfigDBConnector_Native) configDbConnector.Connect(false) @@ -76,9 +72,9 @@ func getTrustedCertCommonNames() ([]string, error) { clientCrtCommonNames, err := client.Hget(configDbConnector, "GNMI", "certs", "client_crt_cname"); if err != nil { // config item does not exist, return empty array - return []string{}, nil + return []string{} } var clientCrtCommonNameArray = strings.Split(clientCrtCommonNames, ",") - return clientCrtCommonNameArray, nil + return clientCrtCommonNameArray } diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 2f2754cc..60c940e3 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4280,6 +4280,12 @@ func TestCommonNameMatch(t *testing.T) { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } + // check a invalid cert cname + err = CommonNameMatch("certname3") + if err == nil { + t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) + } + swsscommon.DeleteTable(gnmiTable) swsscommon.DeleteDBConnector(configDb) } From 7c4b830bea63768f4be164dca8a4481ced715c63 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Tue, 28 May 2024 09:52:17 +0000 Subject: [PATCH 04/16] Improve code --- gnmi_server/server_test.go | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 60c940e3..edc3db83 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4197,48 +4197,36 @@ func TestGetTrustedCertCommonNames(t *testing.T) { configDb.Flushdb() // check get nil cert name - trustedCertCommonNames, err := getTrustedCertCommonNames() - if err != nil { - t.Errorf("get DPU address should success: %v", err) - } - + trustedCertCommonNames := getTrustedCertCommonNames() if len(trustedCertCommonNames) != 0 { - t.Errorf("get DPU address should get 0 result, but get %s", trustedCertCommonNames) + t.Errorf("GetTrustedCertCommonNames should get 0 result, but get %s", trustedCertCommonNames) } // check get 1 cert name var gnmiTable = swsscommon.NewTable(configDb, "GNMI") gnmiTable.Hset("certs", "client_crt_cname", "certname1") - trustedCertCommonNames, err = getTrustedCertCommonNames() - if err != nil { - t.Errorf("get DPU address should success: %v", err) - } - + trustedCertCommonNames = getTrustedCertCommonNames() if len(trustedCertCommonNames) != 1 { - t.Errorf("get DPU address should get 1 result, but get %s", trustedCertCommonNames) + t.Errorf("GetTrustedCertCommonNames should get 1 result, but get %s", trustedCertCommonNames) } if trustedCertCommonNames[0] != "certname1" { - t.Errorf("get DPU address should get 'certname1', but get %s", trustedCertCommonNames[0]) + t.Errorf("GetTrustedCertCommonNames should get 'certname1', but get %s", trustedCertCommonNames[0]) } // check get multiple cert names gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") - trustedCertCommonNames, err = getTrustedCertCommonNames() - if err != nil { - t.Errorf("get DPU address should success: %v", err) - } - + trustedCertCommonNames = getTrustedCertCommonNames() if len(trustedCertCommonNames) != 2 { - t.Errorf("get DPU address should get 2 result, but get %s", trustedCertCommonNames) + t.Errorf("GetTrustedCertCommonNames should get 2 result, but get %s", trustedCertCommonNames) } if trustedCertCommonNames[0] != "certname1" { - t.Errorf("get DPU address should get 'certname1', but get %s", trustedCertCommonNames[0]) + t.Errorf("GetTrustedCertCommonNames should get 'certname1', but get %s", trustedCertCommonNames[0]) } if trustedCertCommonNames[1] != "certname2" { - t.Errorf("get DPU address should get 'certname2', but get %s", trustedCertCommonNames[1]) + t.Errorf("GetTrustedCertCommonNames should get 'certname2', but get %s", trustedCertCommonNames[1]) } swsscommon.DeleteTable(gnmiTable) From 43e7a43a1737aecc394b3de286f770d128245324 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 29 May 2024 02:39:01 +0000 Subject: [PATCH 05/16] Improve code --- .../dialout_server_cli/dialout_server_cli.go | 1 + gnmi_server/clientCertAuth.go | 27 ++----- gnmi_server/debug.go | 2 +- gnmi_server/gnoi.go | 32 ++++----- gnmi_server/server.go | 21 +++--- gnmi_server/server_test.go | 70 ++----------------- telemetry/telemetry.go | 3 + telemetry/telemetry_test.go | 6 +- tools/test/telemetry.sh | 1 + 9 files changed, 48 insertions(+), 115 deletions(-) diff --git a/dialout/dialout_server_cli/dialout_server_cli.go b/dialout/dialout_server_cli/dialout_server_cli.go index f3ad5ba6..56d80ae9 100644 --- a/dialout/dialout_server_cli/dialout_server_cli.go +++ b/dialout/dialout_server_cli/dialout_server_cli.go @@ -22,6 +22,7 @@ var ( serverKey = flag.String("server_key", "", "TLS server private key") insecure = flag.Bool("insecure", false, "Skip providing TLS cert and key, for testing only!") allowNoClientCert = flag.Bool("allow_no_client_auth", false, "When set, telemetry server will request but not require a client certificate.") + clientCrtCname = flag.String("client_crt_cname", "", "Client cert common name") ) func main() { diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 911c9dd3..790cafb1 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -2,8 +2,6 @@ package gnmi import ( "github.com/sonic-net/sonic-gnmi/common_utils" - "github.com/sonic-net/sonic-gnmi/sonic_data_client" - "github.com/sonic-net/sonic-gnmi/swsscommon" "github.com/golang/glog" "golang.org/x/net/context" "google.golang.org/grpc/codes" @@ -13,7 +11,7 @@ import ( "strings" ) -func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { +func ClientCertAuthenAndAuthor(ctx context.Context, clientCrtCname string) (context.Context, error) { rc, ctx := common_utils.GetContext(ctx) p, ok := peer.FromContext(ctx) if !ok { @@ -35,7 +33,7 @@ func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { return ctx, status.Error(codes.Unauthenticated, "invalid username in certificate common name.") } - err := CommonNameMatch(username) + err := CommonNameMatch(username, clientCrtCname) if err != nil { return ctx, err } @@ -48,8 +46,8 @@ func ClientCertAuthenAndAuthor(ctx context.Context) (context.Context, error) { return ctx, nil } -func CommonNameMatch(certCommonName string) error { - var trustedCertCommonNames = getTrustedCertCommonNames() +func CommonNameMatch(certCommonName string, clientCrtCname string) error { + var trustedCertCommonNames = strings.Split(clientCrtCname, ",") if len(trustedCertCommonNames) == 0 { // ignore further check because not config trusted cert common names return nil @@ -62,19 +60,4 @@ func CommonNameMatch(certCommonName string) error { } return status.Errorf(codes.Unauthenticated, "Invalid cert cname:'%s', not a trusted cert common name.", certCommonName) -} - -func getTrustedCertCommonNames() []string { - var configDbConnector = swsscommon.NewConfigDBConnector() - defer swsscommon.DeleteConfigDBConnector_Native(configDbConnector.ConfigDBConnector_Native) - configDbConnector.Connect(false) - - clientCrtCommonNames, err := client.Hget(configDbConnector, "GNMI", "certs", "client_crt_cname"); - if err != nil { - // config item does not exist, return empty array - return []string{} - } - - var clientCrtCommonNameArray = strings.Split(clientCrtCommonNames, ",") - return clientCrtCommonNameArray -} +} \ No newline at end of file diff --git a/gnmi_server/debug.go b/gnmi_server/debug.go index 5239b72e..6099630e 100644 --- a/gnmi_server/debug.go +++ b/gnmi_server/debug.go @@ -35,7 +35,7 @@ import ( func (srv *Server) GetSubscribePreferences(req *spb_gnoi.SubscribePreferencesReq, stream spb_gnoi.Debug_GetSubscribePreferencesServer) error { ctx := stream.Context() - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return err } diff --git a/gnmi_server/gnoi.go b/gnmi_server/gnoi.go index 8bd96536..f29403a4 100644 --- a/gnmi_server/gnoi.go +++ b/gnmi_server/gnoi.go @@ -33,7 +33,7 @@ func RebootSystem(fileName string) error { func (srv *Server) Reboot(ctx context.Context, req *gnoi_system_pb.RebootRequest) (*gnoi_system_pb.RebootResponse, error) { fileName := common_utils.GNMI_WORK_PATH + "/config_db.json.tmp" - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -57,7 +57,7 @@ func (srv *Server) Reboot(ctx context.Context, req *gnoi_system_pb.RebootRequest // TODO: Support GNOI RebootStatus func (srv *Server) RebootStatus(ctx context.Context, req *gnoi_system_pb.RebootStatusRequest) (*gnoi_system_pb.RebootStatusResponse, error) { - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -67,7 +67,7 @@ func (srv *Server) RebootStatus(ctx context.Context, req *gnoi_system_pb.RebootS // TODO: Support GNOI CancelReboot func (srv *Server) CancelReboot(ctx context.Context, req *gnoi_system_pb.CancelRebootRequest) (*gnoi_system_pb.CancelRebootResponse, error) { - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -76,7 +76,7 @@ func (srv *Server) CancelReboot(ctx context.Context, req *gnoi_system_pb.CancelR } func (srv *Server) Ping(req *gnoi_system_pb.PingRequest, rs gnoi_system_pb.System_PingServer) error { ctx := rs.Context() - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return err } @@ -85,7 +85,7 @@ func (srv *Server) Ping(req *gnoi_system_pb.PingRequest, rs gnoi_system_pb.Syste } func (srv *Server) Traceroute(req *gnoi_system_pb.TracerouteRequest, rs gnoi_system_pb.System_TracerouteServer) error { ctx := rs.Context() - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return err } @@ -94,7 +94,7 @@ func (srv *Server) Traceroute(req *gnoi_system_pb.TracerouteRequest, rs gnoi_sys } func (srv *Server) SetPackage(rs gnoi_system_pb.System_SetPackageServer) error { ctx := rs.Context() - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return err } @@ -102,7 +102,7 @@ func (srv *Server) SetPackage(rs gnoi_system_pb.System_SetPackageServer) error { return status.Errorf(codes.Unimplemented, "") } func (srv *Server) SwitchControlProcessor(ctx context.Context, req *gnoi_system_pb.SwitchControlProcessorRequest) (*gnoi_system_pb.SwitchControlProcessorResponse, error) { - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -110,7 +110,7 @@ func (srv *Server) SwitchControlProcessor(ctx context.Context, req *gnoi_system_ return nil, status.Errorf(codes.Unimplemented, "") } func (srv *Server) Time(ctx context.Context, req *gnoi_system_pb.TimeRequest) (*gnoi_system_pb.TimeResponse, error) { - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func (srv *Server) Time(ctx context.Context, req *gnoi_system_pb.TimeRequest) (* func (srv *Server) Authenticate(ctx context.Context, req *spb_jwt.AuthenticateRequest) (*spb_jwt.AuthenticateResponse, error) { // Can't enforce normal authentication here.. maybe only enforce client cert auth if enabled? - // ctx,err := authenticate(srv.config.UserAuth, ctx) + // ctx,err := authenticate(srv.config, ctx) // if err != nil { // return nil, err // } @@ -147,7 +147,7 @@ func (srv *Server) Authenticate(ctx context.Context, req *spb_jwt.AuthenticateRe } func (srv *Server) Refresh(ctx context.Context, req *spb_jwt.RefreshRequest) (*spb_jwt.RefreshResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -175,7 +175,7 @@ func (srv *Server) Refresh(ctx context.Context, req *spb_jwt.RefreshRequest) (*s } func (srv *Server) ClearNeighbors(ctx context.Context, req *spb.ClearNeighborsRequest) (*spb.ClearNeighborsResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -207,7 +207,7 @@ func (srv *Server) ClearNeighbors(ctx context.Context, req *spb.ClearNeighborsRe } func (srv *Server) CopyConfig(ctx context.Context, req *spb.CopyConfigRequest) (*spb.CopyConfigResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -238,7 +238,7 @@ func (srv *Server) CopyConfig(ctx context.Context, req *spb.CopyConfigRequest) ( } func (srv *Server) ShowTechsupport(ctx context.Context, req *spb.TechsupportRequest) (*spb.TechsupportResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -270,7 +270,7 @@ func (srv *Server) ShowTechsupport(ctx context.Context, req *spb.TechsupportRequ } func (srv *Server) ImageInstall(ctx context.Context, req *spb.ImageInstallRequest) (*spb.ImageInstallResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -302,7 +302,7 @@ func (srv *Server) ImageInstall(ctx context.Context, req *spb.ImageInstallReques } func (srv *Server) ImageRemove(ctx context.Context, req *spb.ImageRemoveRequest) (*spb.ImageRemoveResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } @@ -334,7 +334,7 @@ func (srv *Server) ImageRemove(ctx context.Context, req *spb.ImageRemoveRequest) } func (srv *Server) ImageDefault(ctx context.Context, req *spb.ImageDefaultRequest) (*spb.ImageDefaultResponse, error) { - ctx, err := authenticate(srv.config.UserAuth, ctx) + ctx, err := authenticate(srv.config, ctx) if err != nil { return nil, err } diff --git a/gnmi_server/server.go b/gnmi_server/server.go index b080fc11..00aaa3c8 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -65,6 +65,7 @@ type Config struct { EnableNativeWrite bool ZmqPort string IdleConnDuration int + ClientCrtCname string } var AuthLock sync.Mutex @@ -205,30 +206,30 @@ func (srv *Server) Port() int64 { return srv.config.Port } -func authenticate(UserAuth AuthTypes, ctx context.Context) (context.Context, error) { +func authenticate(config *Config, ctx context.Context) (context.Context, error) { var err error success := false rc, ctx := common_utils.GetContext(ctx) - if !UserAuth.Any() { + if !config.UserAuth.Any() { //No Auth enabled rc.Auth.AuthEnabled = false return ctx, nil } rc.Auth.AuthEnabled = true - if UserAuth.Enabled("password") { + if config.UserAuth.Enabled("password") { ctx, err = BasicAuthenAndAuthor(ctx) if err == nil { success = true } } - if !success && UserAuth.Enabled("jwt") { + if !success && config.UserAuth.Enabled("jwt") { _, ctx, err = JwtAuthenAndAuthor(ctx) if err == nil { success = true } } - if !success && UserAuth.Enabled("cert") { - ctx, err = ClientCertAuthenAndAuthor(ctx) + if !success && config.UserAuth.Enabled("cert") { + ctx, err = ClientCertAuthenAndAuthor(ctx, config.ClientCrtCname) if err == nil { success = true } @@ -247,7 +248,7 @@ func authenticate(UserAuth AuthTypes, ctx context.Context) (context.Context, err // Subscribe implements the gNMI Subscribe RPC. func (s *Server) Subscribe(stream gnmipb.GNMI_SubscribeServer) error { ctx := stream.Context() - ctx, err := authenticate(s.config.UserAuth, ctx) + ctx, err := authenticate(s.config, ctx) if err != nil { return err } @@ -332,7 +333,7 @@ func IsNativeOrigin(origin string) bool { // Get implements the Get RPC in gNMI spec. func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetResponse, error) { common_utils.IncCounter(common_utils.GNMI_GET) - ctx, err := authenticate(s.config.UserAuth, ctx) + ctx, err := authenticate(s.config, ctx) if err != nil { common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, err @@ -438,7 +439,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") } - ctx, err := authenticate(s.config.UserAuth, ctx) + ctx, err := authenticate(s.config, ctx) if err != nil { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, err @@ -539,7 +540,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe } func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest) (*gnmipb.CapabilityResponse, error) { - ctx, err := authenticate(s.config.UserAuth, ctx) + ctx, err := authenticate(s.config, ctx) if err != nil { return nil, err } diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index edc3db83..63b78118 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -55,7 +55,6 @@ import ( gnmipb "github.com/openconfig/gnmi/proto/gnmi" gnoi_system_pb "github.com/openconfig/gnoi/system" "github.com/sonic-net/sonic-gnmi/common_utils" - "github.com/sonic-net/sonic-gnmi/swsscommon" ) var clientTypes = []string{gclient.Type} @@ -4188,94 +4187,35 @@ func TestSaveOnSet(t *testing.T) { } } -func TestGetTrustedCertCommonNames(t *testing.T) { - if !swsscommon.SonicDBConfigIsInit() { - swsscommon.SonicDBConfigInitialize() - } - - var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) - configDb.Flushdb() - - // check get nil cert name - trustedCertCommonNames := getTrustedCertCommonNames() - if len(trustedCertCommonNames) != 0 { - t.Errorf("GetTrustedCertCommonNames should get 0 result, but get %s", trustedCertCommonNames) - } - - // check get 1 cert name - var gnmiTable = swsscommon.NewTable(configDb, "GNMI") - gnmiTable.Hset("certs", "client_crt_cname", "certname1") - trustedCertCommonNames = getTrustedCertCommonNames() - if len(trustedCertCommonNames) != 1 { - t.Errorf("GetTrustedCertCommonNames should get 1 result, but get %s", trustedCertCommonNames) - } - - if trustedCertCommonNames[0] != "certname1" { - t.Errorf("GetTrustedCertCommonNames should get 'certname1', but get %s", trustedCertCommonNames[0]) - } - - // check get multiple cert names - gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") - trustedCertCommonNames = getTrustedCertCommonNames() - if len(trustedCertCommonNames) != 2 { - t.Errorf("GetTrustedCertCommonNames should get 2 result, but get %s", trustedCertCommonNames) - } - - if trustedCertCommonNames[0] != "certname1" { - t.Errorf("GetTrustedCertCommonNames should get 'certname1', but get %s", trustedCertCommonNames[0]) - } - - if trustedCertCommonNames[1] != "certname2" { - t.Errorf("GetTrustedCertCommonNames should get 'certname2', but get %s", trustedCertCommonNames[1]) - } - - swsscommon.DeleteTable(gnmiTable) - swsscommon.DeleteDBConnector(configDb) -} - func TestCommonNameMatch(t *testing.T) { - if !swsscommon.SonicDBConfigIsInit() { - swsscommon.SonicDBConfigInitialize() - } - - var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) - configDb.Flushdb() - // check auth with nil cert name - err := CommonNameMatch("certname1") + err := CommonNameMatch("certname1", "") if err != nil { t.Errorf("CommonNameMatch with empty config should success: %v", err) } // check get 1 cert name - var gnmiTable = swsscommon.NewTable(configDb, "GNMI") - gnmiTable.Hset("certs", "client_crt_cname", "certname1") - err = CommonNameMatch("certname1") + err = CommonNameMatch("certname1", "certname1") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } // check get multiple cert names - gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") - err = CommonNameMatch("certname1") + err = CommonNameMatch("certname1", "certname1,certname2") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } - gnmiTable.Hset("certs", "client_crt_cname", "certname1,certname2") - err = CommonNameMatch("certname2") + err = CommonNameMatch("certname2", "certname1,certname2") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } // check a invalid cert cname - err = CommonNameMatch("certname3") + err = CommonNameMatch("certname3", "certname1,certname2") if err == nil { t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) } - - swsscommon.DeleteTable(gnmiTable) - swsscommon.DeleteDBConnector(configDb) } func init() { diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go index febee1d3..d4d74eb4 100644 --- a/telemetry/telemetry.go +++ b/telemetry/telemetry.go @@ -43,6 +43,7 @@ type TelemetryConfig struct { CaCert *string ServerCert *string ServerKey *string + ClientCrtCname *string ZmqAddress *string ZmqPort *string Insecure *bool @@ -150,6 +151,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { CaCert: fs.String("ca_crt", "", "CA certificate for client certificate validation. Optional."), ServerCert: fs.String("server_crt", "", "TLS server certificate"), ServerKey: fs.String("server_key", "", "TLS server private key"), + ClientCrtCname: fs.String("client_crt_cname", "", "Client cert common name"), ZmqAddress: fs.String("zmq_address", "", "Orchagent ZMQ address, deprecated, please use zmq_port."), ZmqPort: fs.String("zmq_port", "", "Orchagent ZMQ port, when not set or empty string telemetry server will switch to Redis based communication channel."), Insecure: fs.Bool("insecure", false, "Skip providing TLS cert and key, for testing only!"), @@ -224,6 +226,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { cfg.LogLevel = int(*telemetryCfg.LogLevel) cfg.Threshold = int(*telemetryCfg.Threshold) cfg.IdleConnDuration = int(*telemetryCfg.IdleConnDuration) + cfg.ClientCrtCname = *telemetryCfg.ClientCrtCname // TODO: After other dependent projects are migrated to ZmqPort, remove ZmqAddress zmqAddress := *telemetryCfg.ZmqAddress diff --git a/telemetry/telemetry_test.go b/telemetry/telemetry_test.go index cab194cc..bd69c726 100644 --- a/telemetry/telemetry_test.go +++ b/telemetry/telemetry_test.go @@ -358,13 +358,17 @@ func TestStartGNMIServerCACert(t *testing.T) { }() fs := flag.NewFlagSet("testStartGNMIServerCACert", flag.ContinueOnError) - os.Args = []string{"cmd", "-port", "8080", "-server_crt", testServerCert, "-server_key", testServerKey, "-ca_crt", testServerCACert} + os.Args = []string{"cmd", "-port", "8080", "-server_crt", testServerCert, "-server_key", testServerKey, "-ca_crt", testServerCACert, "-client_crt_cname", "testcname1"} telemetryCfg, cfg, err := setupFlags(fs) if err != nil { t.Errorf("Expected err to be nil, got err %v", err) } + if cfg.ClientCrtCname != "testcname1" { + t.Errorf("Expected err to be testcname1, got %s", cfg.ClientCrtCname) + } + err = createCACert(testServerCACert) if err != nil { diff --git a/tools/test/telemetry.sh b/tools/test/telemetry.sh index c23a1d08..10000341 100755 --- a/tools/test/telemetry.sh +++ b/tools/test/telemetry.sh @@ -12,6 +12,7 @@ for V in "$@"; do -server_crt|--server_crt|-server_crt=*|--server_crt=*) HAS_CERT=1 ;; -server_key|--server_key|-server_key=*|--server_key=*) HAS_CERT=1 ;; -client_auth|--client_auth|-client_auth=*|--client_auth=*) HAS_AUTH=1 ;; + -client_crt_cname|--client_crt_cname|-client_crt_cname=*|--client_crt_cname=*) HAS_CNAME=1 ;; esac ARGV+=( $V ) done From ec096c3e8d453651015e82814db3d9924e04b9ad Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 29 May 2024 03:33:13 +0000 Subject: [PATCH 06/16] Fix split method --- gnmi_server/clientCertAuth.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 790cafb1..1b4ed65a 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -47,7 +47,10 @@ func ClientCertAuthenAndAuthor(ctx context.Context, clientCrtCname string) (cont } func CommonNameMatch(certCommonName string, clientCrtCname string) error { - var trustedCertCommonNames = strings.Split(clientCrtCname, ",") + splitAndRemoveEmpty := func(c rune) bool { + return c == ',' + } + trustedCertCommonNames := strings.FieldsFunc(clientCrtCname, splitAndRemoveEmpty) if len(trustedCertCommonNames) == 0 { // ignore further check because not config trusted cert common names return nil From 6b855075b6977c28f15604fce1e45d66f18ec5ed Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 30 May 2024 08:54:50 +0000 Subject: [PATCH 07/16] Improve coverage --- gnmi_server/server_test.go | 55 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 63b78118..57880c5e 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -20,6 +20,9 @@ import ( "time" "unsafe" + "crypto/x509" + "crypto/x509/pkix" + spb "github.com/sonic-net/sonic-gnmi/proto" sgpb "github.com/sonic-net/sonic-gnmi/proto/gnoi" sdc "github.com/sonic-net/sonic-gnmi/sonic_data_client" @@ -42,6 +45,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/keepalive" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" // Register supported client types. @@ -4218,6 +4222,57 @@ func TestCommonNameMatch(t *testing.T) { } } +func TestClientCertAuthenAndAuthor(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + //src, _ := net.ResolveIPAddr("ip", "1.1.1.1") + cert := x509.Certificate{ + Subject: pkix.Name{ + CommonName: "certname1", + }, + } + verifiedCerts := make([][]*x509.Certificate, 1) + verifiedCerts[0] = make([]*x509.Certificate, 1) + verifiedCerts[0][0] = &cert + p := peer.Peer{ + AuthInfo: credentials.TLSInfo{ + State: tls.ConnectionState{ + VerifiedChains: verifiedCerts, + }, + }, + } + ctx = peer.NewContext(ctx, &p) + defer cancel() + mockpopulate := gomonkey.ApplyFunc(PopulateAuthStruct, func(username string, auth *common_utils.AuthInfo, r []string) error { + return nil + }) + defer mockpopulate.Reset() + // check auth with nil cert name + err := status.Error(codes.Unauthenticated, "") + ctx, err = ClientCertAuthenAndAuthor(ctx, "") + if err != nil { + t.Errorf("CommonNameMatch with empty config should success: %v", err) + } + // check get 1 cert name + ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + // check get multiple cert names + ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1,certname2") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1,certname2") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } + // check a invalid cert cname + ctx, err = ClientCertAuthenAndAuthor(ctx, "certname2,certname3") + if err == nil { + t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) + } +} + func init() { // Enable logs at UT setup flag.Lookup("v").Value.Set("10") From 0f211bf2f2bba4257e71e64f2ba8bae4341876c4 Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Fri, 31 May 2024 16:31:53 +0800 Subject: [PATCH 08/16] Update server_test.go --- gnmi_server/server_test.go | 87 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 57880c5e..e58559c2 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4273,6 +4273,93 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { } } +type MockServerStream struct { + grpc.ServerStream +} + +func (x *MockServerStream) Context() context.Context { + return context.Background() +} + +type MockPingServer struct { + MockServerStream +} + +func (x *MockPingServer) Send(m *gnoi_system_pb.PingResponse) error { + return nil +} + +type MockTracerouteServer struct { + MockServerStream +} + +func (x *MockTracerouteServer) Send(m *gnoi_system_pb.TracerouteResponse) error { + return nil +} + +type MockSetPackageServer struct { + MockServerStream +} + +func (x *MockSetPackageServer) Send(m *gnoi_system_pb.SetPackageResponse) error { + return nil +} + +func (x *MockSetPackageServer) SendAndClose(m *gnoi_system_pb.SetPackageResponse) error { + return nil +} + +func (x *MockSetPackageServer) Recv() (*gnoi_system_pb.SetPackageRequest, error) { + return nil, nil +} + +func TestGnoiAuthorization(t *testing.T) { + s := createServer(t, 8081) + go runServer(t, s) + mockAuthenticate := gomonkey.ApplyFunc(s.Authenticate, func(ctx context.Context, req *spb_jwt.AuthenticateRequest) (*spb_jwt.AuthenticateResponse, error) { + return nil, nil + }) + defer mockAuthenticate.Reset() + + err := s.Ping(new(gnoi_system_pb.PingRequest), new(MockPingServer)) + if err == nil { + t.Errorf("Ping should failed, because not implement.") + } + + s.Traceroute(new(gnoi_system_pb.TracerouteRequest), new(MockTracerouteServer)) + if err == nil { + t.Errorf("Traceroute should failed, because not implement.") + } + + s.SetPackage(new(MockSetPackageServer)) + if err == nil { + t.Errorf("SetPackage should failed, because not implement.") + } + + ctx := context.Background() + s.SwitchControlProcessor(ctx, new(gnoi_system_pb.SwitchControlProcessorRequest)) + if err == nil { + t.Errorf("SwitchControlProcessor should failed, because not implement.") + } + + s.Refresh(ctx, new(spb_jwt.RefreshRequest)) + if err == nil { + t.Errorf("Refresh should failed, because not implement.") + } + + s.ClearNeighbors(ctx, new(spb_gnoi.ClearNeighborsRequest)) + if err == nil { + t.Errorf("ClearNeighbors should failed, because not implement.") + } + + s.CopyConfig(ctx, new(spb_gnoi.CopyConfigRequest)) + if err == nil { + t.Errorf("CopyConfig should failed, because not implement.") + } + + s.Stop() +} + func init() { // Enable logs at UT setup flag.Lookup("v").Value.Set("10") From 3ee207a1b990b15bdfd3b0d1d5b6f38c80d7de2b Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Fri, 31 May 2024 16:45:48 +0800 Subject: [PATCH 09/16] Update server_test.go --- gnmi_server/server_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index e58559c2..8b13df96 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -25,6 +25,7 @@ import ( spb "github.com/sonic-net/sonic-gnmi/proto" sgpb "github.com/sonic-net/sonic-gnmi/proto/gnoi" + spb_jwt "github.com/sonic-net/sonic-gnmi/proto/gnoi/jwt" sdc "github.com/sonic-net/sonic-gnmi/sonic_data_client" sdcfg "github.com/sonic-net/sonic-gnmi/sonic_db_config" ssc "github.com/sonic-net/sonic-gnmi/sonic_service_client" @@ -4347,12 +4348,12 @@ func TestGnoiAuthorization(t *testing.T) { t.Errorf("Refresh should failed, because not implement.") } - s.ClearNeighbors(ctx, new(spb_gnoi.ClearNeighborsRequest)) + s.ClearNeighbors(ctx, new(sgpb.ClearNeighborsRequest)) if err == nil { t.Errorf("ClearNeighbors should failed, because not implement.") } - s.CopyConfig(ctx, new(spb_gnoi.CopyConfigRequest)) + s.CopyConfig(ctx, new(sgpb.CopyConfigRequest)) if err == nil { t.Errorf("CopyConfig should failed, because not implement.") } From f2882ddb5bf0cba45fc70d80feb5b3fbee161799 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 3 Jun 2024 09:17:16 +0000 Subject: [PATCH 10/16] Add cert role mapping --- gnmi_server/clientCertAuth.go | 54 ++++++++++++++++++-------------- gnmi_server/server.go | 5 +-- gnmi_server/server_test.go | 59 +++++++++++++++-------------------- telemetry/telemetry.go | 6 ++-- 4 files changed, 62 insertions(+), 62 deletions(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 1b4ed65a..4f48a712 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -2,16 +2,16 @@ package gnmi import ( "github.com/sonic-net/sonic-gnmi/common_utils" + "github.com/sonic-net/sonic-gnmi/swsscommon" "github.com/golang/glog" "golang.org/x/net/context" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/peer" "google.golang.org/grpc/status" - "strings" ) -func ClientCertAuthenAndAuthor(ctx context.Context, clientCrtCname string) (context.Context, error) { +func ClientCertAuthenAndAuthor(ctx context.Context, serviceConfigTableName string) (context.Context, error) { rc, ctx := common_utils.GetContext(ctx) p, ok := peer.FromContext(ctx) if !ok { @@ -33,34 +33,40 @@ func ClientCertAuthenAndAuthor(ctx context.Context, clientCrtCname string) (cont return ctx, status.Error(codes.Unauthenticated, "invalid username in certificate common name.") } - err := CommonNameMatch(username, clientCrtCname) - if err != nil { - return ctx, err - } - - if err = PopulateAuthStruct(username, &rc.Auth, nil); err != nil { - glog.Infof("[%s] Failed to retrieve authentication information; %v", rc.ID, err) - return ctx, status.Errorf(codes.Unauthenticated, "") + if serviceConfigTableName != "" { + if err := PopulateAuthStructByCommonName(username, &rc.Auth, serviceConfigTableName); err != nil { + return ctx, err + } + } else { + if err := PopulateAuthStruct(username, &rc.Auth, nil); err != nil { + glog.Infof("[%s] Failed to retrieve authentication information; %v", rc.ID, err) + return ctx, status.Errorf(codes.Unauthenticated, "") + } } return ctx, nil } -func CommonNameMatch(certCommonName string, clientCrtCname string) error { - splitAndRemoveEmpty := func(c rune) bool { - return c == ',' - } - trustedCertCommonNames := strings.FieldsFunc(clientCrtCname, splitAndRemoveEmpty) - if len(trustedCertCommonNames) == 0 { - // ignore further check because not config trusted cert common names - return nil +func PopulateAuthStructByCommonName(certCommonName string, auth *common_utils.AuthInfo, serviceConfigTableName string) error { + if serviceConfigTableName == "" { + return status.Errorf(codes.Unauthenticated, "Service config table name should not be empty") } - for _, trustedCertCommonName := range trustedCertCommonNames { - if certCommonName == trustedCertCommonName { - return nil; - } - } + var configDbConnector = swsscommon.NewConfigDBConnector() + defer swsscommon.DeleteConfigDBConnector_Native(configDbConnector.ConfigDBConnector_Native) + configDbConnector.Connect(false) + + var fieldValuePairs = configDbConnector.Get_entry(serviceConfigTableName, "client_crt_cname") + if fieldValuePairs.Has_key(certCommonName) { + var role = fieldValuePairs.Get(certCommonName) + auth.Roles = []string{role} + } - return status.Errorf(codes.Unauthenticated, "Invalid cert cname:'%s', not a trusted cert common name.", certCommonName) + swsscommon.DeleteFieldValueMap(fieldValuePairs) + + if len(auth.Roles) == 0 { + return status.Errorf(codes.Unauthenticated, "Invalid cert cname:'%s', not a trusted cert common name.", certCommonName) + } else { + return nil + } } \ No newline at end of file diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 00aaa3c8..73ac092d 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -65,7 +65,7 @@ type Config struct { EnableNativeWrite bool ZmqPort string IdleConnDuration int - ClientCrtCname string + ConfigTableName string } var AuthLock sync.Mutex @@ -215,6 +215,7 @@ func authenticate(config *Config, ctx context.Context) (context.Context, error) rc.Auth.AuthEnabled = false return ctx, nil } + rc.Auth.AuthEnabled = true if config.UserAuth.Enabled("password") { ctx, err = BasicAuthenAndAuthor(ctx) @@ -229,7 +230,7 @@ func authenticate(config *Config, ctx context.Context) (context.Context, error) } } if !success && config.UserAuth.Enabled("cert") { - ctx, err = ClientCertAuthenAndAuthor(ctx, config.ClientCrtCname) + ctx, err = ClientCertAuthenAndAuthor(ctx, config.ConfigTableName) if err == nil { success = true } diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 8b13df96..d3c49234 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -60,6 +60,7 @@ import ( gnmipb "github.com/openconfig/gnmi/proto/gnmi" gnoi_system_pb "github.com/openconfig/gnoi/system" "github.com/sonic-net/sonic-gnmi/common_utils" + "github.com/sonic-net/sonic-gnmi/swsscommon" ) var clientTypes = []string{gclient.Type} @@ -4192,38 +4193,22 @@ func TestSaveOnSet(t *testing.T) { } } -func TestCommonNameMatch(t *testing.T) { +func TestPopulateAuthStructByCommonName(t *testing.T) { // check auth with nil cert name - err := CommonNameMatch("certname1", "") - if err != nil { - t.Errorf("CommonNameMatch with empty config should success: %v", err) - } - - // check get 1 cert name - err = CommonNameMatch("certname1", "certname1") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } - - // check get multiple cert names - err = CommonNameMatch("certname1", "certname1,certname2") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } - - err = CommonNameMatch("certname2", "certname1,certname2") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } - - // check a invalid cert cname - err = CommonNameMatch("certname3", "certname1,certname2") + err := PopulateAuthStructByCommonName("certname1", "") if err == nil { - t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) + t.Errorf("PopulateAuthStructByCommonName with empty config table should failed: %v", err) } } func TestClientCertAuthenAndAuthor(t *testing.T) { + if !swsscommon.SonicDBConfigIsInit() { + swsscommon.SonicDBConfigInitialize() + } + + var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) + configDb.Flushdb() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) //src, _ := net.ResolveIPAddr("ip", "1.1.1.1") cert := x509.Certificate{ @@ -4253,25 +4238,33 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { if err != nil { t.Errorf("CommonNameMatch with empty config should success: %v", err) } + // check get 1 cert name - ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1") + var gnmiTable = swsscommon.NewTable(configDb, "GNMI") + gnmiTable.Hset("client_crt_cname", "certname1", "role1") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } + // check get multiple cert names - ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1,certname2") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } - ctx, err = ClientCertAuthenAndAuthor(ctx, "certname1,certname2") + gnmiTable.Hset("client_crt_cname", "certname1", "role1") + gnmiTable.Hset("client_crt_cname", "certname2", "role2") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } + // check a invalid cert cname - ctx, err = ClientCertAuthenAndAuthor(ctx, "certname2,certname3") + configDb.Flushdb() + gnmiTable.Hset("client_crt_cname", "certname2", "role2") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err == nil { t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) } + + swsscommon.DeleteTable(gnmiTable) + swsscommon.DeleteDBConnector(configDb) } type MockServerStream struct { diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go index d4d74eb4..e1edbc7e 100644 --- a/telemetry/telemetry.go +++ b/telemetry/telemetry.go @@ -43,7 +43,7 @@ type TelemetryConfig struct { CaCert *string ServerCert *string ServerKey *string - ClientCrtCname *string + ConfigTableName *string ZmqAddress *string ZmqPort *string Insecure *bool @@ -151,7 +151,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { CaCert: fs.String("ca_crt", "", "CA certificate for client certificate validation. Optional."), ServerCert: fs.String("server_crt", "", "TLS server certificate"), ServerKey: fs.String("server_key", "", "TLS server private key"), - ClientCrtCname: fs.String("client_crt_cname", "", "Client cert common name"), + ConfigTableName: fs.String("config_table_name", "", "Config table name"), ZmqAddress: fs.String("zmq_address", "", "Orchagent ZMQ address, deprecated, please use zmq_port."), ZmqPort: fs.String("zmq_port", "", "Orchagent ZMQ port, when not set or empty string telemetry server will switch to Redis based communication channel."), Insecure: fs.Bool("insecure", false, "Skip providing TLS cert and key, for testing only!"), @@ -226,7 +226,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { cfg.LogLevel = int(*telemetryCfg.LogLevel) cfg.Threshold = int(*telemetryCfg.Threshold) cfg.IdleConnDuration = int(*telemetryCfg.IdleConnDuration) - cfg.ClientCrtCname = *telemetryCfg.ClientCrtCname + cfg.ConfigTableName = *telemetryCfg.ConfigTableName // TODO: After other dependent projects are migrated to ZmqPort, remove ZmqAddress zmqAddress := *telemetryCfg.ZmqAddress From 0d30cfc222cde86716f59f88ace36c4920554722 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 3 Jun 2024 09:57:55 +0000 Subject: [PATCH 11/16] Fix yang issue --- gnmi_server/clientCertAuth.go | 14 +++++++++----- gnmi_server/server_test.go | 10 +++++----- telemetry/telemetry_test.go | 6 +++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/gnmi_server/clientCertAuth.go b/gnmi_server/clientCertAuth.go index 4f48a712..9a4a9a4f 100644 --- a/gnmi_server/clientCertAuth.go +++ b/gnmi_server/clientCertAuth.go @@ -56,11 +56,15 @@ func PopulateAuthStructByCommonName(certCommonName string, auth *common_utils.Au defer swsscommon.DeleteConfigDBConnector_Native(configDbConnector.ConfigDBConnector_Native) configDbConnector.Connect(false) - var fieldValuePairs = configDbConnector.Get_entry(serviceConfigTableName, "client_crt_cname") - if fieldValuePairs.Has_key(certCommonName) { - var role = fieldValuePairs.Get(certCommonName) - auth.Roles = []string{role} - } + var fieldValuePairs = configDbConnector.Get_entry(serviceConfigTableName, certCommonName) + if fieldValuePairs.Size() > 0 { + if fieldValuePairs.Has_key("role") { + var role = fieldValuePairs.Get("role") + auth.Roles = []string{role} + } + } else { + glog.Warningf("Failed to retrieve cert common name mapping; %s", certCommonName) + } swsscommon.DeleteFieldValueMap(fieldValuePairs) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index d3c49234..6f6947b4 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4240,16 +4240,16 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { } // check get 1 cert name - var gnmiTable = swsscommon.NewTable(configDb, "GNMI") - gnmiTable.Hset("client_crt_cname", "certname1", "role1") + var gnmiTable = swsscommon.NewTable(configDb, "TELEMETRY_CLIENT_CERT") + gnmiTable.Hset("certname1", "role", "role1") ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } // check get multiple cert names - gnmiTable.Hset("client_crt_cname", "certname1", "role1") - gnmiTable.Hset("client_crt_cname", "certname2", "role2") + gnmiTable.Hset("certname1", "role", "role1") + gnmiTable.Hset("certname2", "role", "role2") ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) @@ -4257,7 +4257,7 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { // check a invalid cert cname configDb.Flushdb() - gnmiTable.Hset("client_crt_cname", "certname2", "role2") + gnmiTable.Hset("certname2", "role", "role2") ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") if err == nil { t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) diff --git a/telemetry/telemetry_test.go b/telemetry/telemetry_test.go index bd69c726..37ec2739 100644 --- a/telemetry/telemetry_test.go +++ b/telemetry/telemetry_test.go @@ -358,15 +358,15 @@ func TestStartGNMIServerCACert(t *testing.T) { }() fs := flag.NewFlagSet("testStartGNMIServerCACert", flag.ContinueOnError) - os.Args = []string{"cmd", "-port", "8080", "-server_crt", testServerCert, "-server_key", testServerKey, "-ca_crt", testServerCACert, "-client_crt_cname", "testcname1"} + os.Args = []string{"cmd", "-port", "8080", "-server_crt", testServerCert, "-server_key", testServerKey, "-ca_crt", testServerCACert, "-config_table_name", "GNMI_CLIENT_CERT"} telemetryCfg, cfg, err := setupFlags(fs) if err != nil { t.Errorf("Expected err to be nil, got err %v", err) } - if cfg.ClientCrtCname != "testcname1" { - t.Errorf("Expected err to be testcname1, got %s", cfg.ClientCrtCname) + if cfg.ConfigTableName != "GNMI_CLIENT_CERT" { + t.Errorf("Expected err to be GNMI_CLIENT_CERT, got %s", cfg.ConfigTableName) } err = createCACert(testServerCACert) From d305bca1aa6f980e824a7daa923d31a66169a7dc Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 3 Jun 2024 10:13:41 +0000 Subject: [PATCH 12/16] Fix build issue --- gnmi_server/gnoi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnmi_server/gnoi.go b/gnmi_server/gnoi.go index b844118c..2feb0e32 100644 --- a/gnmi_server/gnoi.go +++ b/gnmi_server/gnoi.go @@ -42,7 +42,7 @@ func KillOrRestartProcess(restart bool, serviceName string) error { } func (srv *Server) KillProcess(ctx context.Context, req *gnoi_system_pb.KillProcessRequest) (*gnoi_system_pb.KillProcessResponse, error) { - _, err := authenticate(srv.config.UserAuth, ctx) + _, err := authenticate(srv.config, ctx) if err != nil { return nil, err } From 0ae9398a108a0c2649c467bcfd922f1096d20967 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 3 Jun 2024 10:33:47 +0000 Subject: [PATCH 13/16] Fix test case --- gnmi_server/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 6f6947b4..91322799 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4195,7 +4195,7 @@ func TestSaveOnSet(t *testing.T) { func TestPopulateAuthStructByCommonName(t *testing.T) { // check auth with nil cert name - err := PopulateAuthStructByCommonName("certname1", "") + err := PopulateAuthStructByCommonName("certname1", nil, "") if err == nil { t.Errorf("PopulateAuthStructByCommonName with empty config table should failed: %v", err) } From f47be1c89430c2949e83293b84875d7d2d74ed60 Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Tue, 4 Jun 2024 11:30:17 +0800 Subject: [PATCH 14/16] Update server_test.go --- gnmi_server/server_test.go | 52 ++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 91322799..91e61b6b 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4201,16 +4201,8 @@ func TestPopulateAuthStructByCommonName(t *testing.T) { } } -func TestClientCertAuthenAndAuthor(t *testing.T) { - if !swsscommon.SonicDBConfigIsInit() { - swsscommon.SonicDBConfigInitialize() - } - - var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) - configDb.Flushdb() - +func CreateAuthorizationCtx() (context.Context, context.CancelFunc) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - //src, _ := net.ResolveIPAddr("ip", "1.1.1.1") cert := x509.Certificate{ Subject: pkix.Name{ CommonName: "certname1", @@ -4227,42 +4219,70 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { }, } ctx = peer.NewContext(ctx, &p) - defer cancel() + return ctx, cancel +} + +func TestClientCertAuthenAndAuthor(t *testing.T) { + if !swsscommon.SonicDBConfigIsInit() { + swsscommon.SonicDBConfigInitialize() + } + + var configDb = swsscommon.NewDBConnector("CONFIG_DB", uint(0), true) + var gnmiTable = swsscommon.NewTable(configDb, "GNMI_CLIENT_CERT") + configDb.Flushdb() + + // initialize err variable + err := status.Error(codes.Unauthenticated, "") + + // when config table is empty, will authorize with PopulateAuthStruct mockpopulate := gomonkey.ApplyFunc(PopulateAuthStruct, func(username string, auth *common_utils.AuthInfo, r []string) error { return nil }) defer mockpopulate.Reset() + // check auth with nil cert name - err := status.Error(codes.Unauthenticated, "") + ctx, cancel := CreateAuthorizationCtx() ctx, err = ClientCertAuthenAndAuthor(ctx, "") if err != nil { - t.Errorf("CommonNameMatch with empty config should success: %v", err) + t.Errorf("CommonNameMatch with empty config table should success: %v", err) } + cancel() + // check get 1 cert name - var gnmiTable = swsscommon.NewTable(configDb, "TELEMETRY_CLIENT_CERT") + ctx, cancel = CreateAuthorizationCtx() + configDb.Flushdb() gnmiTable.Hset("certname1", "role", "role1") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } + cancel() + // check get multiple cert names + ctx, cancel = CreateAuthorizationCtx() + configDb.Flushdb() gnmiTable.Hset("certname1", "role", "role1") gnmiTable.Hset("certname2", "role", "role2") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") if err != nil { t.Errorf("CommonNameMatch with correct cert name should success: %v", err) } + cancel() + // check a invalid cert cname + ctx, cancel = CreateAuthorizationCtx() configDb.Flushdb() gnmiTable.Hset("certname2", "role", "role2") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI") + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") if err == nil { t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) } + cancel() + swsscommon.DeleteTable(gnmiTable) swsscommon.DeleteDBConnector(configDb) } From b7abf5a13e9e426b27dcda07f3598f21de847fdb Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 5 Jun 2024 02:53:45 +0000 Subject: [PATCH 15/16] Fix comments --- .../dialout_server_cli/dialout_server_cli.go | 1 - gnmi_server/server_test.go | 100 +++++++++--------- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/dialout/dialout_server_cli/dialout_server_cli.go b/dialout/dialout_server_cli/dialout_server_cli.go index 56d80ae9..f3ad5ba6 100644 --- a/dialout/dialout_server_cli/dialout_server_cli.go +++ b/dialout/dialout_server_cli/dialout_server_cli.go @@ -22,7 +22,6 @@ var ( serverKey = flag.String("server_key", "", "TLS server private key") insecure = flag.Bool("insecure", false, "Skip providing TLS cert and key, for testing only!") allowNoClientCert = flag.Bool("allow_no_client_auth", false, "When set, telemetry server will request but not require a client certificate.") - clientCrtCname = flag.String("client_crt_cname", "", "Client cert common name") ) func main() { diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 91e61b6b..35646684 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -4202,27 +4202,27 @@ func TestPopulateAuthStructByCommonName(t *testing.T) { } func CreateAuthorizationCtx() (context.Context, context.CancelFunc) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - cert := x509.Certificate{ - Subject: pkix.Name{ - CommonName: "certname1", - }, - } - verifiedCerts := make([][]*x509.Certificate, 1) - verifiedCerts[0] = make([]*x509.Certificate, 1) - verifiedCerts[0][0] = &cert - p := peer.Peer{ - AuthInfo: credentials.TLSInfo{ - State: tls.ConnectionState{ - VerifiedChains: verifiedCerts, - }, - }, - } - ctx = peer.NewContext(ctx, &p) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + cert := x509.Certificate{ + Subject: pkix.Name{ + CommonName: "certname1", + }, + } + verifiedCerts := make([][]*x509.Certificate, 1) + verifiedCerts[0] = make([]*x509.Certificate, 1) + verifiedCerts[0][0] = &cert + p := peer.Peer{ + AuthInfo: credentials.TLSInfo{ + State: tls.ConnectionState{ + VerifiedChains: verifiedCerts, + }, + }, + } + ctx = peer.NewContext(ctx, &p) return ctx, cancel } -func TestClientCertAuthenAndAuthor(t *testing.T) { + func TestClientCertAuthenAndAuthor(t *testing.T) { if !swsscommon.SonicDBConfigIsInit() { swsscommon.SonicDBConfigInitialize() } @@ -4232,54 +4232,54 @@ func TestClientCertAuthenAndAuthor(t *testing.T) { configDb.Flushdb() // initialize err variable - err := status.Error(codes.Unauthenticated, "") + err := status.Error(codes.Unauthenticated, "") // when config table is empty, will authorize with PopulateAuthStruct - mockpopulate := gomonkey.ApplyFunc(PopulateAuthStruct, func(username string, auth *common_utils.AuthInfo, r []string) error { - return nil - }) - defer mockpopulate.Reset() - - // check auth with nil cert name - ctx, cancel := CreateAuthorizationCtx() - ctx, err = ClientCertAuthenAndAuthor(ctx, "") - if err != nil { - t.Errorf("CommonNameMatch with empty config table should success: %v", err) - } + mockpopulate := gomonkey.ApplyFunc(PopulateAuthStruct, func(username string, auth *common_utils.AuthInfo, r []string) error { + return nil + }) + defer mockpopulate.Reset() + + // check auth with nil cert name + ctx, cancel := CreateAuthorizationCtx() + ctx, err = ClientCertAuthenAndAuthor(ctx, "") + if err != nil { + t.Errorf("CommonNameMatch with empty config table should success: %v", err) + } cancel() - // check get 1 cert name - ctx, cancel = CreateAuthorizationCtx() + // check get 1 cert name + ctx, cancel = CreateAuthorizationCtx() configDb.Flushdb() gnmiTable.Hset("certname1", "role", "role1") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } cancel() - // check get multiple cert names - ctx, cancel = CreateAuthorizationCtx() + // check get multiple cert names + ctx, cancel = CreateAuthorizationCtx() configDb.Flushdb() gnmiTable.Hset("certname1", "role", "role1") gnmiTable.Hset("certname2", "role", "role2") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") - if err != nil { - t.Errorf("CommonNameMatch with correct cert name should success: %v", err) - } + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") + if err != nil { + t.Errorf("CommonNameMatch with correct cert name should success: %v", err) + } cancel() - // check a invalid cert cname - ctx, cancel = CreateAuthorizationCtx() + // check a invalid cert cname + ctx, cancel = CreateAuthorizationCtx() configDb.Flushdb() gnmiTable.Hset("certname2", "role", "role2") - ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") - if err == nil { - t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) - } + ctx, err = ClientCertAuthenAndAuthor(ctx, "GNMI_CLIENT_CERT") + if err == nil { + t.Errorf("CommonNameMatch with invalid cert name should fail: %v", err) + } cancel() @@ -4330,10 +4330,10 @@ func (x *MockSetPackageServer) Recv() (*gnoi_system_pb.SetPackageRequest, error) func TestGnoiAuthorization(t *testing.T) { s := createServer(t, 8081) go runServer(t, s) - mockAuthenticate := gomonkey.ApplyFunc(s.Authenticate, func(ctx context.Context, req *spb_jwt.AuthenticateRequest) (*spb_jwt.AuthenticateResponse, error) { + mockAuthenticate := gomonkey.ApplyFunc(s.Authenticate, func(ctx context.Context, req *spb_jwt.AuthenticateRequest) (*spb_jwt.AuthenticateResponse, error) { return nil, nil }) - defer mockAuthenticate.Reset() + defer mockAuthenticate.Reset() err := s.Ping(new(gnoi_system_pb.PingRequest), new(MockPingServer)) if err == nil { From 90322b35571333c889113cf3bf7a4af85401be17 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 5 Jun 2024 05:15:24 +0000 Subject: [PATCH 16/16] Revert unecessary change --- tools/test/telemetry.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/test/telemetry.sh b/tools/test/telemetry.sh index 10000341..c23a1d08 100755 --- a/tools/test/telemetry.sh +++ b/tools/test/telemetry.sh @@ -12,7 +12,6 @@ for V in "$@"; do -server_crt|--server_crt|-server_crt=*|--server_crt=*) HAS_CERT=1 ;; -server_key|--server_key|-server_key=*|--server_key=*) HAS_CERT=1 ;; -client_auth|--client_auth|-client_auth=*|--client_auth=*) HAS_AUTH=1 ;; - -client_crt_cname|--client_crt_cname|-client_crt_cname=*|--client_crt_cname=*) HAS_CNAME=1 ;; esac ARGV+=( $V ) done