From c697c9aaebd68b5be385c7b21d2db31ddf0e8f35 Mon Sep 17 00:00:00 2001 From: Sergey Yashchuk Date: Fri, 18 Sep 2020 01:24:51 +0700 Subject: [PATCH 1/3] dev: ACLs external command hook --- main.go | 5 ++++ pkg/bastion/acl.go | 56 ++++++++++++++++++++++++++++++++++++++--- pkg/bastion/acl_test.go | 2 +- pkg/bastion/ssh.go | 9 ++++--- server.go | 6 +++-- 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/main.go b/main.go index 3e9c46a..d80e63f 100644 --- a/main.go +++ b/main.go @@ -83,6 +83,11 @@ func main() { Value: 0, Usage: "Duration before an inactive connection is timed out (0 to disable)", }, + cli.StringFlag{ + Name: "acl-check-cmd", + EnvVar: "SSHPORTAL_ACL_CHECK_CMD", + Usage: "Execute external command to check ACL", + }, }, }, { Name: "healthcheck", diff --git a/pkg/bastion/acl.go b/pkg/bastion/acl.go index 6c25031..711f7e8 100644 --- a/pkg/bastion/acl.go +++ b/pkg/bastion/acl.go @@ -1,7 +1,11 @@ package bastion import ( + "encoding/json" + "log" + "os/exec" "sort" + "strings" "time" "moul.io/sshportal/pkg/dbmodels" @@ -13,7 +17,7 @@ func (a byWeight) Len() int { return len(a) } func (a byWeight) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a byWeight) Less(i, j int) bool { return a[i].Weight < a[j].Weight } -func checkACLs(user dbmodels.User, host dbmodels.Host) string { +func checkACLs(user dbmodels.User, host dbmodels.Host, aclCheckCmd string) string { currentTime := time.Now() // shared ACLs between user and host @@ -36,7 +40,7 @@ func checkACLs(user dbmodels.User, host dbmodels.Host) string { // deny by default if no shared ACL if len(aclMap) == 0 { - return string(dbmodels.ACLActionDeny) // default action + return checkACLsHook(aclCheckCmd, string(dbmodels.ACLActionDeny), user, host) // default action } // transform map to slice and sort it @@ -46,5 +50,51 @@ func checkACLs(user dbmodels.User, host dbmodels.Host) string { } sort.Sort(byWeight(acls)) - return acls[0].Action + return checkACLsHook(aclCheckCmd, acls[0].Action, user, host) +} + +// checkACLsHook executes external command to check ACL and passes following parameters: +// $1 - SSH Portal action (`allow` or `deny`) +// $2 - User as JSON string +// $3 - Host as JSON string +// External program has to return `allow` or `deny` in stdout. +func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host dbmodels.Host) string { + if aclCheckCmd != "" { + jsonUser, err := json.Marshal(user) + if err != nil { + log.Printf("Error: %v", err) + return action + } + + jsonHost, err := json.Marshal(host) + if err != nil { + log.Printf("Error: %v", err) + return action + } + + args := []string{ + action, + string(jsonUser), + string(jsonHost), + } + + out, err := exec.Command(aclCheckCmd, args...).CombinedOutput() + if err != nil { + log.Printf("Error: %v", err) + return action + } + + outStr := strings.TrimSuffix(string(out), "\n") + + switch outStr { + case string(dbmodels.ACLActionAllow): + return string(dbmodels.ACLActionAllow) + case string(dbmodels.ACLActionDeny): + return string(dbmodels.ACLActionDeny) + default: + log.Printf("Error: acl-check-cmd wrong output '%s'\n", outStr) + return action + } + } + return action } diff --git a/pkg/bastion/acl_test.go b/pkg/bastion/acl_test.go index 3c49fb7..0a7fa0b 100644 --- a/pkg/bastion/acl_test.go +++ b/pkg/bastion/acl_test.go @@ -43,7 +43,7 @@ func TestCheckACLs(t *testing.T) { db.Preload("Groups").Preload("Groups.ACLs").Find(&users) // test - action := checkACLs(users[0], hosts[0]) + action := checkACLs(users[0], hosts[0], "") c.So(action, ShouldEqual, dbmodels.ACLActionAllow) }) } diff --git a/pkg/bastion/ssh.go b/pkg/bastion/ssh.go index 33b4a6f..4b12f44 100644 --- a/pkg/bastion/ssh.go +++ b/pkg/bastion/ssh.go @@ -28,6 +28,7 @@ type authContext struct { db *gorm.DB userKey dbmodels.UserKey logsLocation string + aclCheckCmd string aesKey string dbDriver, dbURL string bindAddr string @@ -206,7 +207,7 @@ func bastionClientConfig(ctx ssh.Context, host *dbmodels.Host) (*gossh.ClientCon return nil, err } - action := checkACLs(tmpUser, tmpHost) + action := checkACLs(tmpUser, tmpHost, actx.aclCheckCmd) switch action { case string(dbmodels.ACLActionAllow): // do nothing @@ -251,12 +252,13 @@ func ShellHandler(s ssh.Session, version, gitSha, gitTag string) { panic("should not happen") } -func PasswordAuthHandler(db *gorm.DB, logsLocation, aesKey, dbDriver, dbURL, bindAddr string, demo bool) ssh.PasswordHandler { +func PasswordAuthHandler(db *gorm.DB, logsLocation, aclCheckCmd, aesKey, dbDriver, dbURL, bindAddr string, demo bool) ssh.PasswordHandler { return func(ctx ssh.Context, pass string) bool { actx := &authContext{ db: db, inputUsername: ctx.User(), logsLocation: logsLocation, + aclCheckCmd: aclCheckCmd, aesKey: aesKey, dbDriver: dbDriver, dbURL: dbURL, @@ -287,12 +289,13 @@ func PrivateKeyFromDB(db *gorm.DB, aesKey string) func(*ssh.Server) error { } } -func PublicKeyAuthHandler(db *gorm.DB, logsLocation, aesKey, dbDriver, dbURL, bindAddr string, demo bool) ssh.PublicKeyHandler { +func PublicKeyAuthHandler(db *gorm.DB, logsLocation, aclCheckCmd, aesKey, dbDriver, dbURL, bindAddr string, demo bool) ssh.PublicKeyHandler { return func(ctx ssh.Context, key ssh.PublicKey) bool { actx := &authContext{ db: db, inputUsername: ctx.User(), logsLocation: logsLocation, + aclCheckCmd: aclCheckCmd, aesKey: aesKey, dbDriver: dbDriver, dbURL: dbURL, diff --git a/server.go b/server.go index 471715b..bbc05ca 100644 --- a/server.go +++ b/server.go @@ -22,6 +22,7 @@ type serverConfig struct { bindAddr string debug, demo bool idleTimeout time.Duration + aclCheckCmd string } func parseServerConfig(c *cli.Context) (*serverConfig, error) { @@ -34,6 +35,7 @@ func parseServerConfig(c *cli.Context) (*serverConfig, error) { demo: c.Bool("demo"), logsLocation: c.String("logs-location"), idleTimeout: c.Duration("idle-timeout"), + aclCheckCmd: c.String("acl-check-cmd"), } switch len(ret.aesKey) { case 0, 16, 24, 32: @@ -119,8 +121,8 @@ func server(c *serverConfig) (err error) { for _, opt := range []ssh.Option{ // custom PublicKeyAuth handler - ssh.PublicKeyAuth(bastion.PublicKeyAuthHandler(db, c.logsLocation, c.aesKey, c.dbDriver, c.dbURL, c.bindAddr, c.demo)), - ssh.PasswordAuth(bastion.PasswordAuthHandler(db, c.logsLocation, c.aesKey, c.dbDriver, c.dbURL, c.bindAddr, c.demo)), + ssh.PublicKeyAuth(bastion.PublicKeyAuthHandler(db, c.logsLocation, c.aclCheckCmd, c.aesKey, c.dbDriver, c.dbURL, c.bindAddr, c.demo)), + ssh.PasswordAuth(bastion.PasswordAuthHandler(db, c.logsLocation, c.aclCheckCmd, c.aesKey, c.dbDriver, c.dbURL, c.bindAddr, c.demo)), // retrieve sshportal SSH private key from database bastion.PrivateKeyFromDB(db, c.aesKey), } { From 32fcfa370c76ba5b7418d54fd6af4d0533a18fbf Mon Sep 17 00:00:00 2001 From: Sergey Yashchuk Date: Fri, 2 Apr 2021 10:29:46 +0700 Subject: [PATCH 2/3] Fixes related to comments in PR --- pkg/bastion/acl.go | 104 +++++++++++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/pkg/bastion/acl.go b/pkg/bastion/acl.go index 711f7e8..f5ebe61 100644 --- a/pkg/bastion/acl.go +++ b/pkg/bastion/acl.go @@ -1,7 +1,9 @@ package bastion import ( + "context" "encoding/json" + "fmt" "log" "os/exec" "sort" @@ -11,6 +13,9 @@ import ( "moul.io/sshportal/pkg/dbmodels" ) +// AclHookTimeout is timeout for external ACL hook execution +const AclHookTimeout = 2 * time.Second + type byWeight []*dbmodels.ACL func (a byWeight) Len() int { return len(a) } @@ -38,9 +43,13 @@ func checkACLs(user dbmodels.User, host dbmodels.Host, aclCheckCmd string) strin } // FIXME: add ACLs that match host pattern - // deny by default if no shared ACL + // if no shared ACL then execute ACLs hook if it exists and return its result if len(aclMap) == 0 { - return checkACLsHook(aclCheckCmd, string(dbmodels.ACLActionDeny), user, host) // default action + action, err := checkACLsHook(aclCheckCmd, string(dbmodels.ACLActionDeny), user, host) + if err != nil { + log.Println(err) + } + return action } // transform map to slice and sort it @@ -50,51 +59,62 @@ func checkACLs(user dbmodels.User, host dbmodels.Host, aclCheckCmd string) strin } sort.Sort(byWeight(acls)) - return checkACLsHook(aclCheckCmd, acls[0].Action, user, host) + action, err := checkACLsHook(aclCheckCmd, acls[0].Action, user, host) + if err != nil { + log.Println(err) + } + + return action } // checkACLsHook executes external command to check ACL and passes following parameters: -// $1 - SSH Portal action (`allow` or `deny`) +// $1 - SSH Portal `action` (`allow` or `deny`) // $2 - User as JSON string // $3 - Host as JSON string // External program has to return `allow` or `deny` in stdout. -func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host dbmodels.Host) string { - if aclCheckCmd != "" { - jsonUser, err := json.Marshal(user) - if err != nil { - log.Printf("Error: %v", err) - return action - } - - jsonHost, err := json.Marshal(host) - if err != nil { - log.Printf("Error: %v", err) - return action - } - - args := []string{ - action, - string(jsonUser), - string(jsonHost), - } - - out, err := exec.Command(aclCheckCmd, args...).CombinedOutput() - if err != nil { - log.Printf("Error: %v", err) - return action - } - - outStr := strings.TrimSuffix(string(out), "\n") - - switch outStr { - case string(dbmodels.ACLActionAllow): - return string(dbmodels.ACLActionAllow) - case string(dbmodels.ACLActionDeny): - return string(dbmodels.ACLActionDeny) - default: - log.Printf("Error: acl-check-cmd wrong output '%s'\n", outStr) - return action - } +// In case of any error function returns `action`. +func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host dbmodels.Host) (string, error) { + if aclCheckCmd == "" { + return action, nil + } + + ctx, cancel := context.WithTimeout(context.Background(), AclHookTimeout) + defer cancel() + + jsonUser, err := json.Marshal(user) + if err != nil { + return action, err + } + + jsonHost, err := json.Marshal(host) + if err != nil { + return action, err + } + + args := []string{ + action, + string(jsonUser), + string(jsonHost), + } + + cmd := exec.CommandContext(ctx, aclCheckCmd, args...) + out, err := cmd.Output() + if err != nil { + return action, err + } + + if ctx.Err() == context.DeadlineExceeded { + return action, fmt.Errorf("external ACL hook command timed out") + } + + outStr := strings.TrimSuffix(string(out), "\n") + + switch outStr { + case string(dbmodels.ACLActionAllow): + return string(dbmodels.ACLActionAllow), nil + case string(dbmodels.ACLActionDeny): + return string(dbmodels.ACLActionDeny), nil + default: + return action, fmt.Errorf("acl-check-cmd wrong output '%s'", outStr) } - return action } From 97bf5d3168f75f0dbe2645f810eb12e25a4add5c Mon Sep 17 00:00:00 2001 From: Sergey Yashchuk Date: Fri, 2 Apr 2021 10:49:18 +0700 Subject: [PATCH 3/3] lint fix --- pkg/bastion/acl.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/bastion/acl.go b/pkg/bastion/acl.go index f5ebe61..f401f83 100644 --- a/pkg/bastion/acl.go +++ b/pkg/bastion/acl.go @@ -13,8 +13,8 @@ import ( "moul.io/sshportal/pkg/dbmodels" ) -// AclHookTimeout is timeout for external ACL hook execution -const AclHookTimeout = 2 * time.Second +// ACLHookTimeout is timeout for external ACL hook execution +const ACLHookTimeout = 2 * time.Second type byWeight []*dbmodels.ACL @@ -78,7 +78,7 @@ func checkACLsHook(aclCheckCmd string, action string, user dbmodels.User, host d return action, nil } - ctx, cancel := context.WithTimeout(context.Background(), AclHookTimeout) + ctx, cancel := context.WithTimeout(context.Background(), ACLHookTimeout) defer cancel() jsonUser, err := json.Marshal(user)