Skip to content

Commit

Permalink
Enhance rootless support.
Browse files Browse the repository at this point in the history
  • Loading branch information
hlandau committed Dec 5, 2015
1 parent 7543e48 commit f5204f5
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 3 deletions.
2 changes: 2 additions & 0 deletions cmd/acmetool/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -144,6 +145,7 @@ func runRedirector() {
return redirector.New(redirector.Config{
Bind: ":80",
ChallengePath: rpath,
ChallengeGID: *redirectorGIDFlag,
})
},
})
Expand Down
7 changes: 4 additions & 3 deletions notify/os.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package notify

import (
deos "github.com/hlandau/degoutils/os"
"os"
"os/exec"
"syscall"
)

func runningAsRoot() bool {
Expand Down Expand Up @@ -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
}

Expand Down
61 changes: 61 additions & 0 deletions redirector/redirector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down

0 comments on commit f5204f5

Please sign in to comment.