diff --git a/changelog/unreleased/ldap-tls-insecure.md b/changelog/unreleased/ldap-tls-insecure.md new file mode 100644 index 0000000000..930f0e6cce --- /dev/null +++ b/changelog/unreleased/ldap-tls-insecure.md @@ -0,0 +1,8 @@ +Enhancement: Safer defaults for TLS verification on LDAP connections + +The LDAP client connections were hardcoded to ignore certificate validation +errors. Now verification is enabled by default and a new config parameter 'insecure' +is introduced to override that default. It is also possible to add trusted Certificates +by using the new 'cacert' config paramter. + +https://github.com/cs3org/reva/pull/2053 diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go index b136a05f5d..d956b26d9d 100644 --- a/pkg/auth/manager/ldap/ldap.go +++ b/pkg/auth/manager/ldap/ldap.go @@ -20,8 +20,6 @@ package ldap import ( "context" - "crypto/tls" - "fmt" "strconv" "strings" @@ -36,6 +34,7 @@ import ( "github.com/cs3org/reva/pkg/logger" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/sharedconf" + "github.com/cs3org/reva/pkg/utils" "github.com/go-ldap/ldap/v3" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -50,17 +49,16 @@ type mgr struct { } type config struct { - Hostname string `mapstructure:"hostname"` - Port int `mapstructure:"port"` - BaseDN string `mapstructure:"base_dn"` - UserFilter string `mapstructure:"userfilter"` - LoginFilter string `mapstructure:"loginfilter"` - BindUsername string `mapstructure:"bind_username"` - BindPassword string `mapstructure:"bind_password"` - Idp string `mapstructure:"idp"` - GatewaySvc string `mapstructure:"gatewaysvc"` - Schema attributes `mapstructure:"schema"` - Nobody int64 `mapstructure:"nobody"` + utils.LDAPConn `mapstructure:",squash"` + BaseDN string `mapstructure:"base_dn"` + UserFilter string `mapstructure:"userfilter"` + LoginFilter string `mapstructure:"loginfilter"` + BindUsername string `mapstructure:"bind_username"` + BindPassword string `mapstructure:"bind_password"` + Idp string `mapstructure:"idp"` + GatewaySvc string `mapstructure:"gatewaysvc"` + Schema attributes `mapstructure:"schema"` + Nobody int64 `mapstructure:"nobody"` } type attributes struct { @@ -137,8 +135,7 @@ func (am *mgr) Configure(m map[string]interface{}) error { func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) (*user.User, map[string]*authpb.Scope, error) { log := appctx.GetLogger(ctx) - - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", am.c.Hostname, am.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&am.c.LDAPConn) if err != nil { return nil, nil, err } diff --git a/pkg/group/manager/ldap/ldap.go b/pkg/group/manager/ldap/ldap.go index 196d608ade..fd50b86d52 100644 --- a/pkg/group/manager/ldap/ldap.go +++ b/pkg/group/manager/ldap/ldap.go @@ -21,7 +21,6 @@ package ldap import ( "bytes" "context" - "crypto/tls" "fmt" "strconv" "strings" @@ -34,6 +33,7 @@ import ( "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/group" "github.com/cs3org/reva/pkg/group/manager/registry" + "github.com/cs3org/reva/pkg/utils" "github.com/go-ldap/ldap/v3" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -50,8 +50,7 @@ type manager struct { } type config struct { - Hostname string `mapstructure:"hostname"` - Port int `mapstructure:"port"` + utils.LDAPConn `mapstructure:",squash"` BaseDN string `mapstructure:"base_dn"` GroupFilter string `mapstructure:"groupfilter"` MemberFilter string `mapstructure:"memberfilter"` @@ -134,7 +133,7 @@ func New(m map[string]interface{}) (group.Manager, error) { func (m *manager) GetGroup(ctx context.Context, gid *grouppb.GroupId) (*grouppb.Group, error) { log := appctx.GetLogger(ctx) - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -211,7 +210,7 @@ func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string) (*gr } log := appctx.GetLogger(ctx) - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -269,7 +268,7 @@ func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string) (*gr } func (m *manager) FindGroups(ctx context.Context, query string) ([]*grouppb.Group, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -321,7 +320,7 @@ func (m *manager) FindGroups(ctx context.Context, query string) ([]*grouppb.Grou } func (m *manager) GetMembers(ctx context.Context, gid *grouppb.GroupId) ([]*userpb.UserId, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index c2dcde670f..cfe4a6b08b 100644 --- a/pkg/user/manager/ldap/ldap.go +++ b/pkg/user/manager/ldap/ldap.go @@ -21,7 +21,6 @@ package ldap import ( "bytes" "context" - "crypto/tls" "fmt" "strconv" "strings" @@ -33,6 +32,7 @@ import ( "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/user" "github.com/cs3org/reva/pkg/user/manager/registry" + "github.com/cs3org/reva/pkg/utils" "github.com/go-ldap/ldap/v3" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -49,8 +49,7 @@ type manager struct { } type config struct { - Hostname string `mapstructure:"hostname"` - Port int `mapstructure:"port"` + utils.LDAPConn `mapstructure:",squash"` BaseDN string `mapstructure:"base_dn"` UserFilter string `mapstructure:"userfilter"` AttributeFilter string `mapstructure:"attributefilter"` @@ -146,7 +145,7 @@ func (m *manager) Configure(ml map[string]interface{}) error { func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) { log := appctx.GetLogger(ctx) - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -234,7 +233,7 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use } log := appctx.GetLogger(ctx) - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -306,7 +305,7 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use } func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return nil, err } @@ -376,7 +375,7 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, } func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]string, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) + l, err := utils.GetLDAPConnection(&m.c.LDAPConn) if err != nil { return []string{}, err } diff --git a/pkg/utils/ldap.go b/pkg/utils/ldap.go new file mode 100644 index 0000000000..7487b2c2c3 --- /dev/null +++ b/pkg/utils/ldap.go @@ -0,0 +1,56 @@ +// Copyright 2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package utils + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "io/ioutil" + + "github.com/go-ldap/ldap/v3" + "github.com/pkg/errors" +) + +// LDAPConn holds the basic parameter for setting up an +// LDAP connection. +type LDAPConn struct { + Hostname string `mapstructure:"hostname"` + Port int `mapstructure:"port"` + Insecure bool `mapstructure:"insecure"` + CACert string `mapstructure:"cacert"` +} + +// GetLDAPConnection initializes an LDAPS connection and allows +// to set TLS options e.g. to add trusted Certificates or disable +// Certificate verification +func GetLDAPConnection(c *LDAPConn) (*ldap.Conn, error) { + tlsconfig := &tls.Config{InsecureSkipVerify: c.Insecure} + + if !c.Insecure && c.CACert != "" { + if pemBytes, err := ioutil.ReadFile(c.CACert); err == nil { + rpool, _ := x509.SystemCertPool() + rpool.AppendCertsFromPEM(pemBytes) + tlsconfig.RootCAs = rpool + } else { + return nil, errors.Wrapf(err, "Error reading LDAP CA Cert '%s.'", c.CACert) + } + } + return ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", c.Hostname, c.Port), tlsconfig) +} diff --git a/tests/oc-integration-tests/drone/ldap-users.toml b/tests/oc-integration-tests/drone/ldap-users.toml index cce99f82c3..7bb96e0c9a 100644 --- a/tests/oc-integration-tests/drone/ldap-users.toml +++ b/tests/oc-integration-tests/drone/ldap-users.toml @@ -13,6 +13,7 @@ auth_manager = "ldap" [grpc.services.authprovider.auth_managers.ldap] hostname="ldap" port=636 +insecure=true base_dn="dc=owncloud,dc=com" loginfilter="(&(objectclass=posixAccount)(|(cn={{login}}))(uid={{login}}))" bind_username="cn=admin,dc=owncloud,dc=com" @@ -30,6 +31,7 @@ driver = "ldap" [grpc.services.userprovider.drivers.ldap] hostname="ldap" port=636 +insecure=true base_dn="dc=owncloud,dc=com" userfilter="(&(objectclass=posixAccount)(|(uid={{.OpaqueId}})(cn={{.OpaqueId}})))" findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))" @@ -51,6 +53,7 @@ driver = "ldap" [grpc.services.groupprovider.drivers.ldap] hostname="ldap" port=636 +insecure=true base_dn="dc=owncloud,dc=com" groupfilter="(&(objectclass=posixGroup)(|(gid={{.OpaqueId}})(cn={{.OpaqueId}})))" findfilter="(&(objectclass=posixGroup)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))" diff --git a/tests/oc-integration-tests/local/ldap-users.toml b/tests/oc-integration-tests/local/ldap-users.toml index 3124ba21c7..d068a0eaca 100644 --- a/tests/oc-integration-tests/local/ldap-users.toml +++ b/tests/oc-integration-tests/local/ldap-users.toml @@ -13,6 +13,7 @@ auth_manager = "ldap" [grpc.services.authprovider.auth_managers.ldap] hostname="localhost" port=636 +insecure=true base_dn="dc=owncloud,dc=com" loginfilter="(&(objectclass=posixAccount)(|(cn={{login}}))(uid={{login}}))" bind_username="cn=admin,dc=owncloud,dc=com" @@ -30,6 +31,7 @@ driver = "ldap" [grpc.services.userprovider.drivers.ldap] hostname="localhost" port=636 +insecure=true base_dn="dc=owncloud,dc=com" userfilter="(&(objectclass=posixAccount)(|(uid={{.OpaqueId}})(cn={{.OpaqueId}})))" findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))" @@ -51,6 +53,7 @@ driver = "ldap" [grpc.services.groupprovider.drivers.ldap] hostname="localhost" port=636 +insecure=true base_dn="dc=owncloud,dc=com" groupfilter="(&(objectclass=posixGroup)(|(gid={{.OpaqueId}})(cn={{.OpaqueId}})))" findfilter="(&(objectclass=posixGroup)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"