From 5450139ba1ed8b434b1a96245869d0dec90af98e Mon Sep 17 00:00:00 2001 From: Nicol Draghici Date: Fri, 12 Aug 2022 15:18:41 +0300 Subject: [PATCH] Get identity when using TLS certificates Signed-off-by: Nicol Draghici --- Makefile | 2 +- pkg/api/authz.go | 30 +++++-- pkg/api/controller_test.go | 140 +++++++++++++++++++++++++++++ test/scripts/gen_nameless_certs.sh | 45 ++++++++++ 4 files changed, 208 insertions(+), 9 deletions(-) create mode 100755 test/scripts/gen_nameless_certs.sh diff --git a/Makefile b/Makefile index b377ea4c..9b45002b 100644 --- a/Makefile +++ b/Makefile @@ -84,7 +84,7 @@ privileged-test: check-skopeo $(TESTDATA) $(NOTATION) go test -failfast -tags needprivileges,$(EXTENSIONS),containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-needprivileges.txt -covermode=atomic ./pkg/storage/... ./pkg/cli/... -run ^TestElevatedPrivileges $(TESTDATA): check-skopeo - $(shell mkdir -p ${TESTDATA}; cd ${TESTDATA}; ../scripts/gen_certs.sh; cd ${TOP_LEVEL}; skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:7 oci:${TESTDATA}/zot-test:0.0.1;skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:8 oci:${TESTDATA}/zot-cve-test:0.0.1) + $(shell mkdir -p ${TESTDATA}; cd ${TESTDATA}; mkdir -p noidentity; ../scripts/gen_certs.sh; cd ${TESTDATA}/noidentity; ../../scripts/gen_nameless_certs.sh; cd ${TOP_LEVEL}; skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:7 oci:${TESTDATA}/zot-test:0.0.1;skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:8 oci:${TESTDATA}/zot-cve-test:0.0.1) $(shell chmod -R a=rwx ${TESTDATA}) .PHONY: run-bench diff --git a/pkg/api/authz.go b/pkg/api/authz.go index 54563cb7..8bd5a24d 100644 --- a/pkg/api/authz.go +++ b/pkg/api/authz.go @@ -189,22 +189,36 @@ func AuthzHandler(ctlr *Controller) mux.MiddlewareFunc { acCtrlr := NewAccessController(ctlr.Config) // allow anonymous authz if no authn present and only default policies are present - var username string + var identity string var err error - /* To be implemented: verify client certs and get its username(subject DN) - if request.TLS.VerifiedChains != nil, then get subject DN - issue: https: //github.com/project-zot/zot/issues/614 */ - + // allow anonymous authz if no authn present and only default policies are present + identity = "" if isAuthnEnabled(ctlr.Config) && request.Header.Get("Authorization") != "" { - username, _, err = getUsernamePasswordBasicAuth(request) + identity, _, err = getUsernamePasswordBasicAuth(request) if err != nil { authFail(response, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) } } - ctx := acCtrlr.getContext(username, request) + if request.TLS != nil { + verifiedChains := request.TLS.VerifiedChains + // still no identity, get it from TLS certs + if identity == "" && verifiedChains != nil && + len(verifiedChains) > 0 && len(verifiedChains[0]) > 0 { + for _, cert := range request.TLS.PeerCertificates { + identity = cert.Subject.CommonName + } + // if we still don't have an identity + if identity == "" { + acCtrlr.Log.Info().Msg("couldn't get identity from TLS certificate") + authFail(response, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) + } + } + } + + ctx := acCtrlr.getContext(identity, request) // will return only repos on which client is authorized to read if request.RequestURI == fmt.Sprintf("%s%s", constants.RoutePrefix, constants.ExtCatalogPrefix) { @@ -236,7 +250,7 @@ func AuthzHandler(ctlr *Controller) mux.MiddlewareFunc { action = DELETE } - can := acCtrlr.can(username, action, resource) + can := acCtrlr.can(identity, action, resource) if !can { authzFail(response, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay) } else { diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 7e826c05..7c26da7d 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -971,6 +971,146 @@ func TestTLSWithBasicAuthAllowReadAccess(t *testing.T) { }) } +func TestMutualTLSAuthWithUserPermissions(t *testing.T) { + Convey("Make a new controller", t, func() { + caCert, err := ioutil.ReadFile(CACert) + So(err, ShouldBeNil) + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + htpasswdPath := test.MakeHtpasswdFile() + defer os.Remove(htpasswdPath) + + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + secureBaseURL := test.GetSecureBaseURL(port) + + resty.SetTLSClientConfig(&tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}) + defer func() { resty.SetTLSClientConfig(nil) }() + conf := config.New() + conf.HTTP.Port = port + + conf.HTTP.TLS = &config.TLSConfig{ + Cert: ServerCert, + Key: ServerKey, + CACert: CACert, + } + + conf.AccessControl = &config.AccessControlConfig{ + Repositories: config.Repositories{ + AuthorizationAllRepos: config.PolicyGroup{ + Policies: []config.Policy{ + { + Users: []string{"*"}, + Actions: []string{"read"}, + }, + }, + }, + }, + } + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + resp, err := resty.R().Get(baseURL) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + + repoPolicy := conf.AccessControl.Repositories[AuthorizationAllRepos] + + // setup TLS mutual auth + cert, err := tls.LoadX509KeyPair("../../test/data/client.cert", "../../test/data/client.key") + So(err, ShouldBeNil) + + resty.SetCertificates(cert) + defer func() { resty.SetCertificates(tls.Certificate{}) }() + + // with client certs but without creds, should succeed + resp, err = resty.R().Get(secureBaseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // with creds, should get expected status code + resp, _ = resty.R().Get(secureBaseURL) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + // without creds, writes should fail + resp, err = resty.R().Post(secureBaseURL + "/v2/repo/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusForbidden) + + // empty default authorization and give user the permission to create + repoPolicy.Policies[0].Actions = append(repoPolicy.Policies[0].Actions, "create") + conf.AccessControl.Repositories[AuthorizationAllRepos] = repoPolicy + resp, err = resty.R().Post(secureBaseURL + "/v2/repo/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + }) +} + +func TestMutualTLSAuthWithoutCN(t *testing.T) { + Convey("Make a new controller", t, func() { + caCert, err := ioutil.ReadFile("../../test/data/noidentity/ca.crt") + So(err, ShouldBeNil) + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + htpasswdPath := test.MakeHtpasswdFile() + defer os.Remove(htpasswdPath) + + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + secureBaseURL := test.GetSecureBaseURL(port) + + resty.SetTLSClientConfig(&tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}) + defer func() { resty.SetTLSClientConfig(nil) }() + conf := config.New() + conf.HTTP.Port = port + + conf.HTTP.TLS = &config.TLSConfig{ + Cert: "../../test/data/noidentity/server.cert", + Key: "../../test/data/noidentity/server.key", + CACert: "../../test/data/noidentity/ca.crt", + } + + conf.AccessControl = &config.AccessControlConfig{ + Repositories: config.Repositories{ + AuthorizationAllRepos: config.PolicyGroup{ + Policies: []config.Policy{ + { + Users: []string{"*"}, + Actions: []string{"read"}, + }, + }, + }, + }, + } + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + // setup TLS mutual auth + cert, err := tls.LoadX509KeyPair("../../test/data/noidentity/client.cert", "../../test/data/noidentity/client.key") + So(err, ShouldBeNil) + + resty.SetCertificates(cert) + defer func() { resty.SetCertificates(tls.Certificate{}) }() + + // with client certs but without TLS mutual auth setup should get certificate error + resp, _ := resty.R().Get(secureBaseURL + "/v2/_catalog") + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) +} + func TestTLSMutualAuth(t *testing.T) { Convey("Make a new controller", t, func() { caCert, err := ioutil.ReadFile(CACert) diff --git a/test/scripts/gen_nameless_certs.sh b/test/scripts/gen_nameless_certs.sh new file mode 100755 index 00000000..fd976724 --- /dev/null +++ b/test/scripts/gen_nameless_certs.sh @@ -0,0 +1,45 @@ +#!/bin/bash -xe + +openssl req \ + -newkey rsa:2048 \ + -nodes \ + -days 3650 \ + -x509 \ + -keyout ca.key \ + -out ca.crt \ + -subj "/CN=" + +openssl req \ + -newkey rsa:2048 \ + -nodes \ + -keyout server.key \ + -out server.csr \ + -subj "/OU=TestServer/CN=" + +openssl x509 \ + -req \ + -days 3650 \ + -sha256 \ + -in server.csr \ + -CA ca.crt \ + -CAkey ca.key \ + -CAcreateserial \ + -out server.cert \ + -extfile <(echo subjectAltName = IP:127.0.0.1) + +openssl req \ + -newkey rsa:2048 \ + -nodes \ + -keyout client.key \ + -out client.csr \ + -subj "/OU=TestClient/CN=" + +openssl x509 \ + -req \ + -days 3650 \ + -sha256 \ + -in client.csr \ + -CA ca.crt \ + -CAkey ca.key \ + -CAcreateserial \ + -out client.cert