From ea16ec55cb848e7cae2b0efc4e65be65fda055c5 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Thu, 16 Jan 2025 12:42:51 -0300 Subject: [PATCH 1/7] Move /auth/export code to own file --- lib/web/apiserver.go | 30 ------- lib/web/apiserver_test.go | 148 ------------------------------- lib/web/ca_export.go | 58 +++++++++++++ lib/web/ca_export_test.go | 178 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 178 deletions(-) create mode 100644 lib/web/ca_export.go create mode 100644 lib/web/ca_export_test.go diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 91491831a6fb1..d541467bf0582 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -5206,36 +5206,6 @@ func SSOSetWebSessionAndRedirectURL(w http.ResponseWriter, r *http.Request, resp return nil } -// authExportPublic returns the CA Certs that can be used to set up a chain of trust which includes the current Teleport Cluster -// -// GET /webapi/sites/:site/auth/export?type= -// GET /webapi/auth/export?type= -func (h *Handler) authExportPublic(w http.ResponseWriter, r *http.Request, p httprouter.Params) { - err := rateLimitRequest(r, h.limiter) - if err != nil { - http.Error(w, err.Error(), trace.ErrorToCode(err)) - return - } - authorities, err := client.ExportAuthorities( - r.Context(), - h.GetProxyClient(), - client.ExportAuthoritiesRequest{ - AuthType: r.URL.Query().Get("type"), - }, - ) - if err != nil { - h.logger.DebugContext(r.Context(), "Failed to generate CA Certs", "error", err) - http.Error(w, err.Error(), trace.ErrorToCode(err)) - return - } - - reader := strings.NewReader(authorities) - - // ServeContent sets the correct headers: Content-Type, Content-Length and Accept-Ranges. - // It also handles the Range negotiation - http.ServeContent(w, r, "authorized_hosts.txt", time.Now(), reader) -} - const robots = `User-agent: * Disallow: /` diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 58c0d1132f3d2..245e498b9afdb 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -30,7 +30,6 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" - "encoding/pem" "errors" "fmt" "io" @@ -4076,153 +4075,6 @@ func mustCreateDatabase(t *testing.T, name, protocol, uri string) *types.Databas return database } -func TestAuthExport(t *testing.T) { - env := newWebPack(t, 1) - clusterName := env.server.ClusterName() - - proxy := env.proxies[0] - pack := proxy.authPack(t, "test-user@example.com", nil) - - validateTLSCertificateDERFunc := func(t *testing.T, b []byte) { - cert, err := x509.ParseCertificate(b) - require.NoError(t, err) - require.NotNil(t, cert, "ParseCertificate failed") - require.Equal(t, "localhost", cert.Subject.CommonName, "unexpected certificate subject CN") - } - - validateTLSCertificatePEMFunc := func(t *testing.T, b []byte) { - pemBlock, _ := pem.Decode(b) - require.NotNil(t, pemBlock, "pem.Decode failed") - - validateTLSCertificateDERFunc(t, pemBlock.Bytes) - } - - for _, tt := range []struct { - name string - authType string - expectedStatus int - assertBody func(t *testing.T, bs []byte) - }{ - { - name: "all", - authType: "", - expectedStatus: http.StatusOK, - assertBody: func(t *testing.T, b []byte) { - require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") - require.Contains(t, string(b), "cert-authority ecdsa-sha2-nistp256") - }, - }, - { - name: "host", - authType: "host", - expectedStatus: http.StatusOK, - assertBody: func(t *testing.T, b []byte) { - require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") - }, - }, - { - name: "user", - authType: "user", - expectedStatus: http.StatusOK, - assertBody: func(t *testing.T, b []byte) { - require.Contains(t, string(b), "cert-authority ecdsa-sha2-nistp256") - }, - }, - { - name: "windows", - authType: "windows", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificateDERFunc, - }, - { - name: "db", - authType: "db", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificatePEMFunc, - }, - { - name: "db-der", - authType: "db-der", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificateDERFunc, - }, - { - name: "db-client", - authType: "db-client", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificatePEMFunc, - }, - { - name: "db-client-der", - authType: "db-client-der", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificateDERFunc, - }, - { - name: "tls", - authType: "tls", - expectedStatus: http.StatusOK, - assertBody: validateTLSCertificatePEMFunc, - }, - { - name: "invalid", - authType: "invalid", - expectedStatus: http.StatusBadRequest, - assertBody: func(t *testing.T, b []byte) { - require.Contains(t, string(b), `"invalid" authority type is not supported`) - }, - }, - } { - t.Run(tt.name, func(t *testing.T) { - // export host certificate - t.Run("deprecated endpoint", func(t *testing.T) { - endpointExport := pack.clt.Endpoint("webapi", "sites", clusterName, "auth", "export") - authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) - }) - t.Run("new endpoint", func(t *testing.T) { - endpointExport := pack.clt.Endpoint("webapi", "auth", "export") - authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) - }) - }) - } -} - -func authExportTestByEndpoint(t *testing.T, endpointExport, authType string, expectedStatus int, assertBody func(t *testing.T, bs []byte)) { - ctx := context.Background() - - if authType != "" { - endpointExport = fmt.Sprintf("%s?type=%s", endpointExport, authType) - } - - reqCtx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() - - req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, endpointExport, nil) - require.NoError(t, err) - - anonHTTPClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - }, - } - - resp, err := anonHTTPClient.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - bs, err := io.ReadAll(resp.Body) - require.NoError(t, err) - - require.Equal(t, expectedStatus, resp.StatusCode, "invalid status code with body %s", string(bs)) - - require.NotEmpty(t, bs, "unexpected empty body from http response") - if assertBody != nil { - assertBody(t, bs) - } -} - func TestClusterDatabasesGet_NoRole(t *testing.T) { env := newWebPack(t, 1) diff --git a/lib/web/ca_export.go b/lib/web/ca_export.go new file mode 100644 index 0000000000000..8f1d04298ad75 --- /dev/null +++ b/lib/web/ca_export.go @@ -0,0 +1,58 @@ +// Teleport +// Copyright (C) 2025 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package web + +import ( + "net/http" + "strings" + "time" + + "github.com/gravitational/trace" + "github.com/julienschmidt/httprouter" + + "github.com/gravitational/teleport/lib/client" +) + +// authExportPublic returns the CA Certs that can be used to set up a chain of trust which includes the current Teleport Cluster +// +// GET /webapi/sites/:site/auth/export?type= +// GET /webapi/auth/export?type= +func (h *Handler) authExportPublic(w http.ResponseWriter, r *http.Request, p httprouter.Params) { + err := rateLimitRequest(r, h.limiter) + if err != nil { + http.Error(w, err.Error(), trace.ErrorToCode(err)) + return + } + authorities, err := client.ExportAuthorities( + r.Context(), + h.GetProxyClient(), + client.ExportAuthoritiesRequest{ + AuthType: r.URL.Query().Get("type"), + }, + ) + if err != nil { + h.logger.DebugContext(r.Context(), "Failed to generate CA Certs", "error", err) + http.Error(w, err.Error(), trace.ErrorToCode(err)) + return + } + + reader := strings.NewReader(authorities) + + // ServeContent sets the correct headers: Content-Type, Content-Length and Accept-Ranges. + // It also handles the Range negotiation + http.ServeContent(w, r, "authorized_hosts.txt", time.Now(), reader) +} diff --git a/lib/web/ca_export_test.go b/lib/web/ca_export_test.go new file mode 100644 index 0000000000000..228158c46bea9 --- /dev/null +++ b/lib/web/ca_export_test.go @@ -0,0 +1,178 @@ +// Teleport +// Copyright (C) 2025 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package web + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "fmt" + "io" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestAuthExport(t *testing.T) { + env := newWebPack(t, 1) + clusterName := env.server.ClusterName() + + proxy := env.proxies[0] + pack := proxy.authPack(t, "test-user@example.com", nil) + + validateTLSCertificateDERFunc := func(t *testing.T, b []byte) { + cert, err := x509.ParseCertificate(b) + require.NoError(t, err) + require.NotNil(t, cert, "ParseCertificate failed") + require.Equal(t, "localhost", cert.Subject.CommonName, "unexpected certificate subject CN") + } + + validateTLSCertificatePEMFunc := func(t *testing.T, b []byte) { + pemBlock, _ := pem.Decode(b) + require.NotNil(t, pemBlock, "pem.Decode failed") + + validateTLSCertificateDERFunc(t, pemBlock.Bytes) + } + + for _, tt := range []struct { + name string + authType string + expectedStatus int + assertBody func(t *testing.T, bs []byte) + }{ + { + name: "all", + authType: "", + expectedStatus: http.StatusOK, + assertBody: func(t *testing.T, b []byte) { + require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") + require.Contains(t, string(b), "cert-authority ecdsa-sha2-nistp256") + }, + }, + { + name: "host", + authType: "host", + expectedStatus: http.StatusOK, + assertBody: func(t *testing.T, b []byte) { + require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") + }, + }, + { + name: "user", + authType: "user", + expectedStatus: http.StatusOK, + assertBody: func(t *testing.T, b []byte) { + require.Contains(t, string(b), "cert-authority ecdsa-sha2-nistp256") + }, + }, + { + name: "windows", + authType: "windows", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificateDERFunc, + }, + { + name: "db", + authType: "db", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificatePEMFunc, + }, + { + name: "db-der", + authType: "db-der", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificateDERFunc, + }, + { + name: "db-client", + authType: "db-client", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificatePEMFunc, + }, + { + name: "db-client-der", + authType: "db-client-der", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificateDERFunc, + }, + { + name: "tls", + authType: "tls", + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificatePEMFunc, + }, + { + name: "invalid", + authType: "invalid", + expectedStatus: http.StatusBadRequest, + assertBody: func(t *testing.T, b []byte) { + require.Contains(t, string(b), `"invalid" authority type is not supported`) + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + // export host certificate + t.Run("deprecated endpoint", func(t *testing.T) { + endpointExport := pack.clt.Endpoint("webapi", "sites", clusterName, "auth", "export") + authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) + }) + t.Run("new endpoint", func(t *testing.T) { + endpointExport := pack.clt.Endpoint("webapi", "auth", "export") + authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) + }) + }) + } +} + +func authExportTestByEndpoint(t *testing.T, endpointExport, authType string, expectedStatus int, assertBody func(t *testing.T, bs []byte)) { + ctx := context.Background() + + if authType != "" { + endpointExport = fmt.Sprintf("%s?type=%s", endpointExport, authType) + } + + reqCtx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, endpointExport, nil) + require.NoError(t, err) + + anonHTTPClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + + resp, err := anonHTTPClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + bs, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, expectedStatus, resp.StatusCode, "invalid status code with body %s", string(bs)) + + require.NotEmpty(t, bs, "unexpected empty body from http response") + if assertBody != nil { + assertBody(t, bs) + } +} From a404c6c12e8775d7d0604dada9d55bb3b0eacc3c Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Thu, 16 Jan 2025 12:52:34 -0300 Subject: [PATCH 2/7] Implement "/auth/export?format=zip" --- lib/web/ca_export.go | 82 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/lib/web/ca_export.go b/lib/web/ca_export.go index 8f1d04298ad75..04defa2ce4294 100644 --- a/lib/web/ca_export.go +++ b/lib/web/ca_export.go @@ -17,6 +17,9 @@ package web import ( + "archive/zip" + "bytes" + "fmt" "net/http" "strings" "time" @@ -32,27 +35,88 @@ import ( // GET /webapi/sites/:site/auth/export?type= // GET /webapi/auth/export?type= func (h *Handler) authExportPublic(w http.ResponseWriter, r *http.Request, p httprouter.Params) { - err := rateLimitRequest(r, h.limiter) - if err != nil { + if err := h.authExportPublicError(w, r, p); err != nil { http.Error(w, err.Error(), trace.ErrorToCode(err)) return } - authorities, err := client.ExportAuthorities( - r.Context(), + + // Success output handled by authExportPublicInternal. +} + +// authExportPublicError implements authExportPublic, except it returns an error +// in case of failure. Output is only written on success. +func (h *Handler) authExportPublicError(w http.ResponseWriter, r *http.Request, p httprouter.Params) error { + err := rateLimitRequest(r, h.limiter) + if err != nil { + return trace.Wrap(err) + } + + query := r.URL.Query() + caType := query.Get("type") // validated by ExportAllAuthorities + format := query.Get("format") + + const formatZip = "zip" + if format != "" && format != formatZip { + return trace.BadParameter("unsupported format %q", format) + } + + ctx := r.Context() + authorities, err := client.ExportAllAuthorities( + ctx, h.GetProxyClient(), client.ExportAuthoritiesRequest{ - AuthType: r.URL.Query().Get("type"), + AuthType: caType, }, ) if err != nil { - h.logger.DebugContext(r.Context(), "Failed to generate CA Certs", "error", err) - http.Error(w, err.Error(), trace.ErrorToCode(err)) - return + h.logger.DebugContext(ctx, "Failed to generate CA Certs", "error", err) + return trace.Wrap(err) } - reader := strings.NewReader(authorities) + if format == formatZip { + return h.authExportPublicZip(w, r, authorities) + } + if l := len(authorities); l > 1 { + return trace.BadParameter("found %d authorities to export, use format=%s to export all", l, formatZip) + } // ServeContent sets the correct headers: Content-Type, Content-Length and Accept-Ranges. // It also handles the Range negotiation + reader := strings.NewReader(string(authorities[0].Data)) http.ServeContent(w, r, "authorized_hosts.txt", time.Now(), reader) + return nil +} + +func (h *Handler) authExportPublicZip( + w http.ResponseWriter, + r *http.Request, + authorities []*client.ExportedAuthority, +) error { + now := h.clock.Now().UTC() + + // Write authorities to a zip buffer as files named "ca$i.cert". + out := &bytes.Buffer{} + zipWriter := zip.NewWriter(out) + for i, authority := range authorities { + fh := &zip.FileHeader{ + Name: fmt.Sprintf("ca%d.cer", i), + Method: zip.Deflate, + Modified: now, + } + fh.SetMode(0644) + + fileWriter, err := zipWriter.CreateHeader(fh) + if err != nil { + return trace.Wrap(err) + } + fileWriter.Write(authority.Data) + } + if err := zipWriter.Close(); err != nil { + return trace.Wrap(err) + } + + const zipName = "Teleport_CA.zip" + w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment;filename="%s"`, zipName)) + http.ServeContent(w, r, zipName, now, bytes.NewReader(out.Bytes())) + return nil } From a3f494617a34f9b606d6e1f33019740a6585230c Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Tue, 21 Jan 2025 15:21:55 -0300 Subject: [PATCH 3/7] Refactor existing tests --- lib/web/ca_export_test.go | 104 +++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 40 deletions(-) diff --git a/lib/web/ca_export_test.go b/lib/web/ca_export_test.go index 228158c46bea9..42d9136b54129 100644 --- a/lib/web/ca_export_test.go +++ b/lib/web/ca_export_test.go @@ -21,9 +21,9 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" - "fmt" "io" "net/http" + "net/url" "testing" "time" @@ -40,7 +40,6 @@ func TestAuthExport(t *testing.T) { validateTLSCertificateDERFunc := func(t *testing.T, b []byte) { cert, err := x509.ParseCertificate(b) require.NoError(t, err) - require.NotNil(t, cert, "ParseCertificate failed") require.Equal(t, "localhost", cert.Subject.CommonName, "unexpected certificate subject CN") } @@ -51,15 +50,16 @@ func TestAuthExport(t *testing.T) { validateTLSCertificateDERFunc(t, pemBlock.Bytes) } + ctx := context.Background() + for _, tt := range []struct { name string - authType string + params url.Values expectedStatus int assertBody func(t *testing.T, bs []byte) }{ { name: "all", - authType: "", expectedStatus: http.StatusOK, assertBody: func(t *testing.T, b []byte) { require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") @@ -67,60 +67,78 @@ func TestAuthExport(t *testing.T) { }, }, { - name: "host", - authType: "host", + name: "host", + params: url.Values{ + "type": []string{"host"}, + }, expectedStatus: http.StatusOK, assertBody: func(t *testing.T, b []byte) { require.Contains(t, string(b), "@cert-authority localhost,*.localhost ecdsa-sha2-nistp256 ") }, }, { - name: "user", - authType: "user", + name: "user", + params: url.Values{ + "type": []string{"user"}, + }, expectedStatus: http.StatusOK, assertBody: func(t *testing.T, b []byte) { require.Contains(t, string(b), "cert-authority ecdsa-sha2-nistp256") }, }, { - name: "windows", - authType: "windows", + name: "windows", + params: url.Values{ + "type": []string{"windows"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificateDERFunc, }, { - name: "db", - authType: "db", + name: "db", + params: url.Values{ + "type": []string{"db"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificatePEMFunc, }, { - name: "db-der", - authType: "db-der", + name: "db-der", + params: url.Values{ + "type": []string{"db-der"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificateDERFunc, }, { - name: "db-client", - authType: "db-client", + name: "db-client", + params: url.Values{ + "type": []string{"db-client"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificatePEMFunc, }, { - name: "db-client-der", - authType: "db-client-der", + name: "db-client-der", + params: url.Values{ + "type": []string{"db-client-der"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificateDERFunc, }, { - name: "tls", - authType: "tls", + name: "tls", + params: url.Values{ + "type": []string{"tls"}, + }, expectedStatus: http.StatusOK, assertBody: validateTLSCertificatePEMFunc, }, { - name: "invalid", - authType: "invalid", + name: "invalid", + params: url.Values{ + "type": []string{"invalid"}, + }, expectedStatus: http.StatusBadRequest, assertBody: func(t *testing.T, b []byte) { require.Contains(t, string(b), `"invalid" authority type is not supported`) @@ -128,30 +146,36 @@ func TestAuthExport(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - // export host certificate + runTest := func(t *testing.T, endpoint string) { + authExportTestByEndpoint(ctx, t, endpoint, tt.params, tt.expectedStatus, tt.assertBody) + } + t.Run("deprecated endpoint", func(t *testing.T) { - endpointExport := pack.clt.Endpoint("webapi", "sites", clusterName, "auth", "export") - authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) + runTest(t, pack.clt.Endpoint("webapi", "sites", clusterName, "auth", "export")) }) t.Run("new endpoint", func(t *testing.T) { - endpointExport := pack.clt.Endpoint("webapi", "auth", "export") - authExportTestByEndpoint(t, endpointExport, tt.authType, tt.expectedStatus, tt.assertBody) + runTest(t, pack.clt.Endpoint("webapi", "auth", "export")) }) }) } } -func authExportTestByEndpoint(t *testing.T, endpointExport, authType string, expectedStatus int, assertBody func(t *testing.T, bs []byte)) { - ctx := context.Background() - - if authType != "" { - endpointExport = fmt.Sprintf("%s?type=%s", endpointExport, authType) - } - +func authExportTestByEndpoint( + ctx context.Context, + t *testing.T, + exportEndpoint string, + params url.Values, + expectedStatus int, + assertBody func(t *testing.T, bs []byte), +) { reqCtx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, endpointExport, nil) + encodedParams := params.Encode() + if encodedParams != "" { + exportEndpoint = exportEndpoint + "?" + encodedParams + } + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, exportEndpoint, nil) require.NoError(t, err) anonHTTPClient := &http.Client{ @@ -164,15 +188,15 @@ func authExportTestByEndpoint(t *testing.T, endpointExport, authType string, exp resp, err := anonHTTPClient.Do(req) require.NoError(t, err) - defer resp.Body.Close() - bs, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) + resp.Body.Close() require.NoError(t, err) - require.Equal(t, expectedStatus, resp.StatusCode, "invalid status code with body %s", string(bs)) + require.Equal(t, expectedStatus, resp.StatusCode, "invalid status code with body %s", string(body)) - require.NotEmpty(t, bs, "unexpected empty body from http response") + require.NotEmpty(t, body, "unexpected empty body from http response") if assertBody != nil { - assertBody(t, bs) + assertBody(t, body) } } From 70c6058d1d2d0262fb50f9288e2ed0c214935485 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Tue, 21 Jan 2025 15:56:12 -0300 Subject: [PATCH 4/7] Test format=zip --- lib/web/ca_export_test.go | 73 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/lib/web/ca_export_test.go b/lib/web/ca_export_test.go index 42d9136b54129..258b9f44f4910 100644 --- a/lib/web/ca_export_test.go +++ b/lib/web/ca_export_test.go @@ -17,20 +17,27 @@ package web import ( + "archive/zip" + "bytes" "context" "crypto/tls" "crypto/x509" "encoding/pem" + "fmt" "io" "net/http" "net/url" + "sort" "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestAuthExport(t *testing.T) { + t.Parallel() + env := newWebPack(t, 1) clusterName := env.server.ClusterName() @@ -50,6 +57,39 @@ func TestAuthExport(t *testing.T) { validateTLSCertificateDERFunc(t, pemBlock.Bytes) } + validateFormatZip := func( + t *testing.T, + body []byte, + wantCAFiles int, + validateCAFile func(t *testing.T, contents []byte), + ) { + r, err := zip.NewReader(bytes.NewReader(body), int64(len(body))) + require.NoError(t, err, "zip.NewReader") + + files := r.File + assert.Len(t, files, wantCAFiles, "mismatched number of CA files inside zip") + + // Traverse files in order. We want them to be named "ca0.cer, "ca1.cer", + // etc. + sort.Slice(files, func(i, j int) bool { + return files[i].Name < files[j].Name + }) + for i, f := range files { + wantName := fmt.Sprintf("ca%d.cer", i) + assert.Equal(t, wantName, f.Name, "mismatched name of CA file inside zip") + + fileReader, err := f.Open() + require.NoError(t, err, "open CA file inside zip") + fileBytes, err := io.ReadAll(fileReader) + require.NoError(t, err, "read CA file contents inside zip") + + validateCAFile(t, fileBytes) + } + } + validateFormatZipPEM := func(t *testing.T, body []byte, wantCAFiles int) { + validateFormatZip(t, body, wantCAFiles, validateTLSCertificatePEMFunc) + } + ctx := context.Background() for _, tt := range []struct { @@ -144,8 +184,41 @@ func TestAuthExport(t *testing.T) { require.Contains(t, string(b), `"invalid" authority type is not supported`) }, }, + { + name: "format empty", + params: url.Values{ + "type": []string{"tls-user"}, + "format": []string{""}, + }, + expectedStatus: http.StatusOK, + assertBody: validateTLSCertificatePEMFunc, + }, + { + name: "format invalid", + params: url.Values{ + "type": []string{"tls-user"}, + "format": []string{"invalid"}, + }, + expectedStatus: http.StatusBadRequest, + assertBody: func(t *testing.T, b []byte) { + assert.Contains(t, string(b), "unsupported format") + }, + }, + { + name: "format=zip", + params: url.Values{ + "type": []string{"tls-user"}, + "format": []string{"zip"}, + }, + expectedStatus: http.StatusOK, + assertBody: func(t *testing.T, b []byte) { + validateFormatZipPEM(t, b, 1 /* wantCAFiles */) + }, + }, } { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + runTest := func(t *testing.T, endpoint string) { authExportTestByEndpoint(ctx, t, endpoint, tt.params, tt.expectedStatus, tt.assertBody) } From 55be97cc4efd303cf89e65d809bc84530896a575 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 22 Jan 2025 16:12:11 -0300 Subject: [PATCH 5/7] Fix comment --- lib/web/ca_export.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web/ca_export.go b/lib/web/ca_export.go index 04defa2ce4294..768369ab2c46e 100644 --- a/lib/web/ca_export.go +++ b/lib/web/ca_export.go @@ -40,7 +40,7 @@ func (h *Handler) authExportPublic(w http.ResponseWriter, r *http.Request, p htt return } - // Success output handled by authExportPublicInternal. + // Success output handled by authExportPublicError. } // authExportPublicError implements authExportPublic, except it returns an error From 24a2e7ed4ed15a06a7c3412201aad5fa8d628415 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 22 Jan 2025 16:13:04 -0300 Subject: [PATCH 6/7] Use bytes.NewReader --- lib/web/ca_export.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/web/ca_export.go b/lib/web/ca_export.go index 768369ab2c46e..99e548f84ece1 100644 --- a/lib/web/ca_export.go +++ b/lib/web/ca_export.go @@ -21,7 +21,6 @@ import ( "bytes" "fmt" "net/http" - "strings" "time" "github.com/gravitational/trace" @@ -82,7 +81,7 @@ func (h *Handler) authExportPublicError(w http.ResponseWriter, r *http.Request, // ServeContent sets the correct headers: Content-Type, Content-Length and Accept-Ranges. // It also handles the Range negotiation - reader := strings.NewReader(string(authorities[0].Data)) + reader := bytes.NewReader(authorities[0].Data) http.ServeContent(w, r, "authorized_hosts.txt", time.Now(), reader) return nil } From 9e1b3097f866ddc4c35c119250ef6066bbc852bb Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 22 Jan 2025 16:41:52 -0300 Subject: [PATCH 7/7] Remove lib/client.ExportAuthorities --- lib/client/ca_export.go | 26 -------------------------- lib/client/ca_export_test.go | 22 ++-------------------- 2 files changed, 2 insertions(+), 46 deletions(-) diff --git a/lib/client/ca_export.go b/lib/client/ca_export.go index 5331f0c2a1923..011be4c4682d9 100644 --- a/lib/client/ca_export.go +++ b/lib/client/ca_export.go @@ -157,32 +157,6 @@ func exportAllAuthorities( return authorities, nil } -// ExportAuthorities is the single-authority version of [ExportAllAuthorities]. -// Soft-deprecated, prefer using [ExportAllAuthorities] and handling exports -// with more than one authority gracefully. -func ExportAuthorities(ctx context.Context, client authclient.ClientI, req ExportAuthoritiesRequest) (string, error) { - // TODO(codingllama): Remove ExportAuthorities. - return exportAuthorities(ctx, client, req, ExportAllAuthorities) -} - -func exportAuthorities( - ctx context.Context, - client authclient.ClientI, - req ExportAuthoritiesRequest, - exportAllFunc func(context.Context, authclient.ClientI, ExportAuthoritiesRequest) ([]*ExportedAuthority, error), -) (string, error) { - authorities, err := exportAllFunc(ctx, client, req) - if err != nil { - return "", trace.Wrap(err) - } - // At least one authority is guaranteed on success by both ExportAll methods. - if l := len(authorities); l > 1 { - return "", trace.BadParameter("export returned %d authorities, expected exactly one", l) - } - - return string(authorities[0].Data), nil -} - func exportAuth(ctx context.Context, client authclient.ClientI, req ExportAuthoritiesRequest, exportSecrets bool) ([]*ExportedAuthority, error) { var typesToExport []types.CertAuthType diff --git a/lib/client/ca_export_test.go b/lib/client/ca_export_test.go index bfd1d5f428190..2bec5410e195c 100644 --- a/lib/client/ca_export_test.go +++ b/lib/client/ca_export_test.go @@ -24,7 +24,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "fmt" "math/big" "testing" "time" @@ -323,34 +322,17 @@ func TestExportAuthorities(t *testing.T) { assertFunc(t, exported) } - runUnaryTest := func( - t *testing.T, - exportFunc func(context.Context, authclient.ClientI, ExportAuthoritiesRequest) (string, error), - assertFunc func(t *testing.T, output string), - ) { - exported, err := exportFunc(ctx, mockedAuthClient, tt.req) - tt.errorCheck(t, err) - if err != nil { - return - } - - assertFunc(t, exported) - } - t.Run(tt.name, func(t *testing.T) { t.Parallel() - t.Run(fmt.Sprintf("%s/ExportAllAuthorities", tt.name), func(t *testing.T) { + t.Run("ExportAllAuthorities", func(t *testing.T) { runTest(t, ExportAllAuthorities, tt.assertNoSecrets) }) - t.Run(fmt.Sprintf("%s/ExportAuthorities", tt.name), func(t *testing.T) { - runUnaryTest(t, ExportAuthorities, tt.assertNoSecrets) - }) if tt.skipSecrets { return } - t.Run(fmt.Sprintf("%s/ExportAllAuthoritiesSecrets", tt.name), func(t *testing.T) { + t.Run("ExportAllAuthoritiesSecrets", func(t *testing.T) { runTest(t, ExportAllAuthoritiesSecrets, tt.assertSecrets) }) })