diff options
author | Nick Thomas <nick@gitlab.com> | 2018-03-21 20:29:55 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-03-21 20:29:55 +0300 |
commit | 5987070946907a8d3411869ca166fd192d261b65 (patch) | |
tree | 8c222620310e6bb5a967f099314e40428e920c73 | |
parent | fe1561978ed164220e471129c9b2fa6b89d07992 (diff) | |
parent | 51f0df18e3d8dd5f1e0faeea3b2a41e6ff73f551 (diff) |
Merge branch 'ac/dns-and-ssl' into 'master'
Add /etc/resolv.conf and /etc/ssl/certs to pages chroot
Closes #86
See merge request gitlab-org/gitlab-pages!51
-rw-r--r-- | README.md | 11 | ||||
-rw-r--r-- | acceptance_test.go | 16 | ||||
-rw-r--r-- | daemon.go | 113 | ||||
-rw-r--r-- | helpers_test.go | 12 | ||||
-rw-r--r-- | internal/jail/jail.go | 137 | ||||
-rw-r--r-- | internal/jail/jail_test.go | 236 | ||||
-rw-r--r-- | internal/jail/mount_linux.go | 32 | ||||
-rw-r--r-- | internal/jail/mount_not_supported.go | 23 |
8 files changed, 507 insertions, 73 deletions
@@ -89,8 +89,13 @@ To enter this mode, run `gitlab-pages` as the root user and pass it the as. The daemon starts listening on ports and reads certificates as root, then -re-executes itself as the specified user. When re-executing it copies its own -binary to `pages-root` and changes root to that directory. +re-executes itself as the specified user. When re-executing it creates a chroot jail +containing a copy of its own binary, `/etc/resolv.conf`, and a bind mount of `pages-root`. + +When `-artifacts-server` points to an HTTPS URL we also need a list of certificates for +the trusted Certification Authorities to copy inside the jail. +A file containing such list can be specified using `SSL_CERT_FILE` environment variable. +(`SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt` on Debian) This make it possible to listen on privileged ports and makes it harder for the process to read files outside of `pages-root`. @@ -101,6 +106,8 @@ $ make $ sudo ./gitlab-pages -listen-http ":80" -pages-root path/to/gitlab/shared/pages -pages-domain example.com -daemon-uid 1000 -daemon-gid 1000 ``` +Please note that changes to `/etc/resolv.conf` or `SSL_CERT_FILE` will be ignored by `gitlab-pages` until restarted. + ### Listen on multiple ports Each of the `listen-http`, `listen-https` and `listen-proxy` arguments can be diff --git a/acceptance_test.go b/acceptance_test.go index f1e5a04e..a7547530 100644 --- a/acceptance_test.go +++ b/acceptance_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/pem" "fmt" "io/ioutil" "mime" @@ -350,7 +351,7 @@ func TestArtifactProxyRequest(t *testing.T) { content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>" contentLength := int64(len(content)) - testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.RawPath { case "/api/v4/projects/group%2Fproject/jobs/1/artifacts/delayed_200.html": time.Sleep(2 * time.Second) @@ -371,6 +372,17 @@ func TestArtifactProxyRequest(t *testing.T) { } })) defer testServer.Close() + + require.NotEmpty(t, testServer.TLS.Certificates, "testserver must implement TLS") + require.NotEmpty(t, testServer.TLS.Certificates[0].Certificate, "testserver TLS config has no certificates") + artifactsCert := testServer.TLS.Certificates[0].Certificate[0] + pemCert, err := ioutil.TempFile("", "test-server-cert") + require.NoError(t, err) + defer os.Remove(pemCert.Name()) + err = pem.Encode(pemCert, &pem.Block{Type: "CERTIFICATE", Bytes: artifactsCert}) + require.NoError(t, err) + pemCert.Close() + cases := []struct { Host string Path string @@ -441,7 +453,7 @@ func TestArtifactProxyRequest(t *testing.T) { for _, c := range cases { t.Run(fmt.Sprintf("Proxy Request Test: %s", c.Description), func(t *testing.T) { - teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-artifacts-server="+testServer.URL+"/api/v4", c.BinaryOption) + teardown := RunPagesProcessWithSSLCertFile(t, *pagesBinary, listeners, "", pemCert.Name(), "-artifacts-server="+testServer.URL+"/api/v4", c.BinaryOption) defer teardown() resp, err := GetPageFromListener(t, httpListener, c.Host, c.Path) require.NoError(t, err) @@ -1,21 +1,23 @@ package main import ( - "crypto/rand" "encoding/json" - "fmt" - "io" "os" "os/exec" "os/signal" - "path/filepath" "syscall" "github.com/kardianos/osext" log "github.com/sirupsen/logrus" + + "gitlab.com/gitlab-org/gitlab-pages/internal/jail" ) -const daemonRunProgram = "gitlab-pages-unprivileged" +const ( + daemonRunProgram = "gitlab-pages-unprivileged" + + pagesRootInChroot = "/pages" +) func daemonMain() { if os.Args[0] != daemonRunProgram { @@ -100,77 +102,55 @@ func passSignals(cmd *exec.Cmd) { }() } -func copyFile(dest, src string, perm os.FileMode) (err error) { - srcFile, err := os.Open(src) - if err != nil { - return - } - defer srcFile.Close() - - destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE|os.O_EXCL, perm) - if err != nil { - return - } - defer destFile.Close() +func daemonChroot(cmd *exec.Cmd) (*jail.Jail, error) { + cage := jail.TimestampedJail("gitlab-pages", 0755) - _, err = io.Copy(destFile, srcFile) - return -} - -func daemonCreateExecutable(name string, perm os.FileMode) (file *os.File, err error) { - // We assume that crypto random generates true random, non-colliding hash - b := make([]byte, 16) - _, err = rand.Read(b) + wd, err := os.Getwd() if err != nil { - return + return nil, err } - path := fmt.Sprintf("%s.%x", name, b) - file, err = os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, perm) + // Add gitlab-pages inside the jail + err = cage.CopyTo("/gitlab-pages", cmd.Path) if err != nil { - return + return nil, err } - return -} -func daemonChroot(cmd *exec.Cmd) (path string, err error) { - wd, err := os.Getwd() + // Add /etc/resolv.conf inside the jail + cage.MkDir("/etc", 0755) + err = cage.Copy("/etc/resolv.conf") if err != nil { - return + return nil, err } - // Generate temporary file - temporaryExecutable, err := daemonCreateExecutable(".daemon", 0755) - if err != nil { - return - } - defer temporaryExecutable.Close() - defer func() { - // Remove the temporary file in case of failure + // Copy SSL_CERT_FILE inside the jail + sslCertFile := os.Getenv("SSL_CERT_FILE") + if sslCertFile != "" { + cage.MkDir("/etc/ssl", 0755) + err = cage.CopyTo("/etc/ssl/ca-bundle.pem", sslCertFile) if err != nil { - os.Remove(temporaryExecutable.Name()) + return nil, err } - }() - - // Open current executable - executableFile, err := os.Open(cmd.Path) - if err != nil { - return + cmd.Env = append(os.Environ(), "SSL_CERT_FILE=/etc/ssl/ca-bundle.pem") + } else { + log.Print("Missing SSL_CERT_FILE environment variable. HTTPS requests will fail") } - defer executableFile.Close() - // Copy the executable content - _, err = io.Copy(temporaryExecutable, executableFile) + // Bind mount shared folder + cage.MkDir(pagesRootInChroot, 0755) + cage.Bind(pagesRootInChroot, wd) + + // Update command to use chroot + cmd.SysProcAttr.Chroot = cage.Path() + cmd.Path = "/gitlab-pages" + cmd.Dir = pagesRootInChroot + + err = cage.Build() if err != nil { - return + return nil, err } - // Update command to use chroot - cmd.SysProcAttr.Chroot = wd - cmd.Path = temporaryExecutable.Name() - cmd.Dir = "/" - path = filepath.Join(wd, temporaryExecutable.Name()) - return + return cage, nil } func daemonize(config appConfig, uid, gid uint) { @@ -193,12 +173,12 @@ func daemonize(config appConfig, uid, gid uint) { defer killProcess(cmd) // Run daemon in chroot environment - temporaryExecutable, err := daemonChroot(cmd) + chroot, err := daemonChroot(cmd) if err != nil { - log.WithError(err).Error("chroot failed") + log.WithError(err).Print("chroot failed") return } - defer os.Remove(temporaryExecutable) + defer chroot.Dispose() // Create a pipe to pass the configuration configReader, configWriter, err := os.Pipe() @@ -222,15 +202,18 @@ func daemonize(config appConfig, uid, gid uint) { return } + //detach binded mountpoints + if err = chroot.LazyUnbind(); err != nil { + log.WithError(err).Print("chroot lazy umount failed") + return + } + // Write the configuration if err = json.NewEncoder(configWriter).Encode(config); err != nil { return } configWriter.Close() - // Remove executable - os.Remove(temporaryExecutable) - // Pass through signals passSignals(cmd) diff --git a/helpers_test.go b/helpers_test.go index 44fc3797..a8d804c2 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -161,20 +161,24 @@ func (l ListenSpec) JoinHostPort() string { // // If run as root via sudo, the gitlab-pages process will drop privileges func RunPagesProcess(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string, extraArgs ...string) (teardown func()) { - return runPagesProcess(t, true, pagesPath, listeners, promPort, extraArgs...) + return runPagesProcess(t, true, pagesPath, listeners, promPort, nil, extraArgs...) } func RunPagesProcessWithoutWait(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string, extraArgs ...string) (teardown func()) { - return runPagesProcess(t, false, pagesPath, listeners, promPort, extraArgs...) + return runPagesProcess(t, false, pagesPath, listeners, promPort, nil, extraArgs...) } -func runPagesProcess(t *testing.T, wait bool, pagesPath string, listeners []ListenSpec, promPort string, extraArgs ...string) (teardown func()) { +func RunPagesProcessWithSSLCertFile(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string, sslCertFile string, extraArgs ...string) (teardown func()) { + return runPagesProcess(t, true, pagesPath, listeners, promPort, []string{"SSL_CERT_FILE=" + sslCertFile}, extraArgs...) +} + +func runPagesProcess(t *testing.T, wait bool, pagesPath string, listeners []ListenSpec, promPort string, extraEnv []string, extraArgs ...string) (teardown func()) { _, err := os.Stat(pagesPath) require.NoError(t, err) args, tempfiles := getPagesArgs(t, listeners, promPort, extraArgs) cmd := exec.Command(pagesPath, args...) - cmd.Env = os.Environ() + cmd.Env = append(os.Environ(), extraEnv...) cmd.Stdout = &tWriter{t} cmd.Stderr = &tWriter{t} require.NoError(t, cmd.Start()) diff --git a/internal/jail/jail.go b/internal/jail/jail.go new file mode 100644 index 00000000..745ccb2a --- /dev/null +++ b/internal/jail/jail.go @@ -0,0 +1,137 @@ +package jail + +import ( + "fmt" + "io" + "os" + "path" + "time" +) + +type pathAndMode struct { + path string + mode os.FileMode +} + +// Jail is a Chroot jail builder +type Jail struct { + directories []pathAndMode + files map[string]pathAndMode + bindMounts map[string]string +} + +// New returns a Jail for path +func New(path string, perm os.FileMode) *Jail { + return &Jail{ + directories: []pathAndMode{pathAndMode{path: path, mode: perm}}, + files: make(map[string]pathAndMode), + bindMounts: make(map[string]string), + } +} + +// TimestampedJail return a Jail with Path composed by prefix and current timestamp +func TimestampedJail(prefix string, perm os.FileMode) *Jail { + jailPath := path.Join(os.TempDir(), fmt.Sprintf("%s-%d", prefix, time.Now().UnixNano())) + + return New(jailPath, perm) +} + +// Path returns the path of the jail +func (j *Jail) Path() string { + return j.directories[0].path +} + +// Build creates the jail, making directories and copying files +func (j *Jail) Build() error { + for _, dir := range j.directories { + if err := os.Mkdir(dir.path, dir.mode); err != nil { + return fmt.Errorf("Can't create directory %q. %s", dir.path, err) + } + } + + for dest, src := range j.files { + if err := copyFile(dest, src.path, src.mode); err != nil { + return fmt.Errorf("Can't copy %q -> %q. %s", src.path, dest, err) + } + } + + return j.mount() +} + +// Dispose erases everything inside the jail +func (j *Jail) Dispose() error { + err := j.unmount() + if err != nil { + return err + } + + err = os.RemoveAll(j.Path()) + if err != nil { + return fmt.Errorf("Can't delete jail %q. %s", j.Path(), err) + } + + return nil +} + +// MkDir enqueue a mkdir operation at jail building time +func (j *Jail) MkDir(path string, perm os.FileMode) { + j.directories = append(j.directories, pathAndMode{path: j.ExternalPath(path), mode: perm}) +} + +// CopyTo enqueues a file copy operation at jail building time +func (j *Jail) CopyTo(dest, src string) error { + fi, err := os.Stat(src) + if err != nil { + return fmt.Errorf("Can't stat %q. %s", src, err) + } + + if fi.IsDir() { + return fmt.Errorf("Can't copy directories. %s", src) + } + + jailedDest := j.ExternalPath(dest) + j.files[jailedDest] = pathAndMode{ + path: src, + mode: fi.Mode(), + } + + return nil +} + +// Copy enqueues a file copy operation at jail building time +func (j *Jail) Copy(path string) error { + return j.CopyTo(path, path) +} + +// Bind enqueues a bind mount operation at jail building time +func (j *Jail) Bind(dest, src string) { + jailedDest := j.ExternalPath(dest) + j.bindMounts[jailedDest] = src +} + +// LazyUnbind detaches all binded mountpoints +func (j *Jail) LazyUnbind() error { + return j.unmount() +} + +// ExternalPath converts a jail internal path to the equivalent jail external path +func (j *Jail) ExternalPath(internal string) string { + return path.Join(j.Path(), internal) +} + +func copyFile(dest, src string, perm os.FileMode) error { + srcFile, err := os.Open(src) + if err != nil { + return err + } + defer srcFile.Close() + + destFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm) + if err != nil { + return err + } + defer destFile.Close() + + _, err = io.Copy(destFile, srcFile) + return err +} diff --git a/internal/jail/jail_test.go b/internal/jail/jail_test.go new file mode 100644 index 00000000..2095000e --- /dev/null +++ b/internal/jail/jail_test.go @@ -0,0 +1,236 @@ +package jail_test + +import ( + "fmt" + "io/ioutil" + "os" + "path" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/jail" +) + +func tmpJailPath() string { + return path.Join(os.TempDir(), fmt.Sprintf("my-jail-%d", time.Now().Unix())) +} + +func TestTimestampedJails(t *testing.T) { + assert := assert.New(t) + + prefix := "jail" + var mode os.FileMode = 0755 + + j1 := jail.TimestampedJail(prefix, mode) + j2 := jail.TimestampedJail(prefix, mode) + + assert.NotEqual(j1.Path, j2.Path()) +} + +func TestJailPath(t *testing.T) { + assert := assert.New(t) + + jailPath := tmpJailPath() + cage := jail.New(jailPath, 0755) + + assert.Equal(jailPath, cage.Path()) +} + +func TestJailBuild(t *testing.T) { + assert := assert.New(t) + + jailPath := tmpJailPath() + cage := jail.New(jailPath, 0755) + + _, err := os.Stat(cage.Path()) + assert.Error(err, "Jail path should not exist before Jail.Build()") + + err = cage.Build() + assert.NoError(err) + defer cage.Dispose() + + _, err = os.Stat(cage.Path()) + assert.NoError(err, "Jail path should exist after Jail.Build()") +} + +func TestJailDispose(t *testing.T) { + assert := assert.New(t) + + jailPath := tmpJailPath() + cage := jail.New(jailPath, 0755) + + err := cage.Build() + assert.NoError(err) + + err = cage.Dispose() + assert.NoError(err) + + _, err = os.Stat(cage.Path()) + assert.Error(err, "Jail path should not exist after Jail.Dispose()") +} + +func TestJailDisposeDoNotFailOnMissingPath(t *testing.T) { + assert := assert.New(t) + + jailPath := tmpJailPath() + cage := jail.New(jailPath, 0755) + + _, err := os.Stat(cage.Path()) + assert.Error(err, "Jail path should not exist") + + err = cage.Dispose() + assert.NoError(err) +} + +func TestJailWithFiles(t *testing.T) { + tests := []struct { + name string + directories []string + files []string + error bool + }{ + { + name: "Happy path", + directories: []string{"/tmp", "/tmp/foo", "/bar"}, + }, + { + name: "Missing direcories in path", + directories: []string{"/tmp/foo/bar"}, + error: true, + }, + { + name: "copy /etc/resolv.conf", + directories: []string{"/etc"}, + files: []string{"/etc/resolv.conf"}, + }, + { + name: "copy /etc/resolv.conf without creating /etc", + files: []string{"/etc/resolv.conf"}, + error: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert := assert.New(t) + + cage := jail.TimestampedJail("jail-mkdir", 0755) + for _, dir := range test.directories { + cage.MkDir(dir, 0755) + } + for _, file := range test.files { + if err := cage.Copy(file); err != nil { + t.Errorf("Can't prepare copy of %s inside the jail. %s", file, err) + } + } + + err := cage.Build() + defer cage.Dispose() + + if test.error { + assert.Error(err) + } else { + assert.NoError(err) + + for _, dir := range test.directories { + _, err := os.Stat(path.Join(cage.Path(), dir)) + assert.NoError(err, "jailed dir should exist") + } + + for _, file := range test.files { + _, err := os.Stat(path.Join(cage.Path(), file)) + assert.NoError(err, "Jailed file should exist") + } + } + }) + } +} + +func TestJailCopyTo(t *testing.T) { + assert := assert.New(t) + + content := "hello" + + cage := jail.TimestampedJail("check-file-copy", 0755) + + tmpFile, err := ioutil.TempFile("", "dummy-file") + if err != nil { + t.Error("Can't create temporary file") + } + defer os.Remove(tmpFile.Name()) + tmpFile.WriteString(content) + + filePath := tmpFile.Name() + jailedFilePath := cage.ExternalPath(path.Base(filePath)) + + err = cage.CopyTo(path.Base(filePath), filePath) + assert.NoError(err) + + err = cage.Build() + defer cage.Dispose() + assert.NoError(err) + + jailedFI, err := os.Stat(jailedFilePath) + assert.NoError(err) + + fi, err := os.Stat(filePath) + assert.NoError(err) + + assert.Equal(fi.Mode(), jailedFI.Mode(), "jailed file should preserve file mode") + assert.Equal(fi.Size(), jailedFI.Size(), "jailed file should have same size of original file") + + jailedContent, err := ioutil.ReadFile(jailedFilePath) + assert.NoError(err) + assert.Equal(content, string(jailedContent), "jailed file should preserve file content") +} + +func TestJailLazyUnbind(t *testing.T) { + if os.Geteuid() != 0 || runtime.GOOS != "linux" { + t.Skip("chroot binding requires linux and root permissions") + } + + assert := assert.New(t) + + toBind, err := ioutil.TempDir("", "to-bind") + require.NoError(t, err) + defer os.RemoveAll(toBind) + + tmpFilePath := path.Join(toBind, "a-file") + tmpFile, err := os.OpenFile(tmpFilePath, os.O_CREATE, 0644) + require.NoError(t, err) + tmpFile.Close() + + jailPath := tmpJailPath() + cage := jail.New(jailPath, 0755) + + cage.MkDir("/my-bind", 0755) + cage.Bind("/my-bind", toBind) + + err = cage.Build() + assert.NoError(err, "jail build failed") + + bindedTmpFilePath := cage.ExternalPath("/my-bind/a-file") + f, err := os.Open(bindedTmpFilePath) + assert.NoError(err, "temporary file not binded") + require.NotNil(t, f) + + err = cage.LazyUnbind() + assert.NoError(err, "lazy unbind failed") + + f.Close() + _, err = os.Stat(bindedTmpFilePath) + assert.Error(err, "lazy unbind should remove mount-point after file close") + + err = cage.Dispose() + assert.NoError(err, "dispose failed") + + _, err = os.Stat(cage.Path()) + assert.Error(err, "Jail path should not exist after Jail.Dispose()") + + _, err = os.Stat(tmpFilePath) + assert.NoError(err, "disposing a jail should not delete files under binded directories") +} diff --git a/internal/jail/mount_linux.go b/internal/jail/mount_linux.go new file mode 100644 index 00000000..f285f8b4 --- /dev/null +++ b/internal/jail/mount_linux.go @@ -0,0 +1,32 @@ +package jail + +import ( + "fmt" + + "golang.org/x/sys/unix" +) + +func (j *Jail) mount() error { + for dest, src := range j.bindMounts { + var opts uintptr = unix.MS_BIND | unix.MS_REC + if err := unix.Mount(src, dest, "none", opts, ""); err != nil { + return fmt.Errorf("Failed to bind mount %s on %s. %s", src, dest, err) + } + } + + return nil +} + +func (j *Jail) unmount() error { + for dest := range j.bindMounts { + if err := unix.Unmount(dest, unix.MNT_DETACH); err != nil { + // A second invocation on unmount with MNT_DETACH flag will return EINVAL + // there's no need to abort with an error if bind mountpoint is already unmounted + if err != unix.EINVAL { + return fmt.Errorf("Failed to unmount %s. %s", dest, err) + } + } + } + + return nil +} diff --git a/internal/jail/mount_not_supported.go b/internal/jail/mount_not_supported.go new file mode 100644 index 00000000..b8e359af --- /dev/null +++ b/internal/jail/mount_not_supported.go @@ -0,0 +1,23 @@ +// +build !linux + +package jail + +import ( + "fmt" + "runtime" +) + +func (j *Jail) notSupported() error { + if len(j.bindMounts) > 0 { + return fmt.Errorf("Bind mount not supported on %s", runtime.GOOS) + } + + return nil +} +func (j *Jail) mount() error { + return j.notSupported() +} + +func (j *Jail) unmount() error { + return j.notSupported() +} |