diff options
-rw-r--r-- | .gitlab/ci/test.yml | 26 | ||||
-rw-r--r-- | daemon.go | 206 | ||||
-rw-r--r-- | internal/config/config.go | 25 | ||||
-rw-r--r-- | internal/config/flags.go | 4 | ||||
-rw-r--r-- | internal/jail/jail.go | 262 | ||||
-rw-r--r-- | internal/jail/jail_linux_test.go | 29 | ||||
-rw-r--r-- | internal/jail/jail_test.go | 342 | ||||
-rw-r--r-- | internal/jail/mount_linux.go | 55 | ||||
-rw-r--r-- | internal/jail/mount_not_supported.go | 27 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 8 | ||||
-rw-r--r-- | test/acceptance/acceptance_test.go | 1 | ||||
-rw-r--r-- | test/acceptance/helpers_test.go | 14 |
12 files changed, 26 insertions, 973 deletions
diff --git a/.gitlab/ci/test.yml b/.gitlab/ci/test.yml index 527dd06f..a29acb7d 100644 --- a/.gitlab/ci/test.yml +++ b/.gitlab/ci/test.yml @@ -11,31 +11,31 @@ .tests-unit: extends: .tests-common script: - - echo "Running all unit tests without daemonizing..." + - echo "Running all unit tests..." - make test ARGS='-short' -.tests-acceptance-tmpdir: +.tests-acceptance: extends: .tests-common script: - - echo "Running just the acceptance tests daemonized (tmpdir)...." - - TEST_DAEMONIZE=tmpdir make acceptance + - echo "Running just the acceptance tests...." + - make acceptance -.tests-acceptance-inplace: +.tests-acceptance-daemonize: extends: .tests-common script: - - echo "Running just the acceptance tests daemonized (inplace)...." - - TEST_DAEMONIZE=inplace make acceptance + - echo "Running just the acceptance tests as daemon...." + - make acceptance ARGS='-daemonize' test:1.16: extends: .tests-unit image: golang:1.16 test-acceptance:1.16: - extends: .tests-acceptance-tmpdir + extends: .tests-acceptance image: golang:1.16 -test-acceptance-inplace:1.16: - extends: .tests-acceptance-inplace +test-acceptance-daemonize:1.16: + extends: .tests-acceptance-daemonize image: golang:1.16 test:1.17: @@ -43,11 +43,11 @@ test:1.17: image: golang:1.17 test-acceptance:1.17: - extends: .tests-acceptance-tmpdir + extends: .tests-acceptance image: golang:1.17 -test-acceptance-inplace:1.17: - extends: .tests-acceptance-inplace +test-acceptance-daemonize:1.17: + extends: .tests-acceptance-daemonize image: golang:1.17 race: @@ -1,10 +1,7 @@ package main import ( - "crypto/rand" "encoding/json" - "fmt" - "io/ioutil" "os" "os/exec" "os/signal" @@ -13,10 +10,8 @@ import ( "syscall" "github.com/sirupsen/logrus" - "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab-pages/internal/config" - "gitlab.com/gitlab-org/gitlab-pages/internal/jail" ) const ( @@ -117,168 +112,9 @@ func passSignals(cmd *exec.Cmd) { }() } -func chrootDaemon(cmd *exec.Cmd) (*jail.Jail, error) { - wd, err := os.Getwd() - if err != nil { - return nil, err - } - - chroot := jail.Into(wd) - - // Generate a probabilistically-unique suffix for the copy of the pages - // binary being placed into the chroot - suffix := make([]byte, 16) - if _, err := rand.Read(suffix); err != nil { - return nil, err - } - - tempExecutablePath := fmt.Sprintf("/.daemon.%x", suffix) - - if err := chroot.CopyTo(tempExecutablePath, cmd.Path); err != nil { - return nil, err - } - - // Update command to use chroot - cmd.SysProcAttr.Chroot = chroot.Path() - cmd.Path = tempExecutablePath - cmd.Dir = "/" - - return chroot, nil -} - -func jailCopyCertDir(cage *jail.Jail, sslCertDir, jailCertsDir string) error { - logrus.WithFields(logrus.Fields{ - "ssl-cert-dir": sslCertDir, - }).Debug("Copying certs from SSL_CERT_DIR") - - entries, err := ioutil.ReadDir(sslCertDir) - if err != nil { - return fmt.Errorf("failed to read SSL_CERT_DIR: %+v", err) - } - - for _, fi := range entries { - // Copy only regular files and symlinks - mode := fi.Mode() - if !(mode.IsRegular() || mode&os.ModeSymlink != 0) { - continue - } - - err = cage.CopyTo(jailCertsDir+"/"+fi.Name(), sslCertDir+"/"+fi.Name()) - if err != nil { - logrus.WithError(err).Errorf("failed to copy cert: %q", fi.Name()) - // Go on and try to copy other files. We don't want the whole - // startup process to fail due to a single failure here. - } - } - - return nil -} - -func jailDaemonCerts(cmd *exec.Cmd, cage *jail.Jail) error { - sslCertFile := os.Getenv("SSL_CERT_FILE") - sslCertDir := os.Getenv("SSL_CERT_DIR") - if sslCertFile == "" && sslCertDir == "" { - logrus.Warn("Neither SSL_CERT_FILE nor SSL_CERT_DIR environment variable is set. HTTPS requests will fail.") - return nil - } - - // This assumes cage.MkDir("/etc") has already been called - cage.MkDir("/etc/ssl", 0755) - - // Copy SSL_CERT_FILE inside the jail - if sslCertFile != "" { - jailCertsFile := "/etc/ssl/ca-bundle.pem" - err := cage.CopyTo(jailCertsFile, sslCertFile) - if err != nil { - return fmt.Errorf("failed to copy SSL_CERT_FILE: %+v", err) - } - cmd.Env = append(cmd.Env, "SSL_CERT_FILE="+jailCertsFile) - } - - // Copy all files and symlinks from SSL_CERT_DIR into the jail - if sslCertDir != "" { - jailCertsDir := "/etc/ssl/certs" - cage.MkDir(jailCertsDir, 0755) - err := jailCopyCertDir(cage, sslCertDir, jailCertsDir) - if err != nil { - return err - } - cmd.Env = append(cmd.Env, "SSL_CERT_DIR="+jailCertsDir) - } - - return nil -} - -func jailCreate(cmd *exec.Cmd) (*jail.Jail, error) { - cage := jail.CreateTimestamped("gitlab-pages", 0755) - - // Add /dev/urandom and /dev/random inside the jail. This is required to - // support Linux versions < 3.17, which do not have the getrandom() syscall - cage.MkDir("/dev", 0755) - if err := cage.CharDev("/dev/urandom"); err != nil { - return nil, err - } - - if err := cage.CharDev("/dev/random"); err != nil { - return nil, err - } - - // Add gitlab-pages inside the jail - err := cage.CopyTo("/gitlab-pages", cmd.Path) - if err != nil { - return nil, err - } - - // Add /etc/resolv.conf, /etc/hosts and /etc/nsswitch.conf inside the jail - cage.MkDir("/etc", 0755) - err = cage.Copy("/etc/resolv.conf") - if err != nil { - return nil, err - } - err = cage.Copy("/etc/hosts") - if err != nil { - return nil, err - } - - // When cgo is disabled, Go does not read `/etc/hosts` unless `/etc/nsswitch.conf` exists - // https://github.com/golang/go/issues/22846 - err = cage.Copy("/etc/nsswitch.conf") - if err != nil { - logrus.WithError(err).Warn("/etc/nsswitch.conf couldn't be copied to the jail, /etc/hosts might not be applicable") - } - - // Add certificates inside the jail - err = jailDaemonCerts(cmd, cage) - if err != nil { - return nil, err - } - - return cage, nil -} - -func jailDaemon(pagesRoot string, cmd *exec.Cmd) (*jail.Jail, error) { - cage, err := jailCreate(cmd) - if err != nil { - return nil, err - } - - // Bind mount shared folder - cage.MkDirAll(pagesRoot, 0755) - cage.Bind(pagesRoot, pagesRoot) - - // Update command to use chroot - cmd.SysProcAttr.Chroot = cage.Path() - cmd.Path = "/gitlab-pages" - cmd.Dir = pagesRoot - - return cage, nil -} - func daemonize(config *config.Config) error { uid := config.Daemon.UID gid := config.Daemon.GID - inPlace := config.Daemon.InplaceChroot - enableJail := config.Daemon.EnableJail pagesRoot := config.General.RootDir // Ensure pagesRoot is an absolute path. This will produce a different path @@ -293,11 +129,9 @@ func daemonize(config *config.Config) error { } logrus.WithFields(logrus.Fields{ - "uid": uid, - "gid": gid, - "in-place": inPlace, - "enable-jail": enableJail, - "pages-root": pagesRoot, + "uid": uid, + "gid": gid, + "pages-root": pagesRoot, }).Info("running the daemon as unprivileged user") cmd, err := daemonReexec(uid, gid, daemonRunProgram) @@ -306,26 +140,6 @@ func daemonize(config *config.Config) error { } defer killProcess(cmd) - if enableJail { - wrapper, err := createChroot(cmd, pagesRoot, inPlace) - if err != nil { - log.WithError(err).Error("create chroot failed") - return err - } - defer wrapper.Dispose() - - // Unshare mount namespace - // 1. If this fails, in a worst case changes to mounts will propagate to other processes - // 2. Ensures that jail mount is not propagated to the parent mount namespace - // to avoid populating `tmp` directory with old mounts - _ = wrapper.Unshare() - - if err := wrapper.Build(); err != nil { - log.WithError(err).Error("chroot build failed") - return err - } - } - // Create a pipe to pass the configuration configReader, configWriter, err := os.Pipe() if err != nil { @@ -355,20 +169,6 @@ func daemonize(config *config.Config) error { return cmd.Wait() } -func createChroot(cmd *exec.Cmd, pagesRoot string, inPlace bool) (*jail.Jail, error) { - // Run daemon in chroot environment - var wrapper *jail.Jail - var err error - - if inPlace { - wrapper, err = chrootDaemon(cmd) - } else { - wrapper, err = jailDaemon(pagesRoot, cmd) - } - - return wrapper, err -} - func updateFds(config *config.Config, cmd *exec.Cmd) { for _, fds := range [][]uintptr{ config.Listeners.HTTP, diff --git a/internal/config/config.go b/internal/config/config.go index c57eb15d..71ff0eed 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -82,10 +82,8 @@ type Auth struct { // Daemon groups settings related to configuring GitLab Pages daemon type Daemon struct { - UID uint - GID uint - InplaceChroot bool - EnableJail bool + UID uint + GID uint } // Cache configuration for GitLab API @@ -144,10 +142,6 @@ type ZipServing struct { RefreshInterval time.Duration OpenTimeout time.Duration AllowedPaths []string - // TODO: this is a temporary workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/326117#note_546346101 - // where daemon-inplace-chroot=true fails to serve zip archives when pages_serve_with_zip_file_protocol is enabled - // To be removed along with chroot support https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 - ChrootPath string } func internalGitlabServerFromFlags() string { @@ -222,10 +216,8 @@ func loadConfig() (*Config, error) { Scope: *authScope, }, Daemon: Daemon{ - UID: *daemonUID, - GID: *daemonGID, - InplaceChroot: *daemonInplaceChroot, - EnableJail: *daemonEnableJail, + UID: *daemonUID, + GID: *daemonGID, }, Log: Log{ Format: *logFormat, @@ -282,13 +274,6 @@ func loadConfig() (*Config, error) { return nil, err } - // TODO: this is a temporary workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/326117#note_546346101 - // where daemon-inplace-chroot=true fails to serve zip archives when pages_serve_with_zip_file_protocol is enabled - // To be removed along with chroot support https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 - if config.Daemon.InplaceChroot { - config.Zip.ChrootPath = *pagesRoot - } - return config, nil } @@ -298,8 +283,6 @@ func LogConfig(config *Config) { "artifacts-server-timeout": *artifactsServerTimeout, "daemon-gid": *daemonGID, "daemon-uid": *daemonUID, - "daemon-inplace-chroot": *daemonInplaceChroot, - "daemon-enable-jail": *daemonEnableJail, "default-config-filename": flag.DefaultConfigFlagname, "disable-cross-origin-requests": *disableCrossOriginRequests, "domain": config.General.Domain, diff --git a/internal/config/flags.go b/internal/config/flags.go index 63c52631..aa5bf1c5 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -23,8 +23,8 @@ var ( sentryEnvironment = flag.String("sentry-environment", "", "The environment for sentry crash reporting") daemonUID = flag.Uint("daemon-uid", 0, "Drop privileges to this user") daemonGID = flag.Uint("daemon-gid", 0, "Drop privileges to this group") - daemonInplaceChroot = flag.Bool("daemon-inplace-chroot", false, "Fall back to a non-bind-mount chroot of -pages-root when daemonizing") - daemonEnableJail = flag.Bool("daemon-enable-jail", false, "Enable legacy jailing mechanism (disabled by default for API-based configuration, and always enabled for disk-basec configuration)") + _ = flag.Bool("daemon-enable-jail", false, "DEPRECATED and ignored, will be removed in 15.0") + _ = flag.Bool("daemon-inplace-chroot", false, "DEPRECATED and ignored, will be removed in 15.0") // TODO: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/599 propagateCorrelationID = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") logFormat = flag.String("log-format", "json", "The log output format: 'text' or 'json'") logVerbose = flag.Bool("log-verbose", false, "Verbose logging") diff --git a/internal/jail/jail.go b/internal/jail/jail.go deleted file mode 100644 index 4cb5765a..00000000 --- a/internal/jail/jail.go +++ /dev/null @@ -1,262 +0,0 @@ -package jail - -import ( - "fmt" - "io" - "os" - "path" - "strings" - "syscall" - "time" - - "golang.org/x/sys/unix" -) - -type pathAndMode struct { - path string - mode os.FileMode - - // Only respected if mode is os.ModeCharDevice - rdev int -} - -// Jail is a Chroot jail builder -type Jail struct { - root string - deleteRoot bool - directories []pathAndMode - files map[string]pathAndMode - bindMounts map[string]string -} - -// Into returns a Jail on path, assuming it already exists on disk. On disposal, -// the jail *will not* remove the path -func Into(path string) *Jail { - return &Jail{ - root: path, - deleteRoot: false, - files: make(map[string]pathAndMode), - bindMounts: make(map[string]string), - } -} - -// Create returns a Jail on path, creating the directory if needed. On disposal, -// the jail will remove the path -func Create(path string, perm os.FileMode) *Jail { - jail := Into(path) - jail.deleteRoot = true - jail.directories = append(jail.directories, pathAndMode{path: path, mode: perm}) - - return jail -} - -// CreateTimestamped returns a Jail on a path composed by prefix and current -// timestamp, creating the directory. On disposal, the jail will remove the path -func CreateTimestamped(prefix string, perm os.FileMode) *Jail { - jailPath := path.Join(os.TempDir(), fmt.Sprintf("%s-%d", prefix, time.Now().UnixNano())) - - return Create(jailPath, perm) -} - -// Path returns the path of the jail -func (j *Jail) Path() string { - return j.root -} - -// Build creates the jail, making directories and copying files. If an error -// setting up is encountered, a best-effort attempt will be made to remove any -// partial state before returning the error -func (j *Jail) Build() error { - // Simplify error-handling in this method. It's unsafe to run os.RemoveAll() - // across a bind mount. Only one is needed at present, and this restriction - // means there's no need to handle the case where one of several mounts - // failed in j.mount() - // - // Make j.mount() robust before removing this restriction, at the risk of - // extreme data loss - if len(j.bindMounts) > 1 { - return fmt.Errorf("bug: jail does not currently support multiple bind mounts") - } - - for _, dir := range j.directories { - if err := os.Mkdir(dir.path, dir.mode); err != nil { - j.removeAll() - return fmt.Errorf("can't create directory %q. %s", dir.path, err) - } - } - - for dest, src := range j.files { - if err := handleFile(dest, src); err != nil { - j.removeAll() - return fmt.Errorf("can't copy %q -> %q. %s", src.path, dest, err) - } - } - - if err := j.mount(); err != nil { - // Only one bind mount is supported. If it failed to mount, there is - // nothing to unmount, so it is safe to run removeAll() here. - j.removeAll() - return err - } - - return nil -} - -func (j *Jail) removeAll() error { - // Deleting the root will remove all child directories, so there's no need - // to traverse files and directories - if j.deleteRoot { - if err := os.RemoveAll(j.Path()); err != nil { - return fmt.Errorf("can't delete jail %q. %s", j.Path(), err) - } - } else { - for path := range j.files { - if err := os.Remove(path); err != nil { - return fmt.Errorf("can't delete file in jail %q: %s", path, err) - } - } - - // Iterate directories in reverse to remove children before parents - for i := len(j.directories) - 1; i >= 0; i-- { - dest := j.directories[i] - if err := os.Remove(dest.path); err != nil { - return fmt.Errorf("can't delete directory in jail %q: %s", dest.path, err) - } - } - } - - return nil -} - -// Dispose erases everything inside the jail -func (j *Jail) Dispose() error { - if err := j.unmount(); err != nil { - return err - } - - if err := j.removeAll(); 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}) -} - -// MkDirAll enqueue a mkdir operation at jail building time for all directories -// in dir to be created one by one -func (j *Jail) MkDirAll(dir string, perm os.FileMode) { - subPaths := strings.Split(dir, "/") - currentParent := "" - - // skip first subPath as is an empty string given dir starts with "/" - for _, subPath := range subPaths[1:] { - currentPath := path.Join(currentParent, subPath) - j.directories = append(j.directories, pathAndMode{path: j.ExternalPath(currentPath), mode: perm}) - - currentParent = currentPath - } -} - -// CharDev enqueues an mknod operation for the given character device at jail -// building time -func (j *Jail) CharDev(path string) error { - fi, err := os.Stat(path) - if err != nil { - return fmt.Errorf("can't stat %q: %s", path, err) - } - - if (fi.Mode() & os.ModeCharDevice) == 0 { - return fmt.Errorf("can't mknod %q: not a character device", path) - } - - // Read the device number from the underlying unix implementation of stat() - sys, ok := fi.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("couldn't determine rdev for %q", path) - } - - jailedDest := j.ExternalPath(path) - j.files[jailedDest] = pathAndMode{ - path: path, - mode: fi.Mode(), - rdev: int(sys.Rdev), - } - - return nil -} - -// 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 -} - -// 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 handleFile(dest string, src pathAndMode) error { - // Using `io.Copy` on a character device simply doesn't work - if (src.mode & os.ModeCharDevice) > 0 { - return createCharacterDevice(dest, src) - } - - // FIXME: currently, symlinks, block devices, named pipes and other - // non-regular files will be `Open`ed and have that content streamed to a - // regular file inside the chroot. This is actually desired behaviour for, - // e.g., `/etc/resolv.conf`, but was very surprising - return copyFile(dest, src.path, src.mode) -} - -func createCharacterDevice(dest string, src pathAndMode) error { - unixMode := uint32(src.mode.Perm() | syscall.S_IFCHR) - - return unix.Mknod(dest, unixMode, src.rdev) -} - -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_linux_test.go b/internal/jail/jail_linux_test.go deleted file mode 100644 index 9900a769..00000000 --- a/internal/jail/jail_linux_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package jail_test - -import ( - "os" - "testing" - - "github.com/stretchr/testify/require" - - "gitlab.com/gitlab-org/gitlab-pages/internal/jail" -) - -func TestJailCreateAndCleanSubPaths(t *testing.T) { - if os.Geteuid() != 0 { - t.Skip("skipping test for non root user") - } - - cage := jail.CreateTimestamped("gitlab-pages", 0755) - subPath := "/pages/sub/path" - - // Bind mount shared folder - cage.MkDirAll(subPath, 0755) - cage.Bind(subPath, cage.Path()+subPath) - - require.NoError(t, cage.Build()) - require.NoError(t, cage.Dispose()) - - _, err := os.Stat(cage.Path()) - require.True(t, os.IsNotExist(err), "jail was removed") -} diff --git a/internal/jail/jail_test.go b/internal/jail/jail_test.go deleted file mode 100644 index 2ccd0bc8..00000000 --- a/internal/jail/jail_test.go +++ /dev/null @@ -1,342 +0,0 @@ -package jail_test - -import ( - "fmt" - "io/ioutil" - "os" - "path" - "syscall" - "testing" - "time" - - "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) { - require := require.New(t) - - prefix := "jail" - var mode os.FileMode = 0755 - - j1 := jail.CreateTimestamped(prefix, mode) - j2 := jail.CreateTimestamped(prefix, mode) - - require.NotEqual(j1.Path(), j2.Path()) -} - -func TestJailPath(t *testing.T) { - require := require.New(t) - - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - - require.Equal(jailPath, cage.Path()) -} - -func TestJailBuild(t *testing.T) { - require := require.New(t) - - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - - _, err := os.Stat(cage.Path()) - require.Error(err, "Jail path should not exist before Jail.Build()") - - err = cage.Build() - require.NoError(err) - defer cage.Dispose() - - _, err = os.Stat(cage.Path()) - require.NoError(err, "Jail path should exist after Jail.Build()") -} - -func TestJailOnlySupportsOneBindMount(t *testing.T) { - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - - cage.Bind("/bin", "/bin") - cage.Bind("/lib", "/lib") - cage.Bind("/usr", "/usr") - - err := cage.Build() - require.Error(t, err, "Build() is expected to fail in this test") - - _, statErr := os.Stat(cage.Path()) - require.True(t, os.IsNotExist(statErr), "Jail path should not exist") -} - -func TestJailBuildCleansUpWhenMountFails(t *testing.T) { - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - cage.Bind("/foo", "/this/path/does/not/exist/so/mount/will/fail") - - err := cage.Build() - require.Error(t, err, "Build() is expected to fail in this test") - - _, statErr := os.Stat(cage.Path()) - require.True(t, os.IsNotExist(statErr), "Jail path should have been cleaned up") -} - -func TestJailDispose(t *testing.T) { - require := require.New(t) - - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - - err := cage.Build() - require.NoError(err) - - err = cage.Dispose() - require.NoError(err) - - _, err = os.Stat(cage.Path()) - require.Error(err, "Jail path should not exist after Jail.Dispose()") -} - -func TestJailDisposeDoNotFailOnMissingPath(t *testing.T) { - require := require.New(t) - - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - - _, err := os.Stat(cage.Path()) - require.Error(err, "Jail path should not exist") - - err = cage.Dispose() - require.NoError(err) -} - -func TestJailWithCharacterDevice(t *testing.T) { - if os.Geteuid() != 0 { - t.Log("This test only works if run as root") - t.SkipNow() - } - - // Determine the expected rdev - fi, err := os.Stat("/dev/urandom") - require.NoError(t, err) - sys, ok := fi.Sys().(*syscall.Stat_t) - if !ok { - t.Log("Couldn't determine expected rdev for /dev/urandom, skipping") - t.SkipNow() - } - - expectedRdev := sys.Rdev - - jailPath := tmpJailPath() - cage := jail.Create(jailPath, 0755) - cage.MkDir("/dev", 0755) - - require.NoError(t, cage.CharDev("/dev/urandom")) - require.NoError(t, cage.Build()) - defer cage.Dispose() - - fi, err = os.Lstat(path.Join(cage.Path(), "/dev/urandom")) - require.NoError(t, err) - - isCharDev := fi.Mode()&os.ModeCharDevice == os.ModeCharDevice - require.True(t, isCharDev, "Created file was not a character device") - - sys, ok = fi.Sys().(*syscall.Stat_t) - require.True(t, ok, "Couldn't determine rdev of created character device") - require.Equal(t, expectedRdev, sys.Rdev, "Incorrect rdev for /dev/urandom") -} - -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) { - require := require.New(t) - - cage := jail.CreateTimestamped("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 { - require.Error(err) - } else { - require.NoError(err) - - for _, dir := range test.directories { - _, err := os.Stat(path.Join(cage.Path(), dir)) - require.NoError(err, "jailed dir should exist") - } - - for _, file := range test.files { - _, err := os.Stat(path.Join(cage.Path(), file)) - require.NoError(err, "Jailed file should exist") - } - } - }) - } -} - -func TestJailCopyTo(t *testing.T) { - require := require.New(t) - - content := "hello" - - cage := jail.CreateTimestamped("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) - require.NoError(err) - - err = cage.Build() - defer cage.Dispose() - require.NoError(err) - - jailedFI, err := os.Stat(jailedFilePath) - require.NoError(err) - - fi, err := os.Stat(filePath) - require.NoError(err) - - require.Equal(fi.Mode(), jailedFI.Mode(), "jailed file should preserve file mode") - require.Equal(fi.Size(), jailedFI.Size(), "jailed file should have same size of original file") - - jailedContent, err := ioutil.ReadFile(jailedFilePath) - require.NoError(err) - require.Equal(content, string(jailedContent), "jailed file should preserve file content") -} - -func TestJailIntoOnlyCleansSubpaths(t *testing.T) { - jailPath := tmpJailPath() - require.NoError(t, os.MkdirAll(jailPath, 0755)) - defer os.RemoveAll(jailPath) - - chroot := jail.Into(jailPath) - chroot.MkDir("/etc", 0755) - chroot.Copy("/etc/resolv.conf") - require.NoError(t, chroot.Build()) - require.NoError(t, chroot.Dispose()) - - _, err := os.Stat(path.Join(jailPath, "/etc/resolv.conf")) - require.True(t, os.IsNotExist(err), "/etc/resolv.conf in jail was not removed") - _, err = os.Stat(path.Join(jailPath, "/etc")) - require.True(t, os.IsNotExist(err), "/etc in jail was not removed") - _, err = os.Stat(jailPath) - require.NoError(t, err, "/ in jail (corresponding to external directory) was removed") -} - -func TestJailIntoCleansNestedDirs(t *testing.T) { - jailPath := tmpJailPath() - require.NoError(t, os.MkdirAll(jailPath, 0755)) - defer os.RemoveAll(jailPath) - - chroot := jail.Into(jailPath) - - // These need to be cleaned up in reverse order - chroot.MkDir("/way", 0755) - chroot.MkDir("/way/down", 0755) - chroot.MkDir("/way/down/here", 0755) - - require.NoError(t, chroot.Build()) - require.NoError(t, chroot.Dispose()) - - verify := func(inPath string) { - _, err := os.Stat(path.Join(jailPath, inPath)) - require.True(t, os.IsNotExist(err), "{} in jail was not removed", inPath) - } - - verify("/way") - verify("/way/down") - verify("/way/down/here") - - _, err := os.Stat(jailPath) - require.NoError(t, err, "/ in jail (corresponding to external directory) was removed") -} - -func TestJailIntoMkDirFails(t *testing.T) { - jailPath := tmpJailPath() - require.NoError(t, os.MkdirAll(jailPath, 0755)) - defer os.RemoveAll(jailPath) - - pagesRoot := "/pages/sub/path" - - chroot := jail.Into(jailPath) - chroot.MkDir(pagesRoot, 0755) - - err := chroot.Build() - - require.Error(t, err, "err") - require.Contains(t, err.Error(), "no such file or directory") - - _, err = os.Stat(path.Join(jailPath, pagesRoot)) - require.True(t, os.IsNotExist(err), "%s in jail was not removed", pagesRoot) -} - -func TestJailIntoMkDirAll(t *testing.T) { - jailPath := tmpJailPath() - require.NoError(t, os.MkdirAll(jailPath, 0755)) - defer os.RemoveAll(jailPath) - - chroot := jail.Into(jailPath) - - chroot.MkDirAll("/way/down/here", 0755) - - require.NoError(t, chroot.Build()) - require.NoError(t, chroot.Dispose()) - - verify := func(inPath string) { - _, err := os.Stat(path.Join(jailPath, inPath)) - require.True(t, os.IsNotExist(err), "{} in jail was not removed", inPath) - } - - verify("/way") - verify("/way/down") - verify("/way/down/here") - - _, err := os.Stat(jailPath) - require.NoError(t, err, "/ in jail (corresponding to external directory) was not removed") -} diff --git a/internal/jail/mount_linux.go b/internal/jail/mount_linux.go deleted file mode 100644 index cb5a6bed..00000000 --- a/internal/jail/mount_linux.go +++ /dev/null @@ -1,55 +0,0 @@ -package jail - -import ( - "fmt" - "syscall" - - "gitlab.com/gitlab-org/labkit/log" - "golang.org/x/sys/unix" -) - -// Unshare makes the process to own it's own mount namespace -// and prevent the changes for mounts to be propagated -// to other mount namespace. Making all mount changes local -// to current process -func (j *Jail) Unshare() error { - // https://man7.org/linux/man-pages/man7/mount_namespaces.7.html - err := syscall.Unshare(syscall.CLONE_NEWNS) - log.WithError(err).Info("unsharing mount namespace") - if err != nil { - return err - } - - // As documented in `mount_namespaces`: - // An application that creates a new mount namespace directly using - // clone(2) or unshare(2) may desire to prevent propagation of mount - // events to other mount namespaces - err = syscall.Mount("none", "/", "", unix.MS_REC|unix.MS_PRIVATE, "") - log.WithError(err).Info("changing root filesystem propagation") - return err -} - -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 deleted file mode 100644 index b4d3e348..00000000 --- a/internal/jail/mount_not_supported.go +++ /dev/null @@ -1,27 +0,0 @@ -// +build !linux - -package jail - -import ( - "fmt" - "runtime" -) - -func (j *Jail) Unshare() error { - return fmt.Errorf("unshare not supported on %s", runtime.GOOS) -} - -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() -} diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index dc6eb5c4..3dade30f 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -104,12 +104,8 @@ func (fs *zipVFS) Reconfigure(cfg *config.Config) error { } func (fs *zipVFS) reconfigureTransport(cfg *config.Config) error { - chrootPath := "" - if cfg.Daemon.EnableJail { - chrootPath = cfg.Zip.ChrootPath - } - - fsTransport, err := httpfs.NewFileSystemPath(cfg.Zip.AllowedPaths, chrootPath) + // TODO: remove chrootPath from httpsfs package: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/598 + fsTransport, err := httpfs.NewFileSystemPath(cfg.Zip.AllowedPaths, "") if err != nil { return err } diff --git a/test/acceptance/acceptance_test.go b/test/acceptance/acceptance_test.go index e6b50c10..9d0c0cd2 100644 --- a/test/acceptance/acceptance_test.go +++ b/test/acceptance/acceptance_test.go @@ -16,6 +16,7 @@ const ( var ( pagesBinary = flag.String("gitlab-pages-binary", "../../gitlab-pages", "Path to the gitlab-pages binary") + daemonize = flag.Bool("daemonize", false, "run tests as daemon") httpPort = "36000" httpsPort = "37000" diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index c8852976..b8181dca 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -367,8 +367,7 @@ func contains(slice []string, s string) bool { } func getPagesDaemonArgs(t *testing.T) []string { - mode := os.Getenv("TEST_DAEMONIZE") - if mode == "" { + if !(*daemonize) { return nil } @@ -380,17 +379,6 @@ func getPagesDaemonArgs(t *testing.T) []string { out := []string{} - switch mode { - case "tmpdir": - out = append(out, "-daemon-inplace-chroot=false") - case "inplace": - out = append(out, "-daemon-inplace-chroot=true") - default: - t.Log("Unknown daemonize mode", mode) - t.FailNow() - return nil - } - t.Log("Running pages as a daemon") // This triggers the drop-privileges-and-chroot code in the pages daemon |