From 24544a62604d25e98d586ef5de57f0a91ccfb63c Mon Sep 17 00:00:00 2001 From: Xavier Henner Date: Fri, 12 Jul 2019 22:33:22 +0200 Subject: [PATCH] optimisations * use pyke's re cache * get an unlimited number of ldap attributes * get a perturbator for the OTP secret, in case of stolen phone * lowercase the username, to avoid strange behaviour with the OTP --- httpd.go | 2 +- ldap.go | 102 ++++++++++++++++------------- main.go | 28 ++++---- openvpn-dm-mgt-server.conf.example | 11 ++-- vendor/modules.txt | 2 +- vpnserver.go | 6 +- vpnsession.go | 29 ++++---- 7 files changed, 96 insertions(+), 84 deletions(-) diff --git a/httpd.go b/httpd.go index d70666f..39fa6cd 100644 --- a/httpd.go +++ b/httpd.go @@ -115,7 +115,7 @@ func (h *HttpServer) ajaxHandler(w http.ResponseWriter, r *http.Request) { return } - profile, _ := h.ovpn.AuthLoop(h.minProfile, + profile, _, _ := h.ovpn.AuthLoop(h.minProfile, strings.Replace(r.TLS.PeerCertificates[0].Subject.CommonName, " ", "", -1), "", false) if profile != h.neededProfile { http.Error(w, fmt.Sprintf("You need the %s profile", h.neededProfile), 403) diff --git a/ldap.go b/ldap.go index 7cb2278..c59ffb6 100644 --- a/ldap.go +++ b/ldap.go @@ -6,28 +6,27 @@ import ( "fmt" "log" "net" - "regexp" "strings" "time" + "github.com/pyke369/golang-support/rcache" "gopkg.in/ldap.v2" ) type ldapConfig struct { - servers []string - baseDN string - bindCn string - bindPw string - searchFilter string - primaryAttribute string - secondaryAttribute string - validGroups []string - mfaType string - certAuth string - ipMin net.IP - ipMax net.IP - upgradeFrom string - routes []string + servers []string + baseDN string + bindCn string + bindPw string + searchFilter string + attributes []string + validGroups []string + mfaType string + certAuth string + ipMin net.IP + ipMax net.IP + upgradeFrom string + routes []string } func (l *ldapConfig) addIPRange(s string) error { @@ -46,24 +45,28 @@ func (l *ldapConfig) addIPRange(s string) error { // auth loop. Try all auth profiles from startProfile // return the last possible profile and the mail if we found a mail like login -func (s *OpenVpnMgt) AuthLoop(startProfile, user, pass string, overridePwdCheck bool) (string, string) { +func (s *OpenVpnMgt) AuthLoop(startProfile, user, pass string, overridePwdCheck bool) (string, string, string) { login := []string{user} profile := startProfile mail := "" + otpSalt := "" - re := regexp.MustCompile("^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$") + re := rcache.Get("^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]+$") for { - if re.MatchString(login[0]) && mail == "" { + if mail == "" && re.MatchString(login[0]) { mail = login[0] } n := profile - for k, ldap := range s.ldap { if ldap.upgradeFrom != profile { continue } - err, userOk, passOk, secondary := ldap.Auth(login, pass) + err, userOk, passOk, attributes := ldap.Auth(login, pass) + + if otpSalt == "" && len(attributes) > 1 && len(attributes[1]) > 0 { + otpSalt = attributes[1][0] + } // if there is an error, try the other configurations if err != nil { @@ -74,7 +77,7 @@ func (s *OpenVpnMgt) AuthLoop(startProfile, user, pass string, overridePwdCheck // we did find a valid User if userOk { // the login for the new auth level is given by the current one - login = secondary + login = attributes[0] if passOk && profile != "" { // it's at least the second auth level, and we have a valid @@ -96,7 +99,7 @@ func (s *OpenVpnMgt) AuthLoop(startProfile, user, pass string, overridePwdCheck } } - return profile, mail + return profile, mail, otpSalt } // override the real DialTLS function @@ -116,13 +119,11 @@ func myDialTLS(network, addr string, config *tls.Config) (*ldap.Conn, error) { return conn, nil } -func (conf *ldapConfig) Auth(logins []string, pass string) (e error, userOk, passOk bool, attributes []string) { - var primary, secondary []string - +func (conf *ldapConfig) Auth(logins []string, pass string) (e error, userOk, passOk bool, attributes [][]string) { // special case. This configuration is a filter on the previous one if len(conf.servers) == 0 && len(conf.validGroups) > 0 { if inArray(logins, conf.validGroups) { - return nil, true, false, logins + return nil, true, false, [][]string{logins} } } @@ -131,7 +132,7 @@ func (conf *ldapConfig) Auth(logins []string, pass string) (e error, userOk, pas return nil, false, false, nil } - attributes = logins + attributes = [][]string{logins} for _, s := range conf.servers { // we force ldaps because we can l, err := myDialTLS("tcp", s+":636", &tls.Config{ServerName: s}) @@ -147,10 +148,7 @@ func (conf *ldapConfig) Auth(logins []string, pass string) (e error, userOk, pas return err, false, false, nil } - search := []string{"dn", conf.primaryAttribute} - if conf.secondaryAttribute != "" { - search = append(search, conf.secondaryAttribute) - } + search := append(conf.attributes, "dn") // search the user searchRequest := ldap.NewSearchRequest( @@ -172,39 +170,51 @@ func (conf *ldapConfig) Auth(logins []string, pass string) (e error, userOk, pas // check the attributes requested in the search // a valid account must be part of the correct group (per instance) - for _, attribute := range sr.Entries[0].Attributes { - if (*attribute).Name == conf.primaryAttribute { - primary = attribute.Values + + ret := [][]string{} + + for _, needed := range conf.attributes { + ok := false + for _, attribute := range sr.Entries[0].Attributes { + if (*attribute).Name == needed { + ret = append(ret, attribute.Values) + ok = true + } } - if (*attribute).Name == conf.secondaryAttribute { - secondary = attribute.Values + if !ok { + ret = append(ret, []string{}) } } - // user must have both primary and secondary attributes - if len(primary) == 0 { - log.Printf("User %s has no %s attribute", logins[0], conf.primaryAttribute) + // user must have both the first and second attributes + if len(ret[0]) == 0 { + log.Printf("User %s has no %s attribute", logins[0], conf.attributes[0]) return nil, false, false, nil } - if len(secondary) == 0 { - log.Printf("User %s has no %s attribute", logins[0], conf.secondaryAttribute) + if len(ret[1]) == 0 { + log.Printf("User %s has no %s attribute", logins[0], conf.attributes[1]) return nil, false, false, nil } - // check if the primary attributes are in the validGroups list - if len(conf.validGroups) > 0 && !inArray(conf.validGroups, primary) { + // check if the first attribute valus are in the validGroups list + if len(conf.validGroups) > 0 && !inArray(conf.validGroups, ret[0]) { return nil, false, false, nil } - // if there is no validGroups check, pass the primary attributes to the + // if there is no validGroups check, pass the first attribute values to the // next level if len(conf.validGroups) == 0 { - attributes = primary + attributes = [][]string{ret[0]} } else { - attributes = secondary + attributes = [][]string{ret[1]} } + if len(ret) > 2 { + attributes = append(attributes, ret[2:]...) + } + log.Println(attributes) + log.Printf("User %s has a valid account on %s", logins[0], s) userdn := sr.Entries[0].DN diff --git a/main.go b/main.go index 2fa402c..7c9631b 100644 --- a/main.go +++ b/main.go @@ -64,21 +64,25 @@ func main() { for _, profile := range config.GetPaths("config.profiles") { profileName := strings.Split(profile, ".")[2] ldapConf := ldapConfig{ - servers: parseConfigArray(config, profile+".servers"), - baseDN: config.GetString(profile+".baseDN", ""), - bindCn: config.GetString(profile+".bindCn", ""), - bindPw: config.GetString(profile+".bindPw", ""), - searchFilter: config.GetString(profile+".searchFilter", ""), - primaryAttribute: config.GetString(profile+".primaryAttribute", ""), - secondaryAttribute: config.GetString(profile+".secondaryAttribute", ""), - validGroups: parseConfigArray(config, profile+".validGroups"), - routes: parseConfigArray(config, profile+".routes"), - mfaType: config.GetString(profile+".mfa", ""), - certAuth: config.GetString(profile+".cert", "optionnal"), - upgradeFrom: config.GetString(profile+".upgradeFrom", ""), + servers: parseConfigArray(config, profile+".servers"), + baseDN: config.GetString(profile+".baseDN", ""), + bindCn: config.GetString(profile+".bindCn", ""), + bindPw: config.GetString(profile+".bindPw", ""), + searchFilter: config.GetString(profile+".searchFilter", ""), + attributes: parseConfigArray(config, profile+".attributes"), + validGroups: parseConfigArray(config, profile+".validGroups"), + routes: parseConfigArray(config, profile+".routes"), + mfaType: config.GetString(profile+".mfa", ""), + certAuth: config.GetString(profile+".cert", "optionnal"), + upgradeFrom: config.GetString(profile+".upgradeFrom", ""), } ldapConf.addIPRange(config.GetString(profile+".IPRange", "")) + if len(ldapConf.servers) > 0 && len(ldapConf.attributes) < 2 { + log.Println("valud ldap configuration must have 2 attributes") + os.Exit(1) + } + server.ldap[profileName] = ldapConf } diff --git a/openvpn-dm-mgt-server.conf.example b/openvpn-dm-mgt-server.conf.example index f110cb2..8b6577b 100644 --- a/openvpn-dm-mgt-server.conf.example +++ b/openvpn-dm-mgt-server.conf.example @@ -9,8 +9,7 @@ config bindCn: "CN=VPN Service,OU=Services,OU=Dailymotion,DC=office,DC=daily", bindPw: "********************", searchFilter: "(&(sAMAccountName=%s))" - primaryAttribute: "memberOf" - secondaryAttribute: "mail" + attributes: [ "memberOf", "mail" ] validGroups: [ "CN=SEC_VPN_Users_External,OU=Security,OU=Groups,OU=Dailymotion,DC=office,DC=daily", @@ -39,8 +38,7 @@ config bindCn: "CN=VPN Service,OU=Services,OU=Dailymotion,DC=office,DC=daily", bindPw: "********************", searchFilter: "(&(sAMAccountName=%s))" - primaryAttribute: "memberOf" - secondaryAttribute: "mail" + attributes: [ "memberOf", "mail" ] validGroups: [ "CN=SEC_VPN,OU=Security,OU=Groups,OU=Dailymotion,DC=office,DC=daily", @@ -56,8 +54,7 @@ config bindCn: "cn=readonly,dc=dailymotion,dc=com" bindPw: "**********" searchFilter: "(&(mail=%s))" - primaryAttribute: "description" - secondaryAttribute: "sshPublicKey" + attributes: [ "description", "sshPublicKey" ] upgradeFrom: "CORP" mfa: "" cert: "optionnal" @@ -67,7 +64,7 @@ config { validGroups: [ - "infra2", + "infra", "net", "datacenter", ] diff --git a/vendor/modules.txt b/vendor/modules.txt index 38c00a5..dc58086 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,8 +1,8 @@ # github.com/mattevans/pwned-passwords v0.0.0-20190611210716-1da592be4a34 github.com/mattevans/pwned-passwords # github.com/pyke369/golang-support v0.0.0-20190703174728-34ca97aa79e9 -github.com/pyke369/golang-support/uconfig github.com/pyke369/golang-support/rcache +github.com/pyke369/golang-support/uconfig # gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d gopkg.in/asn1-ber.v1 # gopkg.in/ldap.v2 v2.5.1 diff --git a/vpnserver.go b/vpnserver.go index 5591f02..69fc122 100644 --- a/vpnserver.go +++ b/vpnserver.go @@ -8,12 +8,12 @@ import ( "log" "net" "os" - "regexp" "strconv" "strings" "sync" hibp "github.com/mattevans/pwned-passwords" + "github.com/pyke369/golang-support/rcache" ) // Server represents the server @@ -136,7 +136,7 @@ func (s *OpenVpnMgt) Kill(session string, id int) error { // send the help command on all vpn servers. Kind of useless func (s *OpenVpnMgt) Help() (error, map[string]map[string]string) { ret := make(map[string]map[string]string) - re := regexp.MustCompile("^(.*[^ ]) *: (.*)$") + re := rcache.Get("^(.*[^ ]) *: (.*)$") for remote := range s.buf { help := make(map[string]string) err, msg := s.sendCommand([]string{"help"}, remote) @@ -247,7 +247,7 @@ func (s *OpenVpnMgt) ClientReAuth(line, remote string) { // find a client among all registered sessions func (s *OpenVpnMgt) getClient(line, remote string) (error, *vpnSession) { - re := regexp.MustCompile("^[^0-9]*,([0-9]+)[^0-9]*") + re := rcache.Get("^[^0-9]*,([0-9]+)[^0-9]*") match := re.FindStringSubmatch(line) if len(match) == 0 { return errors.New("invalid message"), nil diff --git a/vpnsession.go b/vpnsession.go index 6dfe952..28efcf9 100644 --- a/vpnsession.go +++ b/vpnsession.go @@ -5,9 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "github.com/pyke369/golang-support/rcache" "os" "os/exec" - "regexp" "strconv" "strings" "time" @@ -84,7 +84,7 @@ func (c *vpnSession) AddRoute(ip string) error { func (c *vpnSession) ParseSessionId(line string) error { var err error - re := regexp.MustCompile("^>CLIENT:[^,]*,([0-9]+),([0-9]+)$") + re := rcache.Get("^>CLIENT:[^,]*,([0-9]+),([0-9]+)$") match := re.FindStringSubmatch(line) if len(match) == 0 { return errors.New("invalid message") @@ -101,8 +101,8 @@ func (c *vpnSession) ParseSessionId(line string) error { func (c *vpnSession) ParseEnv(s *OpenVpnMgt, infos *[]string) error { var err error - r := regexp.MustCompile("[^a-zA-Z0-9./_@-]") - renv := regexp.MustCompile("^>CLIENT:ENV,([^=]*)=(.*)$") + r := rcache.Get("[^a-zA-Z0-9./_@-]") + renv := rcache.Get("^>CLIENT:ENV,([^=]*)=(.*)$") for _, line := range *infos { p := renv.FindStringSubmatch(line) if len(p) != 3 { @@ -182,7 +182,7 @@ func (c *vpnSession) ParseEnv(s *OpenVpnMgt, infos *[]string) error { } case "username": - c.Login = r.ReplaceAllString(p[2], "") + c.Login = strings.ToLower(r.ReplaceAllString(p[2], "")) case "dev": c.dev = r.ReplaceAllString(p[2], "") case "ifconfig_netmask": @@ -260,11 +260,20 @@ func (c *vpnSession) auth(s *OpenVpnMgt) (error, int) { c.password = "" } + otpSalt := "" + c.Profile, c.Mail, otpSalt = s.AuthLoop("", c.Login, c.password, tokenPasswordOk) + + // no profile validated, we stop here + if c.Profile == "" { + c.Status = "fail (password)" + return errors.New("Authentication Failed"), -3 + } + // if the otp is not empty, we check it against the valid codes as soon as // possible otpvalidated := false if c.otpCode != "" { - codes, err := s.GenerateOTP(c.Login) + codes, err := s.GenerateOTP(c.Login + otpSalt) if err != nil { return err, -2 } @@ -275,14 +284,6 @@ func (c *vpnSession) auth(s *OpenVpnMgt) (error, int) { } } - c.Profile, c.Mail = s.AuthLoop("", c.Login, c.password, tokenPasswordOk) - - // no profile validated, we stop here - if c.Profile == "" { - c.Status = "fail (password)" - return errors.New("Authentication Failed"), -3 - } - // check the MFA requested by the secured profile c.TwoFA = true switch s.ldap[c.Profile].mfaType {