diff --git a/cli/clitest/cli.go b/cli/clitest/cli.go index f6ff8cf..d288d74 100644 --- a/cli/clitest/cli.go +++ b/cli/clitest/cli.go @@ -49,7 +49,7 @@ func New(t *testing.T, cmd string, args ...string) (context.Context, *cobra.Comm var ( execer = NewFakeExecer() - fs = NewMemFS() + fs = xunixfake.NewMemFS() mnt = &mount.FakeMounter{} client = NewFakeDockerClient() iface = GetNetLink(t) diff --git a/cli/clitest/fake.go b/cli/clitest/fake.go index 025ddb2..048b804 100644 --- a/cli/clitest/fake.go +++ b/cli/clitest/fake.go @@ -8,7 +8,6 @@ import ( "strings" dockertypes "github.com/docker/docker/api/types" - "github.com/spf13/afero" testingexec "k8s.io/utils/exec/testing" "github.com/coder/envbox/dockerutil" @@ -16,13 +15,6 @@ import ( "github.com/coder/envbox/xunix/xunixfake" ) -func NewMemFS() *xunixfake.MemFS { - return &xunixfake.MemFS{ - MemMapFs: &afero.MemMapFs{}, - Owner: map[string]xunixfake.FileOwner{}, - } -} - func NewFakeExecer() *xunixfake.FakeExec { return &xunixfake.FakeExec{ Commands: map[string]*xunixfake.FakeCmd{}, diff --git a/cli/docker.go b/cli/docker.go index 921bcfc..a798ee4 100644 --- a/cli/docker.go +++ b/cli/docker.go @@ -157,20 +157,20 @@ func dockerCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) (err error) { var ( ctx = cmd.Context() - log = slog.Make(slogjson.Sink(io.Discard)) - blog buildlog.Logger = buildlog.NopLogger{} + log = slog.Make(slogjson.Sink(cmd.ErrOrStderr()), slogkubeterminate.Make()).Leveled(slog.LevelDebug) + blog buildlog.Logger = buildlog.JSONLogger{Encoder: json.NewEncoder(os.Stderr)} ) + if flags.noStartupLogs { + log = slog.Make(slogjson.Sink(io.Discard)) + blog = buildlog.NopLogger{} + } + httpClient, err := xhttp.Client(log, flags.extraCertsPath) if err != nil { return xerrors.Errorf("http client: %w", err) } - if !flags.noStartupLogs { - log = slog.Make(slogjson.Sink(cmd.ErrOrStderr()), slogkubeterminate.Make()).Leveled(slog.LevelDebug) - blog = buildlog.JSONLogger{Encoder: json.NewEncoder(os.Stderr)} - } - if !flags.noStartupLogs && flags.agentToken != "" && flags.coderURL != "" { coderURL, err := url.Parse(flags.coderURL) if err != nil { @@ -197,7 +197,6 @@ func dockerCmd() *cobra.Command { } }(&err) - blog.Info("Waiting for dockerd to startup...") sysboxArgs := []string{} if flags.disableIDMappedMount { sysboxArgs = append(sysboxArgs, "--disable-idmapped-mount") @@ -231,6 +230,7 @@ func dockerCmd() *cobra.Command { log.Debug(ctx, "starting dockerd", slog.F("args", args)) + blog.Info("Waiting for sysbox processes to startup...") dockerd := background.New(ctx, log, "dockerd", dargs...) err = dockerd.Start() if err != nil { @@ -289,11 +289,32 @@ func dockerCmd() *cobra.Command { // We wait for the daemon after spawning the goroutine in case // startup causes the daemon to encounter encounter a 'no space left // on device' error. + blog.Info("Waiting for dockerd to startup...") err = dockerutil.WaitForDaemon(ctx, client) if err != nil { return xerrors.Errorf("wait for dockerd: %w", err) } + if flags.extraCertsPath != "" { + // Parse the registry from the inner image + registry, err := name.ParseReference(flags.innerImage) + if err != nil { + return xerrors.Errorf("invalid image: %w", err) + } + registryName := registry.Context().RegistryStr() + + // Write certificates for the registry + err = dockerutil.WriteCertsForRegistry(ctx, registryName, flags.extraCertsPath) + if err != nil { + return xerrors.Errorf("write certs for registry: %w", err) + } + + blog.Infof("Successfully copied certificates from %q to %q", flags.extraCertsPath, filepath.Join("/etc/docker/certs.d", registryName)) + log.Debug(ctx, "wrote certificates for registry", slog.F("registry", registryName), + slog.F("extra_certs_path", flags.extraCertsPath), + ) + } + err = runDockerCVM(ctx, log, client, blog, flags) if err != nil { // It's possible we failed because we ran out of disk while diff --git a/dockerutil/image.go b/dockerutil/image.go index fb1cfaf..55e9d2d 100644 --- a/dockerutil/image.go +++ b/dockerutil/image.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "strings" "time" dockertypes "github.com/docker/docker/api/types" @@ -65,6 +66,14 @@ func PullImage(ctx context.Context, config *PullImageConfig) error { } err = pullImageFn() + // We should bail early if we're going to fail due to a + // certificate error. We can't xerrors.As here since this is + // returned from the daemon so the client is reporting + // essentially an unwrapped error. + if isTLSVerificationErr(err) { + return err + } + if err == nil { return nil } @@ -253,3 +262,7 @@ func DefaultLogImagePullFn(log buildlog.Logger) func(ImagePullEvent) error { return nil } } + +func isTLSVerificationErr(err error) bool { + return err != nil && strings.Contains(err.Error(), "tls: failed to verify certificate: x509: certificate signed by unknown authority") +} diff --git a/dockerutil/registry.go b/dockerutil/registry.go new file mode 100644 index 0000000..71bf814 --- /dev/null +++ b/dockerutil/registry.go @@ -0,0 +1,92 @@ +package dockerutil + +import ( + "context" + "io" + "path/filepath" + + "github.com/spf13/afero" + "golang.org/x/xerrors" + + "github.com/coder/envbox/xunix" +) + +// WriteCertsForRegistry writes the certificates found in the provided directory +// to the correct subdirectory that the Docker daemon uses when pulling images +// from the specified private registry. +func WriteCertsForRegistry(ctx context.Context, registryName, certsDir string) error { + fs := xunix.GetFS(ctx) + + // Docker certs directory. + registryCertsDir := filepath.Join("/etc/docker/certs.d", registryName) + + // If the directory already exists it means someone + // has either wrapped the image or has mounted in certs + // manually. We should assume the user knows what they're + // doing and avoid mucking with their solution. + if _, err := fs.Stat(registryCertsDir); err == nil { + return nil + } + + // Ensure the registry certs directory exists. + err := fs.MkdirAll(registryCertsDir, 0o755) + if err != nil { + return xerrors.Errorf("create registry certs directory: %w", err) + } + + // Check if certsDir is a file. + fileInfo, err := fs.Stat(certsDir) + if err != nil { + return xerrors.Errorf("stat certs directory/file: %w", err) + } + + if !fileInfo.IsDir() { + // If it's a file, copy it directly + err = copyCertFile(fs, certsDir, filepath.Join(registryCertsDir, "ca.crt")) + if err != nil { + return xerrors.Errorf("copy cert file: %w", err) + } + return nil + } + + // If it's a directory, copy all cert files in the root of the directory + entries, err := afero.ReadDir(fs, certsDir) + if err != nil { + return xerrors.Errorf("read certs directory: %w", err) + } + + for _, entry := range entries { + if entry.IsDir() { + continue + } + srcPath := filepath.Join(certsDir, entry.Name()) + dstPath := filepath.Join(registryCertsDir, entry.Name()) + err = copyCertFile(fs, srcPath, dstPath) + if err != nil { + return xerrors.Errorf("copy cert file %s: %w", entry.Name(), err) + } + } + + return nil +} + +func copyCertFile(fs xunix.FS, src, dst string) error { + srcFile, err := fs.Open(src) + if err != nil { + return xerrors.Errorf("open source file: %w", err) + } + defer srcFile.Close() + + dstFile, err := fs.Create(dst) + if err != nil { + return xerrors.Errorf("create destination file: %w", err) + } + defer dstFile.Close() + + _, err = io.Copy(dstFile, srcFile) + if err != nil { + return xerrors.Errorf("copy file contents: %w", err) + } + + return nil +} diff --git a/dockerutil/registry_test.go b/dockerutil/registry_test.go new file mode 100644 index 0000000..c58984e --- /dev/null +++ b/dockerutil/registry_test.go @@ -0,0 +1,100 @@ +package dockerutil_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/envbox/dockerutil" + "github.com/coder/envbox/xunix" + "github.com/coder/envbox/xunix/xunixfake" +) + +func TestWriteCertsForRegistry(t *testing.T) { + t.Parallel() + + t.Run("SingleCertFile", func(t *testing.T) { + t.Parallel() + // Test setup + fs := xunixfake.NewMemFS() + ctx := xunix.WithFS(context.Background(), fs) + + // Create a test certificate file + certContent := []byte("test certificate content") + err := afero.WriteFile(fs, "/certs/ca.crt", certContent, 0o644) + require.NoError(t, err) + + // Run the function + err = dockerutil.WriteCertsForRegistry(ctx, "test.registry.com", "/certs/ca.crt") + require.NoError(t, err) + + // Check the result + copiedContent, err := afero.ReadFile(fs, "/etc/docker/certs.d/test.registry.com/ca.crt") + require.NoError(t, err) + assert.Equal(t, certContent, copiedContent) + }) + + t.Run("MultipleCertFiles", func(t *testing.T) { + t.Parallel() + // Test setup + fs := xunixfake.NewMemFS() + ctx := xunix.WithFS(context.Background(), fs) + + // Create test certificate files + certFiles := []string{"ca.crt", "client.cert", "client.key"} + for _, file := range certFiles { + err := afero.WriteFile(fs, filepath.Join("/certs", file), []byte("content of "+file), 0o644) + require.NoError(t, err) + } + + // Run the function + err := dockerutil.WriteCertsForRegistry(ctx, "test.registry.com", "/certs") + require.NoError(t, err) + + // Check the results + for _, file := range certFiles { + copiedContent, err := afero.ReadFile(fs, filepath.Join("/etc/docker/certs.d/test.registry.com", file)) + require.NoError(t, err) + assert.Equal(t, []byte("content of "+file), copiedContent) + } + }) + t.Run("ExistingRegistryCertsDir", func(t *testing.T) { + t.Parallel() + // Test setup + fs := xunixfake.NewMemFS() + ctx := xunix.WithFS(context.Background(), fs) + + // Create an existing registry certs directory + registryCertsDir := "/etc/docker/certs.d/test.registry.com" + err := fs.MkdirAll(registryCertsDir, 0o755) + require.NoError(t, err) + + // Create a file in the existing directory + existingContent := []byte("existing certificate content") + err = afero.WriteFile(fs, filepath.Join(registryCertsDir, "existing.crt"), existingContent, 0o644) + require.NoError(t, err) + + // Create a test certificate file in the source directory + certContent := []byte("new certificate content") + err = afero.WriteFile(fs, "/certs/ca.crt", certContent, 0o644) + require.NoError(t, err) + + // Run the function + err = dockerutil.WriteCertsForRegistry(ctx, "test.registry.com", "/certs") + require.NoError(t, err) + + // Check that the existing file was not modified + existingFileContent, err := afero.ReadFile(fs, filepath.Join(registryCertsDir, "existing.crt")) + require.NoError(t, err) + assert.Equal(t, existingContent, existingFileContent) + + // Check that the new file was not copied + _, err = fs.Stat(filepath.Join(registryCertsDir, "ca.crt")) + assert.True(t, os.IsNotExist(err), "New certificate file should not have been copied") + }) +} diff --git a/integration/docker_test.go b/integration/docker_test.go index 515d234..54856bd 100644 --- a/integration/docker_test.go +++ b/integration/docker_test.go @@ -327,11 +327,6 @@ func TestDocker(t *testing.T) { TLSPort: strconv.Itoa(registryAddr.Port), }) - // Mount the cert into the expected location - // for the Envbox Docker daemon. - regCAPath := filepath.Join("/etc/docker/certs.d", image.Registry(), "ca.crt") - registryCAMount := integrationtest.BindMount(regCertPath, regCAPath, false) - envs := []string{ integrationtest.EnvVar(cli.EnvAgentToken, "faketoken"), integrationtest.EnvVar(cli.EnvAgentURL, fmt.Sprintf("https://%s:%d", "host.docker.internal", coderAddr.Port)), @@ -343,13 +338,152 @@ func TestDocker(t *testing.T) { Image: image.String(), Username: "coder", Envs: envs, - OuterMounts: append(binds, coderCertMount, registryCAMount), + OuterMounts: append(binds, coderCertMount), }) // This indicates we've made it all the way to end // of the logs we attempt to push. require.True(t, recorder.ContainsLog("Bootstrapping workspace...")) }) + + // This tests the inverse of SelfSignedCerts. We assert that + // the container fails to startup since we don't have a valid + // cert for the registry. It mainly tests that we aren't + // getting a false positive for SelfSignedCerts. + t.Run("InvalidCert", func(t *testing.T) { + t.Parallel() + + var ( + dir = integrationtest.TmpDir(t) + binds = integrationtest.DefaultBinds(t, dir) + ) + + pool, err := dockertest.NewPool("") + require.NoError(t, err) + + // Create some listeners for the Docker and Coder + // services we'll be running with self signed certs. + bridgeIP := integrationtest.DockerBridgeIP(t) + coderListener, err := net.Listen("tcp", fmt.Sprintf("%s:0", bridgeIP)) + require.NoError(t, err) + defer coderListener.Close() + coderAddr := tcpAddr(t, coderListener) + + registryListener, err := net.Listen("tcp", fmt.Sprintf("%s:0", bridgeIP)) + require.NoError(t, err) + err = registryListener.Close() + require.NoError(t, err) + registryAddr := tcpAddr(t, registryListener) + + // Generate a random cert so the same codepaths + // get triggered. + randomCert := integrationtest.GenerateTLSCertificate(t, "host.docker.internal", coderAddr.IP.String()) + // Generate a cert for the registry. We are intentionally + // not passing this to envbox. + registryCert := integrationtest.GenerateTLSCertificate(t, "host.docker.internal", registryAddr.IP.String()) + + certDir := integrationtest.MkdirAll(t, dir, "certs") + registryCertDir := integrationtest.MkdirAll(t, dir, "registry_certs") + + // Write the Coder cert disk. + coderCertPath := filepath.Join(certDir, "random_cert.pem") + coderKeyPath := filepath.Join(certDir, "random_key.pem") + integrationtest.WriteCertificate(t, randomCert, coderCertPath, coderKeyPath) + coderCertMount := integrationtest.BindMount(certDir, "/tmp/certs", false) + + // Write the Registry cert to disk in a separate directory. + regCertPath := filepath.Join(registryCertDir, "registry_cert.crt") + regKeyPath := filepath.Join(registryCertDir, "registry_key.pem") + integrationtest.WriteCertificate(t, registryCert, regCertPath, regKeyPath) + + // Start up the docker registry and push an image + // to it that we can reference. + image := integrationtest.RunLocalDockerRegistry(t, pool, integrationtest.RegistryConfig{ + HostCertPath: regCertPath, + HostKeyPath: regKeyPath, + Image: integrationtest.UbuntuImage, + TLSPort: strconv.Itoa(registryAddr.Port), + }) + + envs := []string{ + integrationtest.EnvVar(cli.EnvExtraCertsPath, "/tmp/certs"), + } + + // Run the envbox container. + _ = integrationtest.RunEnvbox(t, pool, &integrationtest.CreateDockerCVMConfig{ + Image: image.String(), + Username: "coder", + Envs: envs, + OuterMounts: append(binds, coderCertMount), + ExpectFailure: true, + }) + }) + + // InvalidCoderCert tests that an invalid cert + // for the Coder control plane does not result in a + // fatal error. The container should still start up + // but we won't receive any build logs. + t.Run("InvalidCoderCert", func(t *testing.T) { + t.Parallel() + + var ( + dir = integrationtest.TmpDir(t) + binds = integrationtest.DefaultBinds(t, dir) + ) + + pool, err := dockertest.NewPool("") + require.NoError(t, err) + + // Create a listener for the Coder service with a self-signed cert. + bridgeIP := integrationtest.DockerBridgeIP(t) + coderListener, err := net.Listen("tcp", fmt.Sprintf("%s:0", bridgeIP)) + require.NoError(t, err) + defer coderListener.Close() + coderAddr := tcpAddr(t, coderListener) + + coderCert := integrationtest.GenerateTLSCertificate(t, "host.docker.internal", coderAddr.IP.String()) + fakeCert := integrationtest.GenerateTLSCertificate(t, "host.docker.internal", coderAddr.IP.String()) + + // Startup our fake Coder "control-plane". + recorder := integrationtest.FakeBuildLogRecorder(t, coderListener, coderCert) + + certDir := integrationtest.MkdirAll(t, dir, "certs") + + // This is all a little unnecessary we could just not + // write one but let's fully simulate someone mounting + // a bad cert. + fakeCertPath := filepath.Join(certDir, "fake_cert.pem") + fakeKeyPath := filepath.Join(certDir, "fake_key.pem") + integrationtest.WriteCertificate(t, fakeCert, fakeCertPath, fakeKeyPath) + coderCertMount := integrationtest.BindMount(certDir, "/tmp/certs", false) + + envs := []string{ + integrationtest.EnvVar(cli.EnvAgentToken, "faketoken"), + integrationtest.EnvVar(cli.EnvAgentURL, fmt.Sprintf("https://%s:%d", "host.docker.internal", coderAddr.Port)), + integrationtest.EnvVar(cli.EnvExtraCertsPath, "/tmp/certs"), + } + + // Run the envbox container. + resource := integrationtest.RunEnvbox(t, pool, &integrationtest.CreateDockerCVMConfig{ + Image: integrationtest.UbuntuImage, + Username: "coder", + Envs: envs, + OuterMounts: append(binds, coderCertMount), + }) + + // This indicates we've made it all the way to end + // of the logs we attempt to push. + require.Equal(t, 0, recorder.Len()) + + // Sanity check that we're actually running. + output, err := integrationtest.ExecInnerContainer(t, pool, integrationtest.ExecConfig{ + ContainerID: resource.Container.ID, + Cmd: []string{"echo", "hello"}, + User: "root", + }) + require.NoError(t, err) + require.Equal(t, "hello\n", string(output)) + }) } func requireSliceNoContains(t *testing.T, ss []string, els ...string) { diff --git a/integration/integrationtest/coder.go b/integration/integrationtest/coder.go index c3eb5d9..326232e 100644 --- a/integration/integrationtest/coder.go +++ b/integration/integrationtest/coder.go @@ -31,6 +31,12 @@ func (b *BuildLogRecorder) ContainsLog(l string) bool { return false } +func (b *BuildLogRecorder) Len() int { + b.mu.Lock() + defer b.mu.Unlock() + return len(b.logs) +} + func (b *BuildLogRecorder) append(log string) { b.mu.Lock() defer b.mu.Unlock() diff --git a/integration/integrationtest/docker.go b/integration/integrationtest/docker.go index 86b8a00..f24393c 100644 --- a/integration/integrationtest/docker.go +++ b/integration/integrationtest/docker.go @@ -55,10 +55,11 @@ type CreateDockerCVMConfig struct { InnerEnvFilter []string Envs []string - OuterMounts []docker.HostMount - AddFUSE bool - AddTUN bool - CPUs int + OuterMounts []docker.HostMount + AddFUSE bool + AddTUN bool + CPUs int + ExpectFailure bool } func (c CreateDockerCVMConfig) validate(t *testing.T) { @@ -73,13 +74,6 @@ func (c CreateDockerCVMConfig) validate(t *testing.T) { } } -type CoderdOptions struct { - TLSEnable bool - TLSCert string - TLSKey string - DefaultImage string -} - // RunEnvbox runs envbox, it returns once the inner container has finished // spinning up. func RunEnvbox(t *testing.T, pool *dockertest.Pool, conf *CreateDockerCVMConfig) *dockertest.Resource { @@ -109,9 +103,15 @@ func RunEnvbox(t *testing.T, pool *dockertest.Pool, conf *CreateDockerCVMConfig) host.ExtraHosts = []string{"host.docker.internal:host-gateway"} }) require.NoError(t, err) - // t.Cleanup(func() { _ = pool.Purge(resource) }) - waitForCVM(t, pool, resource) + t.Cleanup(func() { + if !t.Failed() { + _ = pool.Purge(resource) + } + }) + + success := waitForCVM(t, pool, resource) + require.Equal(t, !conf.ExpectFailure, success, "expected success=%v but detected %v", !conf.ExpectFailure, success) return resource } @@ -203,13 +203,13 @@ func WaitForCVMDocker(t *testing.T, pool *dockertest.Pool, resource *dockertest. } // waitForCVM waits for the inner container to spin up. -func waitForCVM(t *testing.T, pool *dockertest.Pool, resource *dockertest.Resource) { +func waitForCVM(t *testing.T, pool *dockertest.Pool, resource *dockertest.Resource) bool { t.Helper() rd, wr := io.Pipe() defer rd.Close() defer wr.Close() - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5) defer cancel() go func() { defer wr.Close() @@ -245,11 +245,13 @@ func waitForCVM(t *testing.T, pool *dockertest.Pool, resource *dockertest.Resour } if blog.Type == buildlog.JSONLogTypeError { - t.Fatalf("envbox failed (%s)", blog.Output) + t.Logf("envbox failed (%s)", blog.Output) + return false } } require.NoError(t, scanner.Err()) require.True(t, finished, "unexpected logger exit") + return true } type ExecConfig struct { @@ -403,6 +405,12 @@ func RunLocalDockerRegistry(t testing.TB, pool *dockertest.Pool, conf RegistryCo }) require.NoError(t, err) + t.Cleanup(func() { + if !t.Failed() { + _ = pool.Purge(resource) + } + }) + host := net.JoinHostPort("0.0.0.0", conf.TLSPort) url := fmt.Sprintf("https://%s/v2/_catalog", host) diff --git a/xunix/xunixfake/fs.go b/xunix/xunixfake/fs.go index 36eb2ad..4a54fac 100644 --- a/xunix/xunixfake/fs.go +++ b/xunix/xunixfake/fs.go @@ -14,6 +14,13 @@ type FileOwner struct { GID int } +func NewMemFS() *MemFS { + return &MemFS{ + MemMapFs: &afero.MemMapFs{}, + Owner: map[string]FileOwner{}, + } +} + type MemFS struct { *afero.MemMapFs Owner map[string]FileOwner