From f5204f578ba201b31eb5b9a18abba17ec039f045 Mon Sep 17 00:00:00 2001
From: Hugo Landau <hlandau@devever.net>
Date: Sat, 5 Dec 2015 04:38:31 +0000
Subject: [PATCH] Enhance rootless support.

---
 cmd/acmetool/main.go     |  2 ++
 notify/os.go             |  7 +++--
 redirector/redirector.go | 61 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/cmd/acmetool/main.go b/cmd/acmetool/main.go
index 755d959..85ce540 100644
--- a/cmd/acmetool/main.go
+++ b/cmd/acmetool/main.go
@@ -43,6 +43,7 @@ var (
 
 	redirectorCmd      = kingpin.Command("redirector", "HTTP to HTTPS redirector with challenge response support")
 	redirectorPathFlag = redirectorCmd.Flag("path", "Path to serve challenge files from").String()
+	redirectorGIDFlag  = redirectorCmd.Flag("challenge-gid", "GID to chgrp the challenge path to (optional)").String()
 
 	importJWKAccountCmd = kingpin.Command("import-jwk-account", "Import a JWK account key")
 	importJWKURLArg     = importJWKAccountCmd.Arg("provider-url", "Provider URL (e.g. https://acme-v01.api.letsencrypt.org/directory)").Required().String()
@@ -144,6 +145,7 @@ func runRedirector() {
 			return redirector.New(redirector.Config{
 				Bind:          ":80",
 				ChallengePath: rpath,
+				ChallengeGID:  *redirectorGIDFlag,
 			})
 		},
 	})
diff --git a/notify/os.go b/notify/os.go
index c8d6e2d..57c7674 100644
--- a/notify/os.go
+++ b/notify/os.go
@@ -1,9 +1,9 @@
 package notify
 
 import (
+	deos "github.com/hlandau/degoutils/os"
 	"os"
 	"os/exec"
-	"syscall"
 )
 
 func runningAsRoot() bool {
@@ -42,10 +42,11 @@ func shouldSudoFile(fn string, fi os.FileInfo) bool {
 		return false
 	}
 
-	// Don't sudo anything which appears to be setuid-d for a non-root user.
+	// Don't sudo anything which appears to be setuid'd for a non-root user.
 	// This doesn't really buy us anything security-wise, but it's not what
 	// we're expecting.
-	if s, ok := fi.Sys().(*syscall.Stat_t); ok && s.Uid != 0 {
+	uid, err := deos.GetFileUID(fi)
+	if err != nil || uid != 0 {
 		return false
 	}
 
diff --git a/redirector/redirector.go b/redirector/redirector.go
index 96dd119..5b5ba9b 100644
--- a/redirector/redirector.go
+++ b/redirector/redirector.go
@@ -4,8 +4,10 @@ package redirector
 
 import (
 	"fmt"
+	deos "github.com/hlandau/degoutils/os"
 	"github.com/hlandau/xlog"
 	"gopkg.in/hlandau/service.v2/daemon/chroot"
+	"gopkg.in/hlandau/service.v2/passwd"
 	"gopkg.in/tylerb/graceful.v1"
 	"html"
 	"net"
@@ -20,6 +22,7 @@ var log, Log = xlog.New("acme.redirector")
 type Config struct {
 	Bind          string `default:":80" usage:"Bind address"`
 	ChallengePath string `default:"/var/run/acme/acme-challenge" usage:"Path containing HTTP challenge files"`
+	ChallengeGID  string `default:"" usage:"GID to chgrp the challenge path to (optional)"`
 }
 
 type Redirector struct {
@@ -47,6 +50,13 @@ func New(cfg Config) (*Redirector, error) {
 		return nil, err
 	}
 
+	if r.cfg.ChallengeGID != "" {
+		err := enforceGID(r.cfg.ChallengeGID, r.cfg.ChallengePath)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	l, err := net.Listen("tcp", r.httpServer.Server.Addr)
 	if err != nil {
 		return nil, err
@@ -57,6 +67,57 @@ func New(cfg Config) (*Redirector, error) {
 	return r, nil
 }
 
+func enforceGID(gid, path string) error {
+	newGID, err := passwd.ParseGID(gid)
+	if err != nil {
+		return err
+	}
+
+	// So this is a surprisingly complicated dance if we want to be free of
+	// potentially hazardous race conditions. We have a path. We can't assume
+	// anything about its ownership, or mode, whether it's a symlink, etc.
+	//
+	// The big risk is that someone is able to create a symlink pointing to
+	// something they want to illicitly access. Note that since /var/run will
+	// commonly be used and because this directory is world-writeable, ala /tmp,
+	// this is a real risk.
+	//
+	// So we have to make sure we don't follow symlinks. Assume we are running
+	// as root (necessary, since we're chowning), and that nothing running as
+	// root is malicious.
+	//
+	// We open the directory as a file so we can modify it using that reference
+	// without worrying about the resolution of the path changing under us. But
+	// we need to make sure we don't follow symlinks. This requires special OS
+	// support, alas.
+	dir, err := deos.OpenNoSymlinks(path)
+	if err != nil {
+		return err
+	}
+
+	defer dir.Close()
+
+	fi, err := dir.Stat()
+	if err != nil {
+		return err
+	}
+
+	// Attributes of the directory can still change, but its type certainly
+	// can't. This guarantee is enough for our purposes.
+	if (fi.Mode() & os.ModeType) != os.ModeDir {
+		return fmt.Errorf("challenge path %#v is not a directory", path)
+	}
+
+	curUID, err := deos.GetFileUID(fi)
+	if err != nil {
+		return err
+	}
+
+	dir.Chmod((fi.Mode() | 0070) & ^os.ModeType) // Ignore errors.
+	dir.Chown(curUID, newGID)                    // Ignore errors.
+	return nil
+}
+
 func (r *Redirector) commonHandler(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		rw.Header().Set("Server", "acmetool-redirector")