diff --git a/README.md b/README.md index 92ac06a..b21a0bf 100644 --- a/README.md +++ b/README.md @@ -319,7 +319,12 @@ by the user who runs the binary. CGI processes will then be unable to read any of those sensitive files. If the binary is not SETUID but is run by the superuser/root, then Molly will change its UID to that of the `nobody` user before accepting network connections, so CGI -processes will again not be able to read sensitive files. +processes will again not be able to read sensitive files. Note that +while these measures can protect Molly's own sensitive files from +CGI processes, CGI processes may still be able to read other sensitive +files anywhere else on the system. Consider chroot()-ing Molly Brown +into a small corner of the filesystem (see `ChrootDir` below) to +reduce this risk. When compiled on GNU/Linux with Go versions 1.15 or earlier, Molly Brown is completley unable to reliably change its UID due to the way @@ -382,6 +387,15 @@ facility. status code of 60. Requests made with a certificate not in the list will cause a response with a status code of 60. +### Security settings + +* `ChrootDir`: A directory to which Molly Brown should chroot(), + making it more difficult for the server itself or spawned CGI + processes to read or write any files higher in the hiearch. The + chroot happens immediately after reading the config file. All other + paths specified in the config file (e.g. `DocBase`, `KeyPath`, + `AccessLog`) must be specified relative to `ChrootDir`. + ## .molly files In order to allow users of shared-hosting who do not have access to diff --git a/config.go b/config.go index 5ec24b2..40ed862 100644 --- a/config.go +++ b/config.go @@ -16,6 +16,7 @@ type Config struct { KeyPath string DocBase string HomeDocBase string + ChrootDir string GeminiExt string DefaultLang string DefaultEncoding string @@ -59,6 +60,7 @@ func getConfig(filename string) (Config, error) { config.KeyPath = "key.pem" config.DocBase = "/var/gemini/" config.HomeDocBase = "users" + config.ChrootDir = "" config.GeminiExt = "gmi" config.DefaultLang = "" config.DefaultEncoding = "" @@ -92,13 +94,29 @@ func getConfig(filename string) (Config, error) { return config, errors.New("Invalid DirectorySort value.") } - // Absolutise DocBase + // Validate chroot() dir + if config.ChrootDir != "" { + config.ChrootDir = filepath.Abs(config.ChrootDir) + _, err := os.Stat(config.ChrootDir) + if os.IsNotExist(err) { + return config, err + } + } + + // Absolutise DocBase, relative to the chroot dir if !filepath.IsAbs(config.DocBase) { abs, err := filepath.Abs(config.DocBase) if err != nil { return config, err } - config.DocBase = abs + if config.ChrootDir != "" { + config.DocBase, err = filepath.Rel(config.ChrootDir, abs) + if err != nil { + return config, err + } + } else { + config.DocBase = abs + } } // Absolutise CGI paths diff --git a/handler.go b/handler.go index a1ee9cc..5ab0993 100644 --- a/handler.go +++ b/handler.go @@ -44,7 +44,9 @@ func handleGeminiRequest(conn net.Conn, config Config, accessLogEntries chan Log log.RemoteAddr = conn.RemoteAddr() log.RequestURL = "-" log.Status = 0 - defer func() { accessLogEntries <- log }() + if accessLogEntries != nil { + defer func() { accessLogEntries <- log }() + } // Read request URL, err := readRequest(conn, &log, errorLog) diff --git a/main.go b/main.go index 8c68b74..121836b 100644 --- a/main.go +++ b/main.go @@ -7,6 +7,7 @@ import ( "log" "os" "os/signal" + "os/user" "strconv" "sync" "syscall" @@ -41,6 +42,29 @@ func main() { log.Fatal(err) } + // If we are running as root, find the UID of the "nobody" user, before a + // chroot() possibly stops seeing /etc/passwd + uid := os.Getuid() + nobody_uid := -1 + if uid == 0 { + nobody_user, err := user.Lookup("nobody") + if err != nil { + log.Fatal("Running as root but could not lookup UID for user " + "nobody" + ": " + err.Error()) + } + nobody_uid, err = strconv.Atoi(nobody_user.Uid) + if err != nil { + log.Fatal("Running as root but could not lookup UID fr user " + "nobody" + ": " + err.Error()) + } + } + + // Chroot, if asked + if config.ChrootDir != "" { + err := syscall.Chroot(config.ChrootDir) + if err != nil { + log.Fatal("Could not chroot to " + config.ChrootDir + ": " + err.Error()) + } + } + // Open log files var errorLogFile *os.File if config.ErrorLog == "" { @@ -55,13 +79,9 @@ func main() { errorLog := log.New(errorLogFile, "", log.Ldate|log.Ltime) var accessLogFile *os.File - // TODO: Find a more elegant/portable way to disable logging - if config.AccessLog == "" { - config.AccessLog = "/dev/null" - } if config.AccessLog == "-" { accessLogFile = os.Stdout - } else { + } else if config.AccessLog != "" { accessLogFile, err = os.OpenFile(config.AccessLog, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { errorLog.Println("Error opening access log file: " + err.Error()) @@ -92,11 +112,15 @@ func main() { ClientAuth: tls.RequestClientCert, } - // Chdir to / so we don't block any mountpoints - err = os.Chdir("/") - if err != nil { - errorLog.Println("Could not change working directory to /: " + err.Error()) - } + // Try to chdir to /, so we don't block any mountpoints + // But if we can't for some reason it's no big deal + err = os.Chdir("/") + if err != nil { + errorLog.Println("Could not change working directory to /: " + err.Error()) + } + + // Apply security restrictions + enableSecurityRestrictions(config, nobody_uid, errorLog) // Create TLS listener listener, err := tls.Listen("tcp", ":"+strconv.Itoa(config.Port), tlscfg) @@ -107,16 +131,18 @@ func main() { defer listener.Close() // Start log handling routines - accessLogEntries := make(chan LogEntry, 10) - go func() { - for { - entry := <-accessLogEntries - writeLogEntry(accessLogFile, entry) - } - }() - - // Restrict access to the files specified in config - enableSecurityRestrictions(config, errorLog) + var accessLogEntries chan LogEntry + if config.AccessLog == "" { + accessLogEntries = nil + } else { + accessLogEntries := make(chan LogEntry, 10) + go func() { + for { + entry := <-accessLogEntries + writeLogEntry(accessLogFile, entry) + } + }() + } // Start listening for signals shutdown := make(chan struct{}) diff --git a/security_dropprivs.go b/security_dropprivs.go index 59ad693..47a1d45 100644 --- a/security_dropprivs.go +++ b/security_dropprivs.go @@ -5,41 +5,23 @@ package main import ( "log" "os" - "os/user" "strconv" "syscall" ) -func DropPrivs(config Config, errorLog *log.Logger) { +func DropPrivs(config Config, nobody_uid int, errorLog *log.Logger) { // Get our real and effective UIDs uid := os.Getuid() euid := os.Geteuid() - // If these are equal and non-zero, there's nothing to do - if uid == euid && uid != 0 { - return - } - - // If our real UID is root, we need to lookup the nobody UID - if uid == 0 { - user, err := user.Lookup("nobody") + // Are we root or are we running as a setuid binary? + if uid == 0 || uid != euid { + err := syscall.Setuid(nobody_uid) if err != nil { - errorLog.Println("Could not lookup UID for user " + "nobody" + ": " + err.Error()) + errorLog.Println("Could not setuid to " + strconv.Itoa(uid) + ": " + err.Error()) log.Fatal(err) } - uid, err = strconv.Atoi(user.Uid) - if err != nil { - errorLog.Println("Could not lookup UID fr user " + "nobody" + ": " + err.Error()) - log.Fatal(err) - } - } - - // Drop priveleges - err := syscall.Setuid(uid) - if err != nil { - errorLog.Println("Could not setuid to " + strconv.Itoa(uid) + ": " + err.Error()) - log.Fatal(err) } } diff --git a/security_openbsd.go b/security_openbsd.go index 8afcabe..8253893 100644 --- a/security_openbsd.go +++ b/security_openbsd.go @@ -11,10 +11,10 @@ import ( // operations available to the molly brown executable. Please note that (S)CGI // processes that molly brown spawns or communicates with are unrestricted // and should pledge their own restrictions and unveil their own files. -func enableSecurityRestrictions(config Config, errorLog *log.Logger) { +func enableSecurityRestrictions(config Config, nobody_uid int, errorLog *log.Logger) { // Setuid to an unprivileged user - DropPrivs(config, errorLog) + DropPrivs(config, nobody_uid, errorLog) // Unveil the configured document base as readable. log.Println("Unveiling \"" + config.DocBase + "\" as readable.") diff --git a/security_other_unix.go b/security_other_unix.go index 0026358..4567588 100644 --- a/security_other_unix.go +++ b/security_other_unix.go @@ -6,8 +6,8 @@ import ( "log" ) -func enableSecurityRestrictions(config Config, errorLog *log.Logger) { +func enableSecurityRestrictions(config Config, nobody_uid int, errorLog *log.Logger) { // Setuid to an unprivileged user - DropPrivs(config, errorLog) + DropPrivs(config, nobody_uid, errorLog) }