From 8329c209db74b19dda34961aa4b806f8f0466ef8 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Mon, 2 Dec 2019 19:45:07 +0800 Subject: [PATCH] Support pinning to authproxy server's cert This commit add an attribute to configurations, whose value is the certificate of authproxy server. When this attribute is set Harbor will pin to this cert when connecting authproxy. This value will also be part of the response of systemInfo API. This commit will be cherrypicked to 1.10 and 1.9 branch. Signed-off-by: Daniel Jiang --- .../postgresql/0012_1.9.4_schema.up.sql | 2 + src/common/config/metadata/metadatalist.go | 1 + src/common/const.go | 1 + src/common/models/config.go | 1 + src/core/api/config_test.go | 18 +-- src/core/auth/authproxy/auth.go | 37 +++-- src/core/auth/authproxy/auth_test.go | 145 ++++++++++++++++-- src/core/config/config.go | 1 + src/core/config/config_test.go | 50 +++++- src/pkg/authproxy/http.go | 17 +- src/pkg/authproxy/http_test.go | 71 +++++++++ 11 files changed, 293 insertions(+), 51 deletions(-) create mode 100644 make/migrations/postgresql/0012_1.9.4_schema.up.sql diff --git a/make/migrations/postgresql/0012_1.9.4_schema.up.sql b/make/migrations/postgresql/0012_1.9.4_schema.up.sql new file mode 100644 index 000000000..ee922fb10 --- /dev/null +++ b/make/migrations/postgresql/0012_1.9.4_schema.up.sql @@ -0,0 +1,2 @@ +/* change the data type to text to accommodate larger data */ +ALTER TABLE properties ALTER COLUMN v TYPE text; \ No newline at end of file diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index 85a5dfa4e..a5177c788 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -139,6 +139,7 @@ var ( {Name: common.HTTPAuthProxyTokenReviewEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyVerifyCert, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, {Name: common.HTTPAuthProxySkipSearch, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, + {Name: common.HTTPAuthProxyServerCertificate, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyCaseSensitive, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, {Name: common.OIDCName, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, diff --git a/src/common/const.go b/src/common/const.go index 9ae956d11..1bc0e738a 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -106,6 +106,7 @@ const ( HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert" HTTPAuthProxySkipSearch = "http_authproxy_skip_search" HTTPAuthProxyCaseSensitive = "http_authproxy_case_sensitive" + HTTPAuthProxyServerCertificate = "http_authproxy_server_certificate" OIDCName = "oidc_name" OIDCEndpoint = "oidc_endpoint" OIDCCLientID = "oidc_client_id" diff --git a/src/common/models/config.go b/src/common/models/config.go index 7094613aa..87d5924eb 100644 --- a/src/common/models/config.go +++ b/src/common/models/config.go @@ -73,6 +73,7 @@ type HTTPAuthProxy struct { TokenReviewEndpoint string `json:"tokenreivew_endpoint"` VerifyCert bool `json:"verify_cert"` SkipSearch bool `json:"skip_search"` + ServerCertificate string `json:"server_certificate"` CaseSensitive bool `json:"case_sensitive"` } diff --git a/src/core/api/config_test.go b/src/core/api/config_test.go index d687ec7fc..42c764675 100644 --- a/src/core/api/config_test.go +++ b/src/core/api/config_test.go @@ -108,20 +108,8 @@ func TestPutConfigMaxLength(t *testing.T) { assert := assert.New(t) apiTest := newHarborAPI() - // length is 512,expected code: 200 + // length is 1059,expected code: 200 cfg := map[string]interface{}{ - common.LDAPGroupSearchFilter: "OSWvgTrQJuhiPRZt7eCReNku29vrtMBBD2cZt6jl7LQN4OZQcirqEhS2vCnkW8X1OAHMJxiO1LyEY26j" + - "YhBEiUFliPKDUt8Q9endowT3H60nJibEnCkSRVjix7QujXKRzlmvxcOK76v1oZAoWeHSwwtv7tZrOk16Jj5LTGYdLOnZd2LIgBniTKmceL" + - "VY5WOgcpmgQCfI5HLbzWsmAqmFfbsDbadirrEDiXYYfZQ0LnF8s6sD4H13eImgenAumXEsBRH43FT37AbNXNxzlaSs8IQYEdPLaMyKoXFb" + - "rfa0LPipwXnU7bl54IlWOTXwCwum0JGS4qBiMl6LwKUBle34ObZ9fTLh5dFOVE1GdzrGE0kQ7qUmYjMZafQbSXzV80zTc22aZt3RQa9Gxt" + - "Dn2VqtgcoKAiZHkEySStiwOJtZpwuplyy1jcM3DcN0R9b8IidYAWOsriqetUBThqb75XIZTXAaRWhHLw4ayROYiaw8dPuLRjeVKhdyznqq" + - "AKxQGyvm", - } - code200, _ := apiTest.PutConfig(*admin, cfg) - assert.Equal(200, code200, "the status code of modifying configurations with admin user should be 200") - - // length is 1059,expected code: 500 - cfg = map[string]interface{}{ common.LDAPGroupSearchFilter: "YU2YcM13JtSx5jtBiftTjfaEM9KZFQ0XA5fKQHU02E9Xe0aLYaSy7YBokrTA8oHFjSkWFSgWZJ6FEmTS" + "Vy5Ovsy5to2kWnFtbVNX3pzbeQpZeAqK3mEGnXdMkMSQu9WTq74s99GpwjEdA628pcZqLx6wCR0IvwryqIcNoRtqPlUcuRGODWA8ZXaC0d" + "Qs7cRUYSe8onHsM2c9JWuUS8Jv4E7KggfytrxeKAT0WGP5DBZsB7rHZKxoAppE3C0NueEeC4yV791PUOODJt9rc0RrcD6ORUIO5RriCwym" + @@ -134,6 +122,6 @@ func TestPutConfigMaxLength(t *testing.T) { "kkqQmtFREitKWl5njO8wLJw0XyeIVAej75NsGKKZWVjyaupaM9Bqn6NFrWjELFacLox6OCcRIDSDl3ntNN8tIzGOF7aXVCIqljJl0IL9Pz" + "NenmmubaNm48YjfkBk8MqOUSYJYaFkO1qCKbVdMg7yTqKEHgSUqEkoFPoJMH6GAozC", } - code500, _ := apiTest.PutConfig(*admin, cfg) - assert.Equal(500, code500, "the status code of modifying configurations with admin user should be 500") + sc, _ := apiTest.PutConfig(*admin, cfg) + assert.Equal(200, sc, "the status code of modifying configurations with admin user should be 200") } diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index b6be87304..a71ab3d32 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -16,9 +16,11 @@ package authproxy import ( "crypto/tls" + "crypto/x509" "encoding/json" "errors" "fmt" + "github.com/goharbor/harbor/src/jobservice/logger" "io/ioutil" "net/http" "strings" @@ -39,13 +41,7 @@ import ( const refreshDuration = 2 * time.Second const userEntryComment = "By Authproxy" -var secureTransport = &http.Transport{ - Proxy: http.ProxyFromEnvironment, -} -var insecureTransport = &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, +var transport = &http.Transport{ Proxy: http.ProxyFromEnvironment, } @@ -56,7 +52,6 @@ type Auth struct { sync.Mutex Endpoint string TokenReviewEndpoint string - SkipCertVerify bool SkipSearch bool // When this attribute is set to false, the name of user/group will be converted to lower-case when onboarded to Harbor, so // as long as the authentication is successful there's no difference in terms of upper or lower case that is used. @@ -229,18 +224,30 @@ func (a *Auth) ensure() error { } a.Endpoint = setting.Endpoint a.TokenReviewEndpoint = setting.TokenReviewEndpoint - a.SkipCertVerify = !setting.VerifyCert a.SkipSearch = setting.SkipSearch + tlsCfg, err := getTLSConfig(setting) + if err != nil { + return err + } + transport.TLSClientConfig = tlsCfg + a.client.Transport = transport } - if a.SkipCertVerify { - a.client.Transport = insecureTransport - } else { - a.client.Transport = secureTransport - } - return nil } +func getTLSConfig(setting *models.HTTPAuthProxy) (*tls.Config, error) { + c := setting.ServerCertificate + if setting.VerifyCert && len(c) > 0 { + certs := x509.NewCertPool() + if !certs.AppendCertsFromPEM([]byte(c)) { + logger.Errorf("Failed to pin server certificate, please double check if it's valid, certificate: %s", c) + return nil, fmt.Errorf("failed to pin server certificate for authproxy") + } + return &tls.Config{RootCAs: certs}, nil + } + return &tls.Config{InsecureSkipVerify: !setting.VerifyCert}, nil +} + func (a *Auth) normalizeName(n string) string { if !a.CaseSensitive { return strings.ToLower(n) diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 126c2c68d..4edfb7ca4 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -15,11 +15,6 @@ package authproxy import ( - "net/http/httptest" - "os" - "testing" - "time" - "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/dao/group" @@ -29,6 +24,9 @@ import ( "github.com/goharbor/harbor/src/core/auth/authproxy/test" "github.com/goharbor/harbor/src/core/config" "github.com/stretchr/testify/assert" + "net/http/httptest" + "os" + "testing" ) var mockSvr *httptest.Server @@ -46,17 +44,14 @@ func TestMain(m *testing.M) { a = &Auth{ Endpoint: mockSvr.URL + "/test/login", TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview", - SkipCertVerify: true, CaseSensitive: false, - // So it won't require mocking the cfgManager - settingTimeStamp: time.Now(), } cfgMap := cut.GetUnitTestConfig() conf := map[string]interface{}{ common.HTTPAuthProxyEndpoint: a.Endpoint, common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint, - common.HTTPAuthProxyVerifyCert: !a.SkipCertVerify, common.HTTPAuthProxyCaseSensitive: a.CaseSensitive, + common.HTTPAuthProxyVerifyCert: false, common.PostGreSQLSSLMode: cfgMap[common.PostGreSQLSSLMode], common.PostGreSQLUsername: cfgMap[common.PostGreSQLUsername], common.PostGreSQLPort: cfgMap[common.PostGreSQLPort], @@ -213,3 +208,135 @@ func TestAuth_OnBoardGroup(t *testing.T) { t.Fatal("Empty user group should failed to OnBoard") } } + +func TestGetTLSConfig(t *testing.T) { + type result struct { + hasError bool + insecure bool + nilRootCA bool + } + cases := []struct { + input *models.HTTPAuthProxy + expect result + }{ + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: false, + SkipSearch: false, + ServerCertificate: "", + CaseSensitive: false, + }, + expect: result{ + hasError: false, + insecure: true, + nilRootCA: true, + }, + }, + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: false, + SkipSearch: false, + ServerCertificate: "This does not look like a cert", + CaseSensitive: false, + }, + expect: result{ + hasError: false, + insecure: true, + nilRootCA: true, + }, + }, + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: true, + SkipSearch: false, + ServerCertificate: "This does not look like a cert", + CaseSensitive: false, + }, + expect: result{ + hasError: true, + }, + }, + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: true, + SkipSearch: false, + ServerCertificate: "", + CaseSensitive: false, + }, + expect: result{ + hasError: false, + insecure: false, + nilRootCA: true, + }, + }, + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: true, + SkipSearch: false, + ServerCertificate: `-----BEGIN CERTIFICATE----- +MIIFXzCCA0egAwIBAgIUY7f2ECRISPMeb1iVNvV5iQsIErUwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCQ04xDDAKBgNVBAgMA1BFSzERMA8GA1UEBwwIQmVpIEpp +bmcxDzANBgNVBAoMBlZNd2FyZTERMA8GA1UEAwwISGFyYm9yQ0EwHhcNMTkxMTE2 +MjI1NjQ0WhcNMjAxMTE1MjI1NjQ0WjBdMQswCQYDVQQGEwJDTjEMMAoGA1UECAwD +UEVLMREwDwYDVQQHDAhCZWkgSmluZzEPMA0GA1UECgwGVk13YXJlMRwwGgYDVQQD +DBNqdC1kZXYuaGFyYm9yLmxvY2FsMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEAwQ/TQTHwnHwEC/KyHP8Tyv/v35GwXRGW6s1MYoqVnyQMPud0scLHAA2u +PZv2F5jy7PtnhcR0ZHGf05L/igY1utV7/4F2aFgOq0ExYMKxvzilitdcvsxmfTLI +m2pwS8+kH/1s1xR9/7ZPlPSdHuxcHgjtMqorljJykRyq0RBLvXCG+fmAY91kdLil +XWiuIU73lNpZHuXEDl4m2XUzb9cuhwvaHYs7aT6BhwqAJZUjwURUqMe1PIOo7vkQ +cKUHe3u3Fg/vbxfecEr3AHcKfIqm5fwI9vdzj5BP3lGT9hrxduAwI6SgehxGGWP4 +aN/cKGIKt/2kzgFoQi/d5p3RBkLVNP/sEyAt9dLJj12ovkQwJzdKDVOy50t3ws9g +Mf3rUUb/wdZADK26lxolep9EXVe4kuWpOo1RvdI+lJJvWc3QaJIoVbr9LM8QN3e7 +Iyk3pYRyaQj9EKZ4k0RgWVbIZfRLy1LkGMqmCcIqunHVdGDDjbO/ri8z0sKocMGl +qrqcBTPYmsau7PEcfzJY8k/HUDYdhZgIv2C1iLBl6eoTVDRbrGFcu8LzleWx2/19 +OA1Cx7S8WyzN+9mjygqwEYc6qMtoeutAkOA5J8JkxBp0yqjUEnAB6E7R07xQP8AY +IKq5oVpkbD8WRI3w7l/X0AAkDtnijbgYWTfPVGihXHhRtkr/b9cCAwEAAaMiMCAw +HgYDVR0RBBcwFYITanQtZGV2LmhhcmJvci5sb2NhbDANBgkqhkiG9w0BAQsFAAOC +AgEAqwU10WwhI5W8w62vOpT+PKSXRVjHKhm3ltaIzAN7S772hiGl6L81cP9dXZ5y +FN0tFUtUVK01JRJHJaduXNwx24HlwRPNp7mLa4IPpeeVfG14/QCoOd8vxHtKG+V6 +zE7Jx2FBVfUJ7P4SngEv4QfvZPt+lCXGK3V1RRTpkLD2knhBfu85rjPi+VW56Z7b +Jb2IEmVRlfR7Z0oYm8z3Obt2XuLIC1/8NtfxthggKr6DeSwZSJXrdGVbyvdiAmk2 +iHQch0+UTkRDinL0je6WWbxBoAPXsWA9Hc69o8kmjcXUa99/i8FrC2QxPDUoxxMn +1zWk0jct2Tsr3VZ5HnaI5e8ifG7RUcE5Vr6w7MI5P44Q88zhboP1ShYQ/s513cu2 +heELKvO3+mqv96lERtkUUwe8tm1zoPKzQI6ecGuqaTcMbXAGax+ud5XnUlz4xzTI +cByAsQ9DNhYIcOftnfz349zkHeWmMum4uiQwfp/+OrqX+O8U0eJYhlfu9vqCU05T +3mE8Hw5veNdLaZx+mzUVIDzrOB3fh/O62J9CsaZKtxwgLlGiT2ltuC1xUqn3DL8s +pkgODrJUf0p5dhcnLyA2nZolRV1rtwlgJstnEV4JpG1MwtmAZYZUilLvnfpVxTtA +y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= +-----END CERTIFICATE----- +`, + CaseSensitive: false, + }, + expect: result{ + hasError: false, + insecure: false, + nilRootCA: false, + }, + }, + } + + for _, c := range cases { + output, err := getTLSConfig(c.input) + if c.expect.hasError { + assert.NotNil(t, err) + continue + } else { + assert.Nil(t, err) + } + if output != nil { + assert.Equal(t, c.expect.insecure, output.InsecureSkipVerify) + assert.Equal(t, c.expect.nilRootCA, output.RootCAs == nil) + } + } + +} diff --git a/src/core/config/config.go b/src/core/config/config.go index fe0e2b7a4..6c725d980 100755 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -475,6 +475,7 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) { TokenReviewEndpoint: cfgMgr.Get(common.HTTPAuthProxyTokenReviewEndpoint).GetString(), VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(), SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(), + ServerCertificate: cfgMgr.Get(common.HTTPAuthProxyServerCertificate).GetString(), CaseSensitive: cfgMgr.Get(common.HTTPAuthProxyCaseSensitive).GetBool(), }, nil } diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index b4e5f4bd3..33260069c 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -216,20 +216,54 @@ func currPath() string { } func TestHTTPAuthProxySetting(t *testing.T) { + certificate := `-----BEGIN CERTIFICATE----- +MIIFXzCCA0egAwIBAgIUY7f2ECRISPMeb1iVNvV5iQsIErUwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCQ04xDDAKBgNVBAgMA1BFSzERMA8GA1UEBwwIQmVpIEpp +bmcxDzANBgNVBAoMBlZNd2FyZTERMA8GA1UEAwwISGFyYm9yQ0EwHhcNMTkxMTE2 +MjI1NjQ0WhcNMjAxMTE1MjI1NjQ0WjBdMQswCQYDVQQGEwJDTjEMMAoGA1UECAwD +UEVLMREwDwYDVQQHDAhCZWkgSmluZzEPMA0GA1UECgwGVk13YXJlMRwwGgYDVQQD +DBNqdC1kZXYuaGFyYm9yLmxvY2FsMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEAwQ/TQTHwnHwEC/KyHP8Tyv/v35GwXRGW6s1MYoqVnyQMPud0scLHAA2u +PZv2F5jy7PtnhcR0ZHGf05L/igY1utV7/4F2aFgOq0ExYMKxvzilitdcvsxmfTLI +m2pwS8+kH/1s1xR9/7ZPlPSdHuxcHgjtMqorljJykRyq0RBLvXCG+fmAY91kdLil +XWiuIU73lNpZHuXEDl4m2XUzb9cuhwvaHYs7aT6BhwqAJZUjwURUqMe1PIOo7vkQ +cKUHe3u3Fg/vbxfecEr3AHcKfIqm5fwI9vdzj5BP3lGT9hrxduAwI6SgehxGGWP4 +aN/cKGIKt/2kzgFoQi/d5p3RBkLVNP/sEyAt9dLJj12ovkQwJzdKDVOy50t3ws9g +Mf3rUUb/wdZADK26lxolep9EXVe4kuWpOo1RvdI+lJJvWc3QaJIoVbr9LM8QN3e7 +Iyk3pYRyaQj9EKZ4k0RgWVbIZfRLy1LkGMqmCcIqunHVdGDDjbO/ri8z0sKocMGl +qrqcBTPYmsau7PEcfzJY8k/HUDYdhZgIv2C1iLBl6eoTVDRbrGFcu8LzleWx2/19 +OA1Cx7S8WyzN+9mjygqwEYc6qMtoeutAkOA5J8JkxBp0yqjUEnAB6E7R07xQP8AY +IKq5oVpkbD8WRI3w7l/X0AAkDtnijbgYWTfPVGihXHhRtkr/b9cCAwEAAaMiMCAw +HgYDVR0RBBcwFYITanQtZGV2LmhhcmJvci5sb2NhbDANBgkqhkiG9w0BAQsFAAOC +AgEAqwU10WwhI5W8w62vOpT+PKSXRVjHKhm3ltaIzAN7S772hiGl6L81cP9dXZ5y +FN0tFUtUVK01JRJHJaduXNwx24HlwRPNp7mLa4IPpeeVfG14/QCoOd8vxHtKG+V6 +zE7Jx2FBVfUJ7P4SngEv4QfvZPt+lCXGK3V1RRTpkLD2knhBfu85rjPi+VW56Z7b +Jb2IEmVRlfR7Z0oYm8z3Obt2XuLIC1/8NtfxthggKr6DeSwZSJXrdGVbyvdiAmk2 +iHQch0+UTkRDinL0je6WWbxBoAPXsWA9Hc69o8kmjcXUa99/i8FrC2QxPDUoxxMn +1zWk0jct2Tsr3VZ5HnaI5e8ifG7RUcE5Vr6w7MI5P44Q88zhboP1ShYQ/s513cu2 +heELKvO3+mqv96lERtkUUwe8tm1zoPKzQI6ecGuqaTcMbXAGax+ud5XnUlz4xzTI +cByAsQ9DNhYIcOftnfz349zkHeWmMum4uiQwfp/+OrqX+O8U0eJYhlfu9vqCU05T +3mE8Hw5veNdLaZx+mzUVIDzrOB3fh/O62J9CsaZKtxwgLlGiT2ltuC1xUqn3DL8s +pkgODrJUf0p5dhcnLyA2nZolRV1rtwlgJstnEV4JpG1MwtmAZYZUilLvnfpVxTtA +y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= +-----END CERTIFICATE----- +` m := map[string]interface{}{ - common.HTTPAuthProxySkipSearch: "true", - common.HTTPAuthProxyVerifyCert: "true", - common.HTTPAuthProxyCaseSensitive: "false", - common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", + common.HTTPAuthProxySkipSearch: "true", + common.HTTPAuthProxyVerifyCert: "true", + common.HTTPAuthProxyCaseSensitive: "false", + common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", + common.HTTPAuthProxyServerCertificate: certificate, } InitWithSettings(m) v, e := HTTPAuthProxySetting() assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ - Endpoint: "https://auth.proxy/suffix", - SkipSearch: true, - VerifyCert: true, - CaseSensitive: false, + Endpoint: "https://auth.proxy/suffix", + SkipSearch: true, + VerifyCert: true, + CaseSensitive: false, + ServerCertificate: certificate, }) } diff --git a/src/pkg/authproxy/http.go b/src/pkg/authproxy/http.go index 61356fd3f..8dce618b6 100644 --- a/src/pkg/authproxy/http.go +++ b/src/pkg/authproxy/http.go @@ -26,10 +26,8 @@ func TokenReview(rawToken string, authProxyConfig *models.HTTPAuthProxy) (k8s_ap GroupVersion: &schema.GroupVersion{}, NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, }, - BearerToken: rawToken, - TLSClientConfig: rest.TLSClientConfig{ - Insecure: !authProxyConfig.VerifyCert, - }, + BearerToken: rawToken, + TLSClientConfig: getTLSConfig(authProxyConfig), } authClient, err := rest.RESTClientFor(authClientCfg) if err != nil { @@ -68,6 +66,17 @@ func TokenReview(rawToken string, authProxyConfig *models.HTTPAuthProxy) (k8s_ap } +func getTLSConfig(config *models.HTTPAuthProxy) rest.TLSClientConfig { + if config.VerifyCert && len(config.ServerCertificate) > 0 { + return rest.TLSClientConfig{ + CAData: []byte(config.ServerCertificate), + } + } + return rest.TLSClientConfig{ + Insecure: !config.VerifyCert, + } +} + // UserFromReviewStatus transform a review status to a user model. // Group entries will be populated if needed. func UserFromReviewStatus(status k8s_api_v1beta1.TokenReviewStatus) (*models.User, error) { diff --git a/src/pkg/authproxy/http_test.go b/src/pkg/authproxy/http_test.go index 24374043e..f2d53ee1f 100644 --- a/src/pkg/authproxy/http_test.go +++ b/src/pkg/authproxy/http_test.go @@ -3,8 +3,10 @@ package authproxy import ( "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/dao/group" + "github.com/goharbor/harbor/src/common/models" "github.com/stretchr/testify/assert" "k8s.io/api/authentication/v1beta1" + "k8s.io/client-go/rest" "os" "testing" ) @@ -85,3 +87,72 @@ func TestUserFromReviewStatus(t *testing.T) { } } } + +func TestGetTLSConfig(t *testing.T) { + certificate := `-----BEGIN CERTIFICATE----- +MIIFXzCCA0egAwIBAgIUY7f2ECRISPMeb1iVNvV5iQsIErUwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCQ04xDDAKBgNVBAgMA1BFSzERMA8GA1UEBwwIQmVpIEpp +bmcxDzANBgNVBAoMBlZNd2FyZTERMA8GA1UEAwwISGFyYm9yQ0EwHhcNMTkxMTE2 +MjI1NjQ0WhcNMjAxMTE1MjI1NjQ0WjBdMQswCQYDVQQGEwJDTjEMMAoGA1UECAwD +UEVLMREwDwYDVQQHDAhCZWkgSmluZzEPMA0GA1UECgwGVk13YXJlMRwwGgYDVQQD +DBNqdC1kZXYuaGFyYm9yLmxvY2FsMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEAwQ/TQTHwnHwEC/KyHP8Tyv/v35GwXRGW6s1MYoqVnyQMPud0scLHAA2u +PZv2F5jy7PtnhcR0ZHGf05L/igY1utV7/4F2aFgOq0ExYMKxvzilitdcvsxmfTLI +m2pwS8+kH/1s1xR9/7ZPlPSdHuxcHgjtMqorljJykRyq0RBLvXCG+fmAY91kdLil +XWiuIU73lNpZHuXEDl4m2XUzb9cuhwvaHYs7aT6BhwqAJZUjwURUqMe1PIOo7vkQ +cKUHe3u3Fg/vbxfecEr3AHcKfIqm5fwI9vdzj5BP3lGT9hrxduAwI6SgehxGGWP4 +aN/cKGIKt/2kzgFoQi/d5p3RBkLVNP/sEyAt9dLJj12ovkQwJzdKDVOy50t3ws9g +Mf3rUUb/wdZADK26lxolep9EXVe4kuWpOo1RvdI+lJJvWc3QaJIoVbr9LM8QN3e7 +Iyk3pYRyaQj9EKZ4k0RgWVbIZfRLy1LkGMqmCcIqunHVdGDDjbO/ri8z0sKocMGl +qrqcBTPYmsau7PEcfzJY8k/HUDYdhZgIv2C1iLBl6eoTVDRbrGFcu8LzleWx2/19 +OA1Cx7S8WyzN+9mjygqwEYc6qMtoeutAkOA5J8JkxBp0yqjUEnAB6E7R07xQP8AY +IKq5oVpkbD8WRI3w7l/X0AAkDtnijbgYWTfPVGihXHhRtkr/b9cCAwEAAaMiMCAw +HgYDVR0RBBcwFYITanQtZGV2LmhhcmJvci5sb2NhbDANBgkqhkiG9w0BAQsFAAOC +AgEAqwU10WwhI5W8w62vOpT+PKSXRVjHKhm3ltaIzAN7S772hiGl6L81cP9dXZ5y +FN0tFUtUVK01JRJHJaduXNwx24HlwRPNp7mLa4IPpeeVfG14/QCoOd8vxHtKG+V6 +zE7Jx2FBVfUJ7P4SngEv4QfvZPt+lCXGK3V1RRTpkLD2knhBfu85rjPi+VW56Z7b +Jb2IEmVRlfR7Z0oYm8z3Obt2XuLIC1/8NtfxthggKr6DeSwZSJXrdGVbyvdiAmk2 +iHQch0+UTkRDinL0je6WWbxBoAPXsWA9Hc69o8kmjcXUa99/i8FrC2QxPDUoxxMn +1zWk0jct2Tsr3VZ5HnaI5e8ifG7RUcE5Vr6w7MI5P44Q88zhboP1ShYQ/s513cu2 +heELKvO3+mqv96lERtkUUwe8tm1zoPKzQI6ecGuqaTcMbXAGax+ud5XnUlz4xzTI +cByAsQ9DNhYIcOftnfz349zkHeWmMum4uiQwfp/+OrqX+O8U0eJYhlfu9vqCU05T +3mE8Hw5veNdLaZx+mzUVIDzrOB3fh/O62J9CsaZKtxwgLlGiT2ltuC1xUqn3DL8s +pkgODrJUf0p5dhcnLyA2nZolRV1rtwlgJstnEV4JpG1MwtmAZYZUilLvnfpVxTtA +y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= +-----END CERTIFICATE----- +` + cases := []struct { + input *models.HTTPAuthProxy + expect rest.TLSClientConfig + }{ + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: false, + SkipSearch: false, + ServerCertificate: "", + CaseSensitive: false, + }, + expect: rest.TLSClientConfig{ + Insecure: true, + }, + }, + { + input: &models.HTTPAuthProxy{ + Endpoint: "https://127.0.0.1/login", + TokenReviewEndpoint: "https://127.0.0.1/tokenreview", + VerifyCert: true, + SkipSearch: false, + ServerCertificate: certificate, + }, + expect: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte(certificate), + }, + }, + } + for _, c := range cases { + assert.Equal(t, c.expect, getTLSConfig(c.input)) + } +}