diff --git a/controller/api/apis.go b/controller/api/apis.go index b953ece1b..6aa2ebbb1 100644 --- a/controller/api/apis.go +++ b/controller/api/apis.go @@ -243,7 +243,10 @@ type RESTFedAuthData struct { type RESTTokenRedirect struct { // The NeuVector URL to redirect after authentication/logout. Redirect string `json:"redirect_endpoint"` - // Optional: The redirect url is used as issuer by default. When isser is specified, it will be used instead of redirecturl. + // (Optional) + // When absent, the redirect url will be used as issuer in SAML request. + // When it is specified, the value here will be used as the issuer. + // This is for Single Logout where redirect url and issue can be different. Issuer string `json:"issuer"` } @@ -351,11 +354,11 @@ type RESTServerSAML struct { RoleGroups map[string][]string `json:"role_groups,omitempty"` // role -> groups GroupMappedRoles []*share.GroupRoleMapping `json:"group_mapped_roles,omitempty"` // group -> (role -> domains) - AuthnSigningEnabled bool `json:"authn_signing_enabled,omitempty"` - SigningCert string `json:"signing_cert,omitempty"` - SigningKey string `json:"signing_key,omitempty"` - SLOEnabled bool `json:"slo_enabled,omitempty"` - SLOURL string `json:"slo_url,omitempty"` + AuthnSigningEnabled bool `json:"authn_signing_enabled,omitempty"` // Optional. Enable signing AuthnRequest. Default off. + SigningCert string `json:"signing_cert,omitempty"` // Optional. + //SigningKey string `json:"signing_key,omitempty"` // Optional. + SLOEnabled bool `json:"slo_enabled,omitempty"` // Optional. + SLOURL string `json:"slo_url,omitempty"` // Optional. } type RESTServerOIDC struct { @@ -431,11 +434,11 @@ type RESTServerSAMLConfig struct { GroupMappedRoles *[]*share.GroupRoleMapping `json:"group_mapped_roles,omitempty"` // group -> (role -> domains) X509CertExtra *[]string `json:"x509_cert_extra,omitempty"` - AuthnSigningEnabled *bool `json:"authn_signing_enabled,omitempty"` // Optional. Enable signing AuthnRequest. + AuthnSigningEnabled *bool `json:"authn_signing_enabled,omitempty"` // Optional. Enable signing AuthnRequest. Default off. + SigningCert *string `json:"signing_cert,omitempty"` // Optional. + SigningKey *string `json:"signing_key,omitempty"` // Optional. SLOEnabled *bool `json:"slo_enabled,omitempty"` // Optional. SLOURL *string `json:"slo_url,omitempty"` // Optional. - SigningCert *string `json:"signing_cert,omitempty"` // Optional. Used as signing cert by default. - SigningKey *string `json:"signing_key,omitempty"` // Optional. Used as signing key by default. } type RESTServerSAMLConfigCfgMap struct { @@ -1059,14 +1062,14 @@ type RESTConversationEndpointConfigData struct { } type RESTConversationReportEntry struct { - Bytes uint64 `json:"bytes"` - Sessions uint32 `json:"sessions"` - Port string `json:"port,omitempty"` - Application string `json:"application,omitempty"` - PolicyAction string `json:"policy_action"` - CIP string `json:"client_ip,omitempty"` - SIP string `json:"server_ip,omitempty"` - FQDN string `json:"fqdn,omitempty"` + Bytes uint64 `json:"bytes"` + Sessions uint32 `json:"sessions"` + Port string `json:"port,omitempty"` + Application string `json:"application,omitempty"` + PolicyAction string `json:"policy_action"` + CIP string `json:"client_ip,omitempty"` + SIP string `json:"server_ip,omitempty"` + FQDN string `json:"fqdn,omitempty"` } type RESTConversationReport struct { diff --git a/controller/kv/config.go b/controller/kv/config.go index 647aa1890..6db083817 100644 --- a/controller/kv/config.go +++ b/controller/kv/config.go @@ -16,7 +16,7 @@ import ( "github.com/neuvector/neuvector/controller/access" "github.com/neuvector/neuvector/controller/api" "github.com/neuvector/neuvector/controller/common" - admission "github.com/neuvector/neuvector/controller/nvk8sapi/nvvalidatewebhookcfg" + "github.com/neuvector/neuvector/controller/nvk8sapi/nvvalidatewebhookcfg" "github.com/neuvector/neuvector/controller/resource" "github.com/neuvector/neuvector/share" "github.com/neuvector/neuvector/share/cluster" @@ -485,11 +485,11 @@ func (c *configHelper) sections2Endpoints(sections []string) []*cfgEndpoint { return eps } -// 1. When import, cluster name is always replaced with the cluster name(if available) specified in the backup file -// 2. When import, fed rules are always replaced with the fed rules specified in the backup file. -// 3. For clusters in fed, Import() doesn't change the existing clusters membership. -// 4. For stand-alone cluster, we allow it to promote to master cluster by importing a master cluster's backup file. -// However, joined clusters list is not imported. Customer needs to manually trigger join-fed operation. +// 1. When import, cluster name is always replaced with the cluster name(if available) specified in the backup file +// 2. When import, fed rules are always replaced with the fed rules specified in the backup file. +// 3. For clusters in fed, Import() doesn't change the existing clusters membership. +// 4. For stand-alone cluster, we allow it to promote to master cluster by importing a master cluster's backup file. +// However, joined clusters list is not imported. Customer needs to manually trigger join-fed operation. func (c *configHelper) Import(rpcEps []*common.RPCEndpoint, localCtrlerID, localCtrlerIP string, loginDomainRoles access.DomainRole, importTask share.CLUSImportTask, tempToken string, revertFedRoles RevertFedRolesFunc, postImportOp PostImportFunc, pauseResumeStoreWatcher PauseResumeStoreWatcherFunc, ignoreFed bool) error { log.Debug() diff --git a/controller/rest/auth.go b/controller/rest/auth.go index de58e2789..79d936205 100644 --- a/controller/rest/auth.go +++ b/controller/rest/auth.go @@ -2609,7 +2609,7 @@ func handlerAuthLogout(w http.ResponseWriter, r *http.Request, ps httprouter.Par fedRole, err := cacher.GetFedMembershipRole(acc) if err == nil && fedRole == api.FedRoleMaster { ids := cacher.GetFedJoinedClusterIdMap(acc) - for id, _ := range ids { + for id := range ids { joinedCluster := cacher.GetFedJoinedCluster(id, acc) if joinedCluster.ID != "" { var token string diff --git a/controller/rest/rest.go b/controller/rest/rest.go index 4b9baac00..d8fdf7bdb 100644 --- a/controller/rest/rest.go +++ b/controller/rest/rest.go @@ -1446,10 +1446,10 @@ func StartRESTServer() { //r.POST("/v1/password_profile", handlerPwdProfileCreate) r.PATCH("/v1/password_profile/:name", handlerPwdProfileConfig) //r.DELETE("/v1/password_profile/:name", handlerPwdProfileDelete) - r.GET("/v1/token_auth_server", handlerTokenAuthServerList) // Skip API document - r.GET("/v1/token_auth_server/:server", handlerTokenAuthServerRequest) // Skip API document - r.POST("/v1/token_auth_server/:server", handlerTokenAuthServerRequest) // Skip API document - r.GET("/v1/token_auth_server/:server/slo", handlerGenerateSLORequest) // Skip API document + r.GET("/v1/token_auth_server", handlerTokenAuthServerList) // Skip API document + r.GET("/v1/token_auth_server/:server", handlerTokenAuthServerRequest) + r.POST("/v1/token_auth_server/:server", handlerTokenAuthServerRequest) + r.GET("/v1/token_auth_server/:server/slo", handlerGenerateSLORequest) r.GET("/v1/server", handlerServerList) r.GET("/v1/server/:name", handlerServerShow) r.GET("/v1/server/:name/user", handlerServerUserList) diff --git a/controller/rest/server.go b/controller/rest/server.go index 314c8e8ca..f94a8042a 100644 --- a/controller/rest/server.go +++ b/controller/rest/server.go @@ -6,7 +6,6 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" - "errors" "fmt" "io/ioutil" "net/http" @@ -14,6 +13,8 @@ import ( "sort" "strings" + "github.com/pkg/errors" + "github.com/julienschmidt/httprouter" log "github.com/sirupsen/logrus" @@ -451,6 +452,7 @@ func handlerGenerateSLORequest(w http.ResponseWriter, r *http.Request, ps httpro // Handle SAML SLO if login.nameid == "" { + log.Debug("This user has no nameid associated. Do not generate SAML SLO request.") restRespSuccess(w, r, &resp, acc, login, nil, "") return } @@ -462,6 +464,7 @@ func handlerGenerateSLORequest(w http.ResponseWriter, r *http.Request, ps httpro } if !cs.SAML.SLOEnabled { + log.Debug("SAML SLO is not enabled.") restRespSuccess(w, r, &resp, acc, login, nil, "") return } @@ -595,7 +598,7 @@ func validateAuthServer(cas *share.CLUSServerAuth) error { if !access.IsValidRole(mappedRoles.GlobalRole, access.CONST_VISIBLE_USER_ROLE) { return fmt.Errorf("Invalid global role(%s) in group(%s) mapping", mappedRoles.GlobalRole, mappedRoles.Group) } - for role, _ := range mappedRoles.RoleDomains { + for role := range mappedRoles.RoleDomains { if !access.IsValidRole(role, access.CONST_VISIBLE_DOMAIN_ROLE) { return fmt.Errorf("Invalid domain role(%s) in group(%s) mapping", role, mappedRoles.Group) } @@ -745,7 +748,11 @@ func validateSAMLServer(cs *share.CLUSServer) error { } } - // TODO: Verify encryption key/certs + if len(csaml.SigningCert) > 0 || len(csaml.SigningKey) > 0 { + if _, err := tls.X509KeyPair([]byte(csaml.SigningCert), []byte(csaml.SigningKey)); err != nil { + return errors.Wrap(err, "invalid key cert pair") + } + } var certs []string certs = append(certs, csaml.X509Cert) // original one diff --git a/share/auth/auth.go b/share/auth/auth.go index 34fdf96e7..27c5e4e8f 100644 --- a/share/auth/auth.go +++ b/share/auth/auth.go @@ -144,30 +144,29 @@ func GenerateSamlSP(csaml *share.CLUSServerSAML, spissuer string, redirurl strin Roots: []*x509.Certificate{}, } - // TODO: fix error handling - parseAndStoreCert := func(x509cert string) { + parseAndStoreCert := func(x509cert string) error { var err error - defer func() { - if err != nil { - log.WithError(err).Warn("failed to decode cert.") - } - }() block, _ := pem.Decode([]byte(x509cert)) if block == nil { - err = errors.New("failed to decode pem block") + return errors.New("failed to decode pem block") } idpCert, err := x509.ParseCertificate(block.Bytes) if err != nil { - return + return err } certStore.Roots = append(certStore.Roots, idpCert) + return nil } - parseAndStoreCert(csaml.X509Cert) + if err := parseAndStoreCert(csaml.X509Cert); err != nil { + log.WithError(err).Error("failed to parse X509Cert. Skip this cert.") + } for _, cert := range csaml.X509CertExtra { - parseAndStoreCert(cert) + if err := parseAndStoreCert(cert); err != nil { + log.WithError(err).Error("failed to parse X509Cert. Skip this cert.") + } } if csaml.SigningCert != "" && csaml.SigningKey != "" { @@ -178,6 +177,7 @@ func GenerateSamlSP(csaml *share.CLUSServerSAML, spissuer string, redirurl strin keystore = dsig.TLSCertKeyStore(cert) } + // For unit-test var clockOverride *dsig.Clock if timeOverride != nil { clockOverride = dsig.NewFakeClock(clockwork.NewFakeClockAt(*timeOverride)) @@ -275,13 +275,8 @@ func (a *remoteAuth) SAMLSPGetLogoutURL(csaml *share.CLUSServerSAML, redir *api. } // Return Name ID, session index, and attributes. -// TODO: Check all certs func (a *remoteAuth) SAMLSPAuth(csaml *share.CLUSServerSAML, tokenData *api.RESTAuthToken) (string, string, map[string][]string, error) { - var certs []string - certs = append(certs, csaml.X509Cert) - certs = append(certs, csaml.X509CertExtra...) - - // For backward compatibility, use Authn response redirect url (AssertionConsumerServiceURL) as SP issuer. (https:///token_auth_server) + // Authn response redirect url (AssertionConsumerServiceURL) as SP issuer. (https:///token_auth_server) sp, err := GenerateSamlSP(csaml, tokenData.Redirect, tokenData.Redirect, a.fakeTime) if err != nil { return "", "", map[string][]string{}, err diff --git a/share/auth/auth_test.go b/share/auth/auth_test.go index ff0603d43..66305d226 100644 --- a/share/auth/auth_test.go +++ b/share/auth/auth_test.go @@ -151,12 +151,12 @@ ma7nkie3ORja96UTROAZ77o= }, }, }, - SSOURL: "https://dev.okta.com/app/dev/xxxxx/sso/saml", - Issuer: "https://example.com/token_auth_server", - X509Cert: string(idpCert), - SLOEnabled: false, - SLOSigningCert: string(cert), - SLOSigningKey: string(key), + SSOURL: "https://dev.okta.com/app/dev/xxxxx/sso/saml", + Issuer: "https://example.com/token_auth_server", + X509Cert: string(idpCert), + SLOEnabled: false, + SigningCert: string(cert), + SigningKey: string(key), }, &reqData, map[string]string{ "ID": "_ec47ec78-a7f4-458d-86ad-f8e5dc85eb8a", @@ -164,7 +164,7 @@ ma7nkie3ORja96UTROAZ77o= ) // Verify if it's consistent with Authn request for Okta. - assert.Equal(t, "https://dev.okta.com/app/dev/xxxxx/sso/saml?SAMLRequest=jJJBb9swDIX%2FisC7LcdNFkOoA2QNhgXotqDJdtglYGVmEWpJnkhl2b8f4rZAd1gwHSk%2Bfg98vGX0%2FWCWWY7hgX5mYlFn3wc240cLOQUTkR2bgJ7YiDXb5ad7U5eVGVKUaGMPbyTXFchMSVwMoNarFvZkp3Oy86bA%2BWFaTGdNVzTvsCsODc0628zosUFQ3yixi6GFuqxAbV6o713oXPhxHfj43MTm4263KTZftjtQy1cTdzFw9pS2lE7O0teH%2BxaOIgMbremMfuiptNFriU8U9pjluGdKJ0qg1syZ1oEFg7RQV%2FVNMamKarqrJ6ZqzE31HdSKWFxAGa2%2Fzu3oVMYnwXEwDsOloM%2BXp5mjvqwQFmMqZmSkxX85utVvJS%2BpfkZP69Um9s7%2BVsu%2Bj7%2FuEqFQC5IygfoQk0f59wIn5WSsuK44jK0mBx7IuoOjDvTiGfr39Sz%2BBAAA%2F%2F8%3D&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256&Signature=MMolrRAjT%2BaWEuh8erz%2B280%2B8pdgAs%2BCeJVEGgDXWo6cLDTjkTCgpP9SkzBpocW6IbjSG9eYcYB0SBTk47vW71Jv16Kq6eVQc6bNMYQVbiXwXEV6ZY0HjLaqeu%2FS0oiE6j6z9SJBSp2U22dYxBhSjrfK6uIorLFvG1mul7XTUTsa5WXQeEF90%2Bu3%2Bh8wJ3GONuWzr6rLOimVbJnqUHFQA8MJLQNWVlIlnv64Me%2FUU25zLSOZ6SKx7%2FxWsAYymoUIOt8%2Ft%2FfFhkulPbxZxVAoLwzjSiJAkLEFIwaRkiRWTZ6Xwi6h7jW37OGN4GjVE25S0fRxR1fMh88u3XRJ0Tr2zw%3D%3D", url) + assert.Equal(t, "https://dev.okta.com/app/dev/xxxxx/sso/saml?SAMLRequest=jJJBb9swDIX%2FisC7LcdNFkOoA2QNhgXotqDJdtglYGVmEWpJnkhl2b8f4rZAd1gwHSk%2Bfg98vGX0%2FWCWWY7hgX5mYlFn3wc240cLOQUTkR2bgJ7YiDXb5ad7U5eVGVKUaGMPbyTXFchMSVwMoNarFvZkp3Oy86bA%2BWFaTGdNVzTvsCsODc0628zosUFQ3yixi6GFuqxAbV6o713oXPhxHfj43MTm4263KTZftjtQy1cTdzFw9pS2lE7O0teH%2BxaOIgMbremMfuiptNFriU8U9pjluGdKJ0qg1syZ1oEFg7RQV%2FVNMamKarqrJ6ZqzE31HdSKWFxAGa2%2Fzu3oVMYnwXEwDsOloM%2BXp5mjvqwQFmMqZmSkxX85utVvJS%2BpfkZP69Um9s7%2BVsu%2Bj7%2FuEqFQC5IygfoQk0f59wIn5WSsuK44jK0mBx7IuoOjDvTiGfr39Sz%2BBAAA%2F%2F8%3D", url) } // Verify NV can accept Okta Authn response. @@ -305,12 +305,12 @@ ma7nkie3ORja96UTROAZ77o= }, SSOURL: "https://dev.okta.com/app/dev/xxxxx/sso/saml", // TODO: Make sure issuer and redirect don't conflict. - Issuer: "https://example.com/token_auth_server", - X509Cert: string(idpCert), - SLOEnabled: true, - SLOURL: "https://dev.okta.com/app/dev/xxxxx/slo/saml", - SLOSigningCert: string(cert), - SLOSigningKey: string(key), + Issuer: "https://example.com/token_auth_server", + X509Cert: string(idpCert), + SLOEnabled: true, + SLOURL: "https://dev.okta.com/app/dev/xxxxx/slo/saml", + SigningCert: string(cert), + SigningKey: string(key), }, &reqData, "aa@bb.cc", "_6d713693-660a-4740-b9e0-c1ac2321fabe",