From a3714d2ed93a0304a7af77f8ecdc44e4b28e718a Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Tue, 24 Aug 2021 08:50:03 -0400 Subject: [PATCH] Add fleetctl debug connection command (#1706) Adds the `fleetctl debug connection` command to investigate connection issues to the fleet server. Closes #1579 . --- changes/issue-1579-add-debug-connection | 1 + cmd/fleetctl/api.go | 144 ++++++++---- cmd/fleetctl/config.go | 2 +- cmd/fleetctl/debug.go | 218 +++++++++++++++++- cmd/fleetctl/debug_test.go | 200 ++++++++++++++++ cmd/fleetctl/flags.go | 17 +- cmd/fleetctl/get.go | 2 +- ee/fleetctl/updates.go | 2 +- orbit/cmd/orbit/orbit.go | 4 +- orbit/cmd/orbit/shell.go | 2 +- orbit/pkg/osquery/osquery.go | 2 +- orbit/pkg/packaging/linux_shared.go | 2 +- orbit/pkg/packaging/macos.go | 2 +- orbit/pkg/packaging/packaging.go | 2 +- orbit/pkg/packaging/windows.go | 2 +- orbit/pkg/update/filestore/filestore.go | 2 +- orbit/pkg/update/update.go | 2 +- pkg/README.md | 3 + {orbit/pkg => pkg}/certificate/certificate.go | 45 ++-- .../certificate/certificate_test.go | 0 .../certificate/testdata/empty.crt | 0 .../pkg => pkg}/certificate/testdata/test.crt | 0 .../pkg => pkg}/certificate/testdata/test.key | 0 {secure => pkg/secure}/secure.go | 0 {secure => pkg/secure}/secure_test.go | 0 server/logging/filesystem.go | 2 +- server/service/client.go | 16 +- 27 files changed, 594 insertions(+), 78 deletions(-) create mode 100644 changes/issue-1579-add-debug-connection create mode 100644 cmd/fleetctl/debug_test.go create mode 100644 pkg/README.md rename {orbit/pkg => pkg}/certificate/certificate.go (54%) rename {orbit/pkg => pkg}/certificate/certificate_test.go (100%) rename {orbit/pkg => pkg}/certificate/testdata/empty.crt (100%) rename {orbit/pkg => pkg}/certificate/testdata/test.crt (100%) rename {orbit/pkg => pkg}/certificate/testdata/test.key (100%) rename {secure => pkg/secure}/secure.go (100%) rename {secure => pkg/secure}/secure_test.go (100%) diff --git a/changes/issue-1579-add-debug-connection b/changes/issue-1579-add-debug-connection new file mode 100644 index 0000000000..d4512cbb2b --- /dev/null +++ b/changes/issue-1579-add-debug-connection @@ -0,0 +1 @@ +* Add ability to troubleshoot connection issues with the `fleetctl debug connection` command. diff --git a/cmd/fleetctl/api.go b/cmd/fleetctl/api.go index 5b3498339f..ee57b863a9 100644 --- a/cmd/fleetctl/api.go +++ b/cmd/fleetctl/api.go @@ -1,8 +1,13 @@ package main import ( + "crypto/tls" + "crypto/x509" "flag" "fmt" + "io/ioutil" + "net/http" + "net/url" "os" "runtime" @@ -12,49 +17,12 @@ import ( ) func unauthenticatedClientFromCLI(c *cli.Context) (*service.Client, error) { - if flag.Lookup("test.v") != nil { - return service.NewClient(os.Getenv("FLEET_SERVER_ADDRESS"), true, "", "") - } - - if err := makeConfigIfNotExists(c.String("config")); err != nil { - return nil, errors.Wrapf(err, "error verifying that config exists at %s", c.String("config")) - } - - config, err := readConfig(c.String("config")) + cc, err := clientConfigFromCLI(c) if err != nil { return nil, err } - cc, ok := config.Contexts[c.String("context")] - if !ok { - return nil, fmt.Errorf("context %q is not found", c.String("context")) - } - - if cc.Address == "" { - return nil, errors.New("set the Fleet API address with: fleetctl config set --address https://localhost:8080") - } - - if runtime.GOOS == "windows" && cc.RootCA == "" && !cc.TLSSkipVerify { - return nil, errors.New("Windows clients must configure rootca (secure) or tls-skip-verify (insecure)") - } - - var options []service.ClientOption - if getDebug(c) { - options = append(options, service.EnableClientDebug()) - } - - fleet, err := service.NewClient( - cc.Address, - cc.TLSSkipVerify, - cc.RootCA, - cc.URLPrefix, - options..., - ) - if err != nil { - return nil, errors.Wrap(err, "error creating Fleet API client handler") - } - - return fleet, nil + return unauthenticatedClientFromConfig(cc, getDebug(c)) } func clientFromCLI(c *cli.Context) (*service.Client, error) { @@ -87,3 +55,101 @@ func clientFromCLI(c *cli.Context) (*service.Client, error) { return fleet, nil } + +func unauthenticatedClientFromConfig(cc Context, debug bool) (*service.Client, error) { + if flag.Lookup("test.v") != nil { + return service.NewClient(os.Getenv("FLEET_SERVER_ADDRESS"), true, "", "") + } + + if cc.Address == "" { + return nil, errors.New("set the Fleet API address with: fleetctl config set --address https://localhost:8080") + } + + if runtime.GOOS == "windows" && cc.RootCA == "" && !cc.TLSSkipVerify { + return nil, errors.New("Windows clients must configure rootca (secure) or tls-skip-verify (insecure)") + } + + var options []service.ClientOption + if debug { + options = append(options, service.EnableClientDebug()) + } + + fleet, err := service.NewClient( + cc.Address, + cc.TLSSkipVerify, + cc.RootCA, + cc.URLPrefix, + options..., + ) + if err != nil { + return nil, errors.Wrap(err, "error creating Fleet API client handler") + } + + return fleet, nil +} + +// returns an HTTP client and the parsed URL for the configured server's +// address. The reason why this exists instead of using +// unauthenticatedClientFromConfig is because this doesn't apply the same rules +// around TLS config - in particular, it only sets a root CA if one is +// explicitly configured. +func rawHTTPClientFromConfig(cc Context) (*http.Client, *url.URL, error) { + if flag.Lookup("test.v") != nil { + cc.Address = os.Getenv("FLEET_SERVER_ADDRESS") + } + baseURL, err := url.Parse(cc.Address) + if err != nil { + return nil, nil, errors.Wrap(err, "parse address") + } + + var rootCA *x509.CertPool + if cc.RootCA != "" { + rootCA = x509.NewCertPool() + // read in the root cert file specified in the context + certs, err := ioutil.ReadFile(cc.RootCA) + if err != nil { + return nil, nil, errors.Wrap(err, "reading root CA") + } + + // add certs to pool + if ok := rootCA.AppendCertsFromPEM(certs); !ok { + return nil, nil, errors.New("failed to add certificates to root CA pool") + } + } + + cli := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: cc.TLSSkipVerify, + RootCAs: rootCA, + }, + }, + } + return cli, baseURL, nil +} + +func clientConfigFromCLI(c *cli.Context) (Context, error) { + if flag.Lookup("test.v") != nil { + return Context{ + Address: os.Getenv("FLEET_SERVER_ADDRESS"), + TLSSkipVerify: true, + }, nil + } + + var zeroCtx Context + + if err := makeConfigIfNotExists(c.String("config")); err != nil { + return zeroCtx, errors.Wrapf(err, "error verifying that config exists at %s", c.String("config")) + } + + config, err := readConfig(c.String("config")) + if err != nil { + return zeroCtx, err + } + + cc, ok := config.Contexts[c.String("context")] + if !ok { + return zeroCtx, fmt.Errorf("context %q is not found", c.String("context")) + } + return cc, nil +} diff --git a/cmd/fleetctl/config.go b/cmd/fleetctl/config.go index 03d266d501..30a68e4941 100644 --- a/cmd/fleetctl/config.go +++ b/cmd/fleetctl/config.go @@ -7,7 +7,7 @@ import ( "path/filepath" "strconv" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/ghodss/yaml" "github.com/pkg/errors" "github.com/urfave/cli/v2" diff --git a/cmd/fleetctl/debug.go b/cmd/fleetctl/debug.go index f433fa4066..299477ff5c 100644 --- a/cmd/fleetctl/debug.go +++ b/cmd/fleetctl/debug.go @@ -2,9 +2,15 @@ package main import ( "archive/tar" + "bytes" "compress/gzip" + "context" + "encoding/json" "fmt" "io/ioutil" + "net" + "net/http" + "net/url" "os" "strings" "time" @@ -12,7 +18,8 @@ import ( "github.com/pkg/errors" "github.com/urfave/cli/v2" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/certificate" + "github.com/fleetdm/fleet/v4/pkg/secure" ) func debugCommand() *cli.Command { @@ -31,6 +38,7 @@ func debugCommand() *cli.Command { debugGoroutineCommand(), debugTraceCommand(), debugArchiveCommand(), + debugConnectionCommand(), }, } } @@ -316,3 +324,211 @@ func debugArchiveCommand() *cli.Command { }, } } + +func debugConnectionCommand() *cli.Command { + const timeoutPerCheck = 10 * time.Second + + return &cli.Command{ + Name: "connection", + ArgsUsage: "[
]", + Usage: "Investigate the cause of a connection failure to the Fleet server.", + Description: `Run a number of checks to debug a connection failure to the Fleet +server. + +If
is provided, this is the address that is investigated, +otherwise the address of the provided context is used, with +the default context used if none is explicitly specified.`, + Flags: []cli.Flag{ + configFlag(), + contextFlag(), + fleetCertificateFlag(), + }, + Action: func(c *cli.Context) error { + var addr string + if narg := c.NArg(); narg > 0 { + if narg > 1 { + return errors.New("too many arguments") + } + addr = c.Args().First() + + // when an address is provided, the --config and --context flags + // cannot be set. + if c.IsSet("config") { + return errors.New("the --config flag cannot be set when an
is provided") + } + if c.IsSet("context") { + return errors.New("the --context flag cannot be set when an
is provided") + } + } else if cert := getFleetCertificate(c); cert != "" { + return errors.New("the --fleet-certificate flag can only be set when an
is provided") + } + + // ensure there is an address to debug (either from the config's context, + // or explicit) + cc, err := clientConfigFromCLI(c) + if err != nil { + return err + } + configContext := c.String("context") + if addr != "" { + cc.Address = addr + + // when an address is explicitly provided, we don't use any of the + // config's context values. + configContext = "none - using provided address" + cc.TLSSkipVerify = false + cc.RootCA = "" + } + if cc.Address == "" { + return errors.New(`set the Fleet API address with: fleetctl config set --address https://localhost:8080 +or provide an
argument to debug: fleetctl debug connection localhost:8080`) + } + + // it's ok if there is no scheme specified, add it automatically + if !strings.Contains(cc.Address, "://") { + cc.Address = "https://" + cc.Address + } + + if certPath := getFleetCertificate(c); certPath != "" { + // if a certificate is provided, use it as root CA + cc.RootCA = certPath + cc.TLSSkipVerify = false + } + + cli, baseURL, err := rawHTTPClientFromConfig(cc) + if err != nil { + return err + } + + // print a summary of the address and TLS context that is investigated + fmt.Fprintf(c.App.Writer, "Debugging connection to %s; Configuration context: %s; ", baseURL.Hostname(), configContext) + rootCA := "(system)" + if cc.RootCA != "" { + rootCA = cc.RootCA + } + fmt.Fprintf(c.App.Writer, "Root CA: %s; ", rootCA) + tlsMode := "secure" + if cc.TLSSkipVerify { + tlsMode = "insecure" + } + fmt.Fprintf(c.App.Writer, "TLS: %s.\n", tlsMode) + + // Check that the url's host resolves to an IP address or is otherwise + // a valid IP address directly. + if err := resolveHostname(c.Context, timeoutPerCheck, baseURL.Hostname()); err != nil { + return errors.Wrap(err, "Fail: resolve host") + } + fmt.Fprintf(c.App.Writer, "Success: can resolve host %s.\n", baseURL.Hostname()) + + // Attempt a raw TCP connection to host:port. + if err := dialHostPort(c.Context, timeoutPerCheck, baseURL.Host); err != nil { + return errors.Wrap(err, "Fail: dial server") + } + fmt.Fprintf(c.App.Writer, "Success: can dial server at %s.\n", baseURL.Host) + + if cert := getFleetCertificate(c); cert != "" { + // Run some validations on the TLS certificate. + if err := checkFleetCert(c.Context, timeoutPerCheck, cert, baseURL.Host); err != nil { + return errors.Wrap(err, "Fail: certificate") + } + fmt.Fprintln(c.App.Writer, "Success: TLS certificate seems valid.") + } + + // Check that the server responds with expected responses (by + // making a POST to /api/v1/osquery/enroll with an invalid + // secret). + if err := checkAPIEndpoint(c.Context, timeoutPerCheck, baseURL, cli); err != nil { + return errors.Wrap(err, "Fail: agent API endpoint") + } + fmt.Fprintln(c.App.Writer, "Success: agent API endpoints are available.") + + return nil + }, + } +} + +func resolveHostname(ctx context.Context, timeout time.Duration, host string) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + var r net.Resolver + ips, err := r.LookupIP(ctx, "ip", host) + if err != nil { + return err + } + if len(ips) == 0 { + return errors.New("no address found for host") + } + return nil +} + +func dialHostPort(ctx context.Context, timeout time.Duration, addr string) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + var d net.Dialer + conn, err := d.DialContext(ctx, "tcp", addr) + if err == nil { + conn.Close() + } + return err +} + +func checkAPIEndpoint(ctx context.Context, timeout time.Duration, baseURL *url.URL, client *http.Client) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + // make an enroll request with a deliberately invalid secret, + // to see if we get the expected error json payload. + var enrollRes struct { + Error string `json:"error"` + NodeInvalid bool `json:"node_invalid"` + } + headers := map[string]string{ + "Content-type": "application/json", + "Accept": "application/json", + } + + baseURL.Path = "/api/v1/osquery/enroll" + req, err := http.NewRequestWithContext( + ctx, + "POST", + baseURL.String(), + bytes.NewBufferString(`{"enroll_secret": "--invalid--"}`), + ) + if err != nil { + return errors.Wrap(err, "creating request object") + } + for k, v := range headers { + req.Header.Set(k, v) + } + + res, err := client.Do(req) + if err != nil { + return errors.Wrap(err, "request failed") + } + defer res.Body.Close() + + if err := json.NewDecoder(res.Body).Decode(&enrollRes); err != nil { + return errors.Wrap(err, "invalid JSON") + } + if res.StatusCode != http.StatusUnauthorized || enrollRes.Error == "" || !enrollRes.NodeInvalid { + return fmt.Errorf("unexpected %d response", res.StatusCode) + } + return nil +} + +func checkFleetCert(ctx context.Context, timeout time.Duration, certPath, addr string) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + certPool, err := certificate.LoadPEM(certPath) + if err != nil { + return err + } + if err := certificate.ValidateConnectionContext(ctx, certPool, "https://"+addr); err != nil { + return err + } + + return nil +} diff --git a/cmd/fleetctl/debug_test.go b/cmd/fleetctl/debug_test.go new file mode 100644 index 0000000000..c2a8a87a14 --- /dev/null +++ b/cmd/fleetctl/debug_test.go @@ -0,0 +1,200 @@ +package main + +import ( + "bytes" + "context" + "crypto/rand" + "encoding/hex" + "encoding/pem" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/pkg/errors" + "github.com/stretchr/testify/require" +) + +const ( + // Generated using this command in `go env GOROOT`/src/crypto/tls: + // go run generate_cert.go --rsa-bits 1024 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h + // Certificate is only valid for example.com, and so should fail validation + // with a localhost-running httptest.NewTLSServer. + exampleDotComCertDotPem = `-----BEGIN CERTIFICATE----- +MIICGzCCAYSgAwIBAgIRAM596905ZjtK0p+hURZWO7IwDQYJKoZIhvcNAQELBQAw +EjEQMA4GA1UEChMHQWNtZSBDbzAgFw03MDAxMDEwMDAwMDBaGA8yMDg0MDEyOTE2 +MDAwMFowEjEQMA4GA1UEChMHQWNtZSBDbzCBnzANBgkqhkiG9w0BAQEFAAOBjQAw +gYkCgYEA57PzoKfRgAYvOte5RVKEm4g6hD6jhxeg/lyvuidbuL9XzyvWesKGqxXh +LxMTrAeH1T3LbLlU0c/OdwcPQRLErqXee0YM3OeVhlZLnnOfyywE7WRFwAtS+uSm +m61Mrx8VHLqXiN8R3yQPiHmekuHIDMvIkC793d2YpaV02grWH7ECAwEAAaNvMG0w +DgYDVR0PAQH/BAQDAgKkMBMGA1UdJQQMMAoGCCsGAQUFBwMBMA8GA1UdEwEB/wQF +MAMBAf8wHQYDVR0OBBYEFI3hGM84qbH234gBQmbCShCq0430MBYGA1UdEQQPMA2C +C2V4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4GBAHqLUn9kpHdAElEwAP/7Xoth +yWkBFCfkIy2ftaWJKTB1nDfxbdEuJ1BfMDYyM5anYd+d/Id7w3fe3Wn+VkOnxxtZ +oug6edBNpdhp8r2/4t6n3AouK0/zG2naAlmXV0JoFuEvy2bX0BbbbPg+v4WNZIsC +0cUq8IOA9g0kHJar8rAI +-----END CERTIFICATE-----` +) + +func TestDebugConnectionCommand(t *testing.T) { + t.Run("without certificate", func(t *testing.T) { + server, ds := runServerWithMockedDS(t) + defer server.Close() + + ds.VerifyEnrollSecretFunc = func(secret string) (*fleet.EnrollSecret, error) { + return nil, errors.New("invalid") + } + + output := runAppForTest(t, []string{"debug", "connection"}) + // 3 successes: resolve host, dial address, check api endpoint + require.Equal(t, 3, strings.Count(output, "Success:")) + }) + + t.Run("invalid certificate flag without address", func(t *testing.T) { + _, _, err := runAppNoChecks([]string{"debug", "connection", "--fleet-certificate", "cert.pem"}) + require.Contains(t, err.Error(), "--fleet-certificate") + }) + + t.Run("invalid context flag with address", func(t *testing.T) { + _, _, err := runAppNoChecks([]string{"debug", "connection", "--context", "test", "localhost:8080"}) + require.Contains(t, err.Error(), "--context") + }) + + t.Run("invalid config flag with address", func(t *testing.T) { + _, _, err := runAppNoChecks([]string{"debug", "connection", "--config", "/tmp/nosuchfile", "localhost:8080"}) + require.Contains(t, err.Error(), "--config") + }) + + t.Run("with valid certificate", func(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, `{"error": "error", "node_invalid": true}`) + })) + defer srv.Close() + os.Setenv("FLEET_SERVER_ADDRESS", srv.URL) + + // get the certificate of the TLS server + certPath := rawCertToPemFile(t, srv.Certificate().Raw) + + output := runAppForTest(t, []string{"debug", "connection", "--fleet-certificate", certPath, srv.URL}) + // 4 successes: resolve host, dial address, certificate, check api endpoint + t.Log(output) + require.Equal(t, 4, strings.Count(output, "Success:")) + }) + + t.Run("with invalid certificate", func(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, `{"error": "error", "node_invalid": true}`) + })) + defer srv.Close() + os.Setenv("FLEET_SERVER_ADDRESS", srv.URL) + + // get the invalid certificate (for example.com) + dir := t.TempDir() + certPath := filepath.Join(dir, "cert.pem") + require.NoError(t, ioutil.WriteFile(certPath, []byte(exampleDotComCertDotPem), 0600)) + + buf, _, err := runAppNoChecks([]string{"debug", "connection", "--fleet-certificate", certPath, srv.URL}) + // 2 successes: resolve host, dial address + t.Log(buf.String()) + require.Equal(t, 2, strings.Count(buf.String(), "Success:")) + // 1 failure: invalid certificate + t.Log(err) + require.Error(t, err) + require.Equal(t, 1, strings.Count(err.Error(), "Fail: certificate:")) + }) +} + +// encodes raw certificate bytes to a PEM-encoded temp file, returns the path. +func rawCertToPemFile(t *testing.T, raw []byte) string { + t.Helper() + + var buf bytes.Buffer + require.NoError(t, pem.Encode(&buf, &pem.Block{ + Type: "CERTIFICATE", + Bytes: raw, + })) + + dir := t.TempDir() + certPath := filepath.Join(dir, "cert.pem") + require.NoError(t, ioutil.WriteFile(certPath, buf.Bytes(), 0600)) + return certPath +} + +func TestDebugConnectionChecks(t *testing.T) { + const timeout = 100 * time.Millisecond + + t.Run("resolveHostname", func(t *testing.T) { + // resolves host name + err := resolveHostname(context.Background(), timeout, "localhost") + require.NoError(t, err) + + // resolves ip4 address + err = resolveHostname(context.Background(), timeout, "127.0.0.1") + require.NoError(t, err) + + // resolves ip6 address + err = resolveHostname(context.Background(), timeout, "::1") + require.NoError(t, err) + + // fails on invalid host + randBytes := make([]byte, 8) + _, err = rand.Read(randBytes) + require.NoError(t, err) + noSuchHost := "no_such_host" + hex.EncodeToString(randBytes) + + err = resolveHostname(context.Background(), timeout, noSuchHost) + require.Error(t, err) + }) + + t.Run("checkAPIEndpoint", func(t *testing.T) { + cases := [...]struct { + code int // == 0 panics, negative value waits for timeout, sets status code to absolute value + body string + errContains string // empty if checkAPIEndpoint should not return an error + }{ + {401, `{"error": "fail", "node_invalid": true}`, ""}, + {-401, `{"error": "fail", "node_invalid": true}`, "deadline exceeded"}, + {200, `{"error": "", "node_invalid": false}`, "unexpected 200 response"}, + {0, `panic`, "EOF"}, + } + var callCount int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + res := cases[callCount] + + switch { + case res.code == 0: + panic(res.body) + case res.code < 0: + time.Sleep(timeout + time.Millisecond) + res.code = -res.code + } + w.WriteHeader(res.code) + fmt.Fprint(w, res.body) + })) + defer srv.Close() + + os.Setenv("FLEET_SERVER_ADDRESS", srv.URL) + cli, base, err := rawHTTPClientFromConfig(Context{Address: srv.URL, TLSSkipVerify: true}) + require.NoError(t, err) + for i, c := range cases { + callCount = i + t.Run(fmt.Sprint(c.code), func(t *testing.T) { + err := checkAPIEndpoint(context.Background(), timeout, base, cli) + if c.errContains == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), c.errContains) + } + }) + } + }) +} diff --git a/cmd/fleetctl/flags.go b/cmd/fleetctl/flags.go index af1bca89d8..3d264842ea 100644 --- a/cmd/fleetctl/flags.go +++ b/cmd/fleetctl/flags.go @@ -3,8 +3,9 @@ package main import "github.com/urfave/cli/v2" const ( - outfileFlagName = "outfile" - debugFlagName = "debug" + outfileFlagName = "outfile" + debugFlagName = "debug" + fleetCertificateFlagName = "fleet-certificate" ) func outfileFlag() cli.Flag { @@ -31,3 +32,15 @@ func debugFlag() cli.Flag { func getDebug(c *cli.Context) bool { return c.Bool(debugFlagName) } + +func fleetCertificateFlag() cli.Flag { + return &cli.StringFlag{ + Name: fleetCertificateFlagName, + EnvVars: []string{"FLEET_CERTIFICATE"}, + Usage: "Path of the TLS fleet certificate, can be used to provide additional connection debugging information", + } +} + +func getFleetCertificate(c *cli.Context) string { + return c.String(fleetCertificateFlagName) +} diff --git a/cmd/fleetctl/get.go b/cmd/fleetctl/get.go index 354013eab9..286a7d85d3 100644 --- a/cmd/fleetctl/get.go +++ b/cmd/fleetctl/get.go @@ -7,7 +7,7 @@ import ( "os" "strconv" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "gopkg.in/guregu/null.v3" "github.com/fleetdm/fleet/v4/server/fleet" diff --git a/ee/fleetctl/updates.go b/ee/fleetctl/updates.go index e8e60119cd..806f8f1f8c 100644 --- a/ee/fleetctl/updates.go +++ b/ee/fleetctl/updates.go @@ -11,7 +11,7 @@ import ( "syscall" "time" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/theupdateframework/go-tuf" "github.com/urfave/cli/v2" diff --git a/orbit/cmd/orbit/orbit.go b/orbit/cmd/orbit/orbit.go index dd166668a1..e8022c8c27 100644 --- a/orbit/cmd/orbit/orbit.go +++ b/orbit/cmd/orbit/orbit.go @@ -13,14 +13,14 @@ import ( "time" "github.com/dgraph-io/badger/v2" - "github.com/fleetdm/fleet/v4/orbit/pkg/certificate" "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/database" "github.com/fleetdm/fleet/v4/orbit/pkg/insecure" "github.com/fleetdm/fleet/v4/orbit/pkg/osquery" "github.com/fleetdm/fleet/v4/orbit/pkg/update" "github.com/fleetdm/fleet/v4/orbit/pkg/update/filestore" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/certificate" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/oklog/run" "github.com/pkg/errors" "github.com/rs/zerolog" diff --git a/orbit/cmd/orbit/shell.go b/orbit/cmd/orbit/shell.go index b1f8c3570c..e927376409 100644 --- a/orbit/cmd/orbit/shell.go +++ b/orbit/cmd/orbit/shell.go @@ -9,7 +9,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/osquery" "github.com/fleetdm/fleet/v4/orbit/pkg/update" "github.com/fleetdm/fleet/v4/orbit/pkg/update/filestore" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/oklog/run" "github.com/pkg/errors" "github.com/rs/zerolog" diff --git a/orbit/pkg/osquery/osquery.go b/orbit/pkg/osquery/osquery.go index 6ae15066d6..700a5d7037 100644 --- a/orbit/pkg/osquery/osquery.go +++ b/orbit/pkg/osquery/osquery.go @@ -10,7 +10,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/process" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/rs/zerolog/log" ) diff --git a/orbit/pkg/packaging/linux_shared.go b/orbit/pkg/packaging/linux_shared.go index 6262aed5da..71b25f9b41 100644 --- a/orbit/pkg/packaging/linux_shared.go +++ b/orbit/pkg/packaging/linux_shared.go @@ -9,7 +9,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/update" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/goreleaser/nfpm/v2" "github.com/goreleaser/nfpm/v2/files" "github.com/pkg/errors" diff --git a/orbit/pkg/packaging/macos.go b/orbit/pkg/packaging/macos.go index fd514b4614..1c3d930ae5 100644 --- a/orbit/pkg/packaging/macos.go +++ b/orbit/pkg/packaging/macos.go @@ -11,7 +11,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/update" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/rs/zerolog/log" ) diff --git a/orbit/pkg/packaging/packaging.go b/orbit/pkg/packaging/packaging.go index f89230f4ca..c859fe40fa 100644 --- a/orbit/pkg/packaging/packaging.go +++ b/orbit/pkg/packaging/packaging.go @@ -11,7 +11,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/update" "github.com/fleetdm/fleet/v4/orbit/pkg/update/filestore" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/rs/zerolog/log" ) diff --git a/orbit/pkg/packaging/windows.go b/orbit/pkg/packaging/windows.go index 8e315489b4..223bb70394 100644 --- a/orbit/pkg/packaging/windows.go +++ b/orbit/pkg/packaging/windows.go @@ -10,7 +10,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/packaging/wix" "github.com/fleetdm/fleet/v4/orbit/pkg/update" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/rs/zerolog/log" ) diff --git a/orbit/pkg/update/filestore/filestore.go b/orbit/pkg/update/filestore/filestore.go index 40f5630663..5479f9e0c8 100644 --- a/orbit/pkg/update/filestore/filestore.go +++ b/orbit/pkg/update/filestore/filestore.go @@ -5,7 +5,7 @@ import ( "os" "github.com/fleetdm/fleet/v4/orbit/pkg/constant" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/theupdateframework/go-tuf/client" ) diff --git a/orbit/pkg/update/update.go b/orbit/pkg/update/update.go index 5b7e6aa420..bcfbc47074 100644 --- a/orbit/pkg/update/update.go +++ b/orbit/pkg/update/update.go @@ -12,7 +12,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/constant" "github.com/fleetdm/fleet/v4/orbit/pkg/platform" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" "github.com/pkg/errors" "github.com/rs/zerolog/log" "github.com/theupdateframework/go-tuf/client" diff --git a/pkg/README.md b/pkg/README.md new file mode 100644 index 0000000000..be6a15939d --- /dev/null +++ b/pkg/README.md @@ -0,0 +1,3 @@ +# pkg directory + +This top-level `pkg` directory contains packages that may be shared between `fleet` and `orbit`. diff --git a/orbit/pkg/certificate/certificate.go b/pkg/certificate/certificate.go similarity index 54% rename from orbit/pkg/certificate/certificate.go rename to pkg/certificate/certificate.go index f704d61ec2..cfcc70cc57 100644 --- a/orbit/pkg/certificate/certificate.go +++ b/pkg/certificate/certificate.go @@ -2,6 +2,7 @@ package certificate import ( + "context" "crypto/tls" "crypto/x509" "io/ioutil" @@ -21,7 +22,7 @@ func LoadPEM(path string) (*x509.CertPool, error) { } if ok := pool.AppendCertsFromPEM(contents); !ok { - return nil, errors.Errorf("no valid ceritificates found in %s", path) + return nil, errors.Errorf("no valid certificates found in %s", path) } return pool, nil @@ -32,29 +33,39 @@ func LoadPEM(path string) (*x509.CertPool, error) { // not sufficient to verify authenticity of the server, but it can help to catch // certificate errors and provide more detailed messages to users. func ValidateConnection(pool *x509.CertPool, fleetURL string) error { + return ValidateConnectionContext(context.Background(), pool, fleetURL) +} + +// ValidateConnectionContext is like ValidateConnection, but it accepts a +// context that may specify a timeout or deadline for the TLS connection check. +func ValidateConnectionContext(ctx context.Context, pool *x509.CertPool, fleetURL string) error { parsed, err := url.Parse(fleetURL) if err != nil { return errors.Wrap(err, "parse url") } - conn, err := tls.Dial("tcp", parsed.Host, &tls.Config{ - ClientCAs: pool, - InsecureSkipVerify: true, - VerifyConnection: func(state tls.ConnectionState) error { - if len(state.PeerCertificates) == 0 { - return errors.New("no peer certificates") - } - cert := state.PeerCertificates[0] - if _, err := cert.Verify(x509.VerifyOptions{ - DNSName: parsed.Hostname(), - Roots: pool, - }); err != nil { - return errors.Wrap(err, "verify certificate") - } + dialer := &tls.Dialer{ + Config: &tls.Config{ + RootCAs: pool, + InsecureSkipVerify: true, + VerifyConnection: func(state tls.ConnectionState) error { + if len(state.PeerCertificates) == 0 { + return errors.New("no peer certificates") + } - return nil + cert := state.PeerCertificates[0] + if _, err := cert.Verify(x509.VerifyOptions{ + DNSName: parsed.Hostname(), + Roots: pool, + }); err != nil { + return errors.Wrap(err, "verify certificate") + } + + return nil + }, }, - }) + } + conn, err := dialer.DialContext(ctx, "tcp", parsed.Host) if err != nil { return errors.Wrap(err, "dial for validate") } diff --git a/orbit/pkg/certificate/certificate_test.go b/pkg/certificate/certificate_test.go similarity index 100% rename from orbit/pkg/certificate/certificate_test.go rename to pkg/certificate/certificate_test.go diff --git a/orbit/pkg/certificate/testdata/empty.crt b/pkg/certificate/testdata/empty.crt similarity index 100% rename from orbit/pkg/certificate/testdata/empty.crt rename to pkg/certificate/testdata/empty.crt diff --git a/orbit/pkg/certificate/testdata/test.crt b/pkg/certificate/testdata/test.crt similarity index 100% rename from orbit/pkg/certificate/testdata/test.crt rename to pkg/certificate/testdata/test.crt diff --git a/orbit/pkg/certificate/testdata/test.key b/pkg/certificate/testdata/test.key similarity index 100% rename from orbit/pkg/certificate/testdata/test.key rename to pkg/certificate/testdata/test.key diff --git a/secure/secure.go b/pkg/secure/secure.go similarity index 100% rename from secure/secure.go rename to pkg/secure/secure.go diff --git a/secure/secure_test.go b/pkg/secure/secure_test.go similarity index 100% rename from secure/secure_test.go rename to pkg/secure/secure_test.go diff --git a/server/logging/filesystem.go b/server/logging/filesystem.go index c936edc16e..f9aff5f53e 100644 --- a/server/logging/filesystem.go +++ b/server/logging/filesystem.go @@ -10,7 +10,7 @@ import ( "sync" "syscall" - "github.com/fleetdm/fleet/v4/secure" + "github.com/fleetdm/fleet/v4/pkg/secure" lumberjack "gopkg.in/natefinch/lumberjack.v2" "github.com/go-kit/kit/log" diff --git a/server/service/client.go b/server/service/client.go index 1b4bbeabd1..10767e5826 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -2,6 +2,7 @@ package service import ( "bytes" + "context" "crypto/tls" "crypto/x509" "encoding/json" @@ -55,7 +56,7 @@ func NewClient(addr string, insecureSkipVerify bool, rootCA, urlPrefix string, o // add certs to pool if ok := rootCAPool.AppendCertsFromPEM(certs); !ok { - return nil, errors.Wrap(err, "adding root CA") + return nil, errors.New("failed to add certificates to root CA pool") } } else if !insecureSkipVerify { // Use only the system certs (doesn't work on Windows) @@ -104,7 +105,7 @@ func EnableClientDebug() ClientOption { } } -func (c *Client) doWithHeaders(verb, path, rawQuery string, params interface{}, headers map[string]string) (*http.Response, error) { +func (c *Client) doContextWithHeaders(ctx context.Context, verb, path, rawQuery string, params interface{}, headers map[string]string) (*http.Response, error) { var bodyBytes []byte var err error if params != nil { @@ -114,7 +115,8 @@ func (c *Client) doWithHeaders(verb, path, rawQuery string, params interface{}, } } - request, err := http.NewRequest( + request, err := http.NewRequestWithContext( + ctx, verb, c.url(path, rawQuery).String(), bytes.NewBuffer(bodyBytes), @@ -130,12 +132,16 @@ func (c *Client) doWithHeaders(verb, path, rawQuery string, params interface{}, } func (c *Client) Do(verb, path, rawQuery string, params interface{}) (*http.Response, error) { + return c.DoContext(context.Background(), verb, path, rawQuery, params) +} + +func (c *Client) DoContext(ctx context.Context, verb, path, rawQuery string, params interface{}) (*http.Response, error) { headers := map[string]string{ "Content-type": "application/json", "Accept": "application/json", } - return c.doWithHeaders(verb, path, rawQuery, params, headers) + return c.doContextWithHeaders(ctx, verb, path, rawQuery, params, headers) } func (c *Client) AuthenticatedDo(verb, path, rawQuery string, params interface{}) (*http.Response, error) { @@ -149,7 +155,7 @@ func (c *Client) AuthenticatedDo(verb, path, rawQuery string, params interface{} "Authorization": fmt.Sprintf("Bearer %s", c.token), } - return c.doWithHeaders(verb, path, rawQuery, params, headers) + return c.doContextWithHeaders(context.Background(), verb, path, rawQuery, params, headers) } func (c *Client) SetToken(t string) {