Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for JWT authentication mode in telemetry #152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions gnmi_server/gnoi.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/sonic-net/sonic-gnmi/common_utils"
"google.golang.org/grpc/status"
"google.golang.org/grpc/codes"
"os/user"
"encoding/json"
jwt "github.com/dgrijalva/jwt-go"
)
Expand Down Expand Up @@ -129,19 +128,18 @@ func (srv *Server) Authenticate(ctx context.Context, req *spb_jwt.AuthenticateRe
log.V(1).Info("gNOI: Sonic Authenticate")


if !srv.config.UserAuth.Enabled("jwt") {
if srv.config.UserAuth.Enabled("jwt") {
log.V(1).Info("gNOI: Sonic Authenticate - JWT enabled")
} else {
log.V(1).Info("gNOI: Sonic Authenticate - JWT not enabled"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax error: missing ) at the end

return nil, status.Errorf(codes.Unimplemented, "")
}
auth_success, _ := UserPwAuth(req.Username, req.Password)
if auth_success {
usr, err := user.Lookup(req.Username)
roles, err := GetUserRoles(req.Username)
if err == nil {
roles, err := GetUserRoles(usr)
if err == nil {
return &spb_jwt.AuthenticateResponse{Token: tokenResp(req.Username, roles)}, nil
}
return &spb_jwt.AuthenticateResponse{Token: tokenResp(req.Username, roles)}, nil
}

}
return nil, status.Errorf(codes.PermissionDenied, "Invalid Username or Password")

Expand All @@ -159,6 +157,7 @@ func (srv *Server) Refresh(ctx context.Context, req *spb_jwt.RefreshRequest) (*s

token, _, err := JwtAuthenAndAuthor(ctx)
if err != nil {
log.Errorf("gNOI: Sonic Refresh - JWTAuthenandAuthor returned error")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add information what error was returned.

return nil, err
}

Expand Down
41 changes: 20 additions & 21 deletions gnmi_server/pamAuth.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package gnmi

import (
"github.com/sonic-net/sonic-gnmi/common_utils"
"github.com/Azure/sonic-telemetry/common_utils"
"errors"
"strings"
"github.com/golang/glog"
"github.com/msteinert/pam"
"golang.org/x/crypto/ssh"
"os/user"
"github.com/Azure/sonic-mgmt-common/translib/transformer"
)

type UserCredential struct {
Expand Down Expand Up @@ -46,32 +47,30 @@ func PAMAuthUser(u string, p string) error {
err := cred.PAMAuthenticate()
return err
}
func GetUserRoles(usr *user.User) ([]string, error) {
// Lookup Roles
gids, err := usr.GroupIds()
if err != nil {
return nil, err
}
roles := make([]string, len(gids))
for idx, gid := range gids {
group, err := user.LookupGroupId(gid)
if err != nil {
return nil, err
func GetUserRoles(username string) ([]string, error) {
// Lookup Roles using Dbus
var cmd string
cmd = "user_auth_mgmt.retrieve_user_roles"
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two lines can be combined in just one:

	cmd := "user_auth_mgmt.retrieve_user_roles"

host_output := transformer.HostQuery(cmd, username)
if host_output.Err != nil {
glog.Errorf("System user roles host query failed")
return nil,errors.New("Failed to retrieve user roles")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a space after the comma.

} else {
if val, _ := host_output.Body[0].(int32); val == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not ignore errors. The condition can look like follows:

if val, ok := host_output.Body[0].(int32); ok && val == 0 {

glog.Infof("Roles retrieved from host")
roles := strings.Split(host_output.Body[1].(string), ",")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not ignore errors. If Body[1] is not a string then the program will panic. Even if this not normally/today possible it might change in the future.

return roles,nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a space after the comma.

} else {
glog.Errorf("Invalid User. no roles")
return nil,errors.New(host_output.Body[1].(string))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a space after the comma.

}
roles[idx] = group.Name
}
return roles, nil
}
func PopulateAuthStruct(username string, auth *common_utils.AuthInfo, r []string) error {
if len(r) == 0 {
AuthLock.Lock()
defer AuthLock.Unlock()
usr, err := user.Lookup(username)
if err != nil {
return err
}

roles, err := GetUserRoles(usr)
roles,err := GetUserRoles(username)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a space after the comma.

if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion gnoi_client/gnoi_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ var (
jwtToken = flag.String("jwt_token", "", "JWT Token if required")
targetName = flag.String("target_name", "hostname.com", "The target name use to verify the hostname returned by TLS handshake")
)
var username string = ""
var password string = ""
Comment on lines +26 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to define those two new variables outside the var declaration starting in line 18?
Could you also add information where the password and username flags are defined?


func setUserCreds(ctx context.Context) context.Context {
if len(*jwtToken) > 0 {
ctx = metadata.AppendToOutgoingContext(ctx, "access_token", *jwtToken)
Expand All @@ -31,6 +34,8 @@ func setUserCreds(ctx context.Context) context.Context {
}
func main() {
flag.Parse()
username = flag.Lookup("username").Value.String()
password = flag.Lookup("password").Value.String()
opts := credentials.ClientCredentials(*targetName)

ctx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -254,7 +259,7 @@ func imageDefault(sc spb.SonicServiceClient, ctx context.Context) {
func authenticate(sc spb_jwt.SonicJwtServiceClient, ctx context.Context) {
fmt.Println("Sonic Authenticate")
ctx = setUserCreds(ctx)
req := &spb_jwt.AuthenticateRequest {}
req := &spb_jwt.AuthenticateRequest {Username: username, Password: password}

json.Unmarshal([]byte(*args), req)

Expand Down
27 changes: 26 additions & 1 deletion patches/gnmi_cli.all.patch
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@
}
--- ./github.com/openconfig/gnmi/cmd/gnmi_cli/gnmi_cli.go 2019-11-22 14:03:29.839103602 -0800
+++ ./github.com/openconfig/gnmi/cmd/gnmi_cli/gnmi_cli.go 2019-11-21 09:30:52.453893674 -0800
@@ -61,7 +61,7 @@

clientTypes = flags.NewStringList(&cfg.ClientTypes, []string{gclient.Type})
queryFlag = &flags.StringList{}
- queryType = flag.String("query_type", client.Once.String(), "Type of result, one of: (o, once, p, polling, s, streaming).")
+ queryType = flag.String("query_type", "", "Type of result, one of: (p, polling, s, streaming).")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to specify the default mode? Say, polling?

queryAddr = flags.NewStringList(&q.Addrs, nil)

reqProto = flag.String("proto", "", "Text proto for gNMI request.")
@@ -76,6 +76,11 @@
caCert = flag.String("ca_crt", "", "CA certificate file. Used to verify server TLS certificate.")
clientCert = flag.String("client_crt", "", "Client certificate file. Used for client certificate-based authentication.")
Expand All @@ -23,7 +32,23 @@
)

func init() {
@@ -278,6 +283,22 @@
@@ -83,7 +88,7 @@ func init() {
flag.Var(queryFlag, "query", "Comma separated list of queries. Each query is a delimited list of OpenConfig path nodes which may also be specified as a glob (*). The delimeter can be specified with the --delimiter flag.")
// Query command-line flags.
flag.Var(queryAddr, "address", "Address of the GNMI target to query.")
- flag.BoolVar(&q.UpdatesOnly, "updates_only", false, "Only stream updates, not the initial sync. Setting this flag for once or polling queries will cause nothing to be returned.")
+ flag.BoolVar(&q.UpdatesOnly, "updates_only", false, "Only stream updates, not the initial sync. Setting this flag for polling queries will cause nothing to be returned.")
// Config command-line flags.
flag.DurationVar(&cfg.PollingInterval, "polling_interval", 30*time.Second, "Interval at which to poll in seconds if polling is specified for query_type.")
flag.UintVar(&cfg.Count, "count", 0, "Number of polling/streaming events (0 is infinite).")
@@ -272,12 +277,28 @@ func executeSubscribe(ctx context.Context) error {
}

if q.Type = cli.QueryType(*queryType); q.Type == client.Unknown {
- return errors.New("--query_type must be one of: (o, once, p, polling, s, streaming)")
+ return errors.New("--query_type must be one of: (p, polling, s, streaming)")
}
// Parse queryFlag into appropriate format.
if len(*queryFlag) == 0 {
return errors.New("--query must be set")
}
Expand Down
1 change: 1 addition & 0 deletions telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func main() {

cfg := &gnmi.Config{}
cfg.Port = int64(*port)
cfg.UserAuth = userAuth
cfg.EnableTranslibWrite = bool(*gnmi_translib_write)
cfg.EnableNativeWrite = bool(*gnmi_native_write)
cfg.LogLevel = 3
Expand Down
Loading