Move trusted_forwarders code to ai3/go-common

Simplifies the device manager code as we don't have to duplicate the
IP/network matching logic there.

README.md

@@ -52,9 +52,6 @@ understands the following attributes:
   * `auth_key`: a long-term key to authenticate HTTP-based cookies
   * `geo_ip_data_files`: GeoIP databases to use (in mmdb format), if
     unset the module will use the default GeoLite2-Country db
-  * `trusted_forwarders`: list of trusted IP addresses (reverse
-    proxies). If a request comes from here, we will use the
-    *remote_addr_header* instead
   * `remote_addr_header`: HTTP header to use to obtain the remote
     client address, when the request comes from a trusted forwarder
 * `keystore`: configures the connection to the keystore service
@@ -73,6 +70,10 @@ understands the following attributes:
       * `path` is a regular expression to match the request URL path
       * `cn` is a regular expression that must match the CommonName
         part of the subject of the client certificate
+  * `trusted_forwarders`: list of trusted IP addresses (reverse
+    proxies). If a request comes from here, we will trust the
+    X-Forwarded-Proto and X-Real-IP headers when determining the
+    client IP address
 
 ## Device tracking
 

cmd/sso-server/main_test.go

@@ -31,8 +31,6 @@ csrf_secret: "XLFtiymBU5p59K/IsqW/oh/5dfP4UC6JSNWMVeiQ8t8GjnB1rzusIFnyho5y4nE1"
 auth_service: sso
 device_manager:
   auth_key: "ffolt81h4CA5kEcwckXmuUUkchwKQmRAeWb1H6Kpzx3+uGqwrVpBfGwzRSYaeir1"
-  trusted_forwarders:
-    - 192.168.10.10
 `
 
 func TestMain_LoadConfig(t *testing.T) {

server/device/manager.go

@@ -24,18 +24,14 @@ func randomDeviceID() string {
 
 // Manager can provide DeviceInfo entries for incoming HTTP requests.
 type Manager struct {
-	store             sessions.Store
-	geodb             *geoIPDb
-	trustedForwarders []net.IPNet
-	remoteAddrHeader  string
+	store sessions.Store
+	geodb *geoIPDb
 }
 
 // Config stores options for the device info manager.
 type Config struct {
-	AuthKey           string   `yaml:"auth_key"`
-	GeoIPDataFiles    []string `yaml:"geo_ip_data_files"`
-	TrustedForwarders []string `yaml:"trusted_forwarders"`
-	RemoteAddrHeader  string   `yaml:"remote_addr_header"`
+	AuthKey        string   `yaml:"auth_key"`
+	GeoIPDataFiles []string `yaml:"geo_ip_data_files"`
 }
 
 // New returns a new Manager with the given configuration.
@@ -43,27 +39,15 @@ func New(config *Config) (*Manager, error) {
 	if config == nil {
 		config = &Config{}
 	}
-	tf, err := parseIPNetList(config.TrustedForwarders)
-	if err != nil {
-		return nil, err
-	}
 
-	var geodb *geoIPDb
-	if geodb, err = newGeoIP(config.GeoIPDataFiles); err != nil {
+	geodb, err := newGeoIP(config.GeoIPDataFiles)
+	if err != nil {
 		log.Printf("Warning: GeoIP disabled: %v", err)
 	}
 
-	// The remote IP header (if any) defaults to X-Forwarded-For.
-	hdr := "X-Forwarded-For"
-	if config.RemoteAddrHeader != "" {
-		hdr = config.RemoteAddrHeader
-	}
-
 	return &Manager{
-		geodb:             geodb,
-		store:             newStore([]byte(config.AuthKey)),
-		trustedForwarders: tf,
-		remoteAddrHeader:  hdr,
+		geodb: geodb,
+		store: newStore([]byte(config.AuthKey)),
 	}, nil
 }
 
@@ -80,7 +64,11 @@ func (m *Manager) GetDeviceInfoFromRequest(w http.ResponseWriter, req *http.Requ
 		// Generate a new Device ID and save it on the client.
 		devID = randomDeviceID()
 		session.Values["id"] = devID
-		session.Save(req, w)
+		if err := session.Save(req, w); err != nil {
+			// This is likely a misconfiguration issue, so
+			// we want to know about it.
+			log.Printf("error saving device manager session: %v", err)
+		}
 	}
 
 	uaStr := req.UserAgent()
@@ -100,7 +88,7 @@ func (m *Manager) GetDeviceInfoFromRequest(w http.ResponseWriter, req *http.Requ
 		d.RemoteZone = "onion"
 	} else {
 		// If we have an IP address, we can add more information.
-		if ip := m.getIPFromRequest(req); ip != nil {
+		if ip := getIPFromRequest(req); ip != nil {
 			d.RemoteAddr = ip.String()
 
 			// Country lookup.
@@ -114,3 +102,12 @@ func (m *Manager) GetDeviceInfoFromRequest(w http.ResponseWriter, req *http.Requ
 
 	return &d
 }
+
+func getIPFromRequest(req *http.Request) net.IP {
+	// Parse the RemoteAddr Request field, for starters.
+	host, _, err := net.SplitHostPort(req.RemoteAddr)
+	if err != nil {
+		host = req.RemoteAddr
+	}
+	return net.ParseIP(host)
+}

vendor/git.autistici.org/ai3/go-common/serverutil/http.go

@@ -20,43 +20,63 @@ var gracefulShutdownTimeout = 3 * time.Second
 type ServerConfig struct {
 	TLS                 *TLSServerConfig `yaml:"tls"`
 	MaxInflightRequests int              `yaml:"max_inflight_requests"`
+	TrustedForwarders   []string         `yaml:"trusted_forwarders"`
 }
 
-// Serve HTTP(S) content on the specified address. If serverConfig is
-// not nil, enable HTTPS and TLS authentication.
-//
-// This function will return an error if there are problems creating
-// the listener, otherwise it will handle graceful termination on
-// SIGINT or SIGTERM and return nil.
-func Serve(h http.Handler, serverConfig *ServerConfig, addr string) (err error) {
+func (config *ServerConfig) buildHTTPServer(h http.Handler, addr string) (*http.Server, error) {
 	var tlsConfig *tls.Config
-	if serverConfig != nil {
-		if serverConfig.TLS != nil {
-			tlsConfig, err = serverConfig.TLS.TLSConfig()
+	var err error
+	if config != nil {
+		if config.TLS != nil {
+			tlsConfig, err = config.TLS.TLSConfig()
 			if err != nil {
-				return err
+				return nil, err
 			}
-			h, err = serverConfig.TLS.TLSAuthWrapper(h)
+			h, err = config.TLS.TLSAuthWrapper(h)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		// If TrustedForwarders is defined, rewrite the request
+		// headers using X-Forwarded-Proto and X-Real-IP.
+		if len(config.TrustedForwarders) > 0 {
+			h, err = newProxyHeaders(h, config.TrustedForwarders)
 			if err != nil {
-				return err
+				return nil, err
 			}
 		}
 
-		if serverConfig.MaxInflightRequests > 0 {
-			h = newLoadSheddingWrapper(serverConfig.MaxInflightRequests, h)
+		// If MaxInflightRequests is set, enable the load
+		// shedding wrapper.
+		if config.MaxInflightRequests > 0 {
+			h = newLoadSheddingWrapper(config.MaxInflightRequests, h)
 		}
 	}
 
 	// These are not meant to be external-facing servers, so we
 	// can be generous with the timeouts to keep the number of
 	// reconnections low.
-	srv := &http.Server{
+	return &http.Server{
 		Addr:         addr,
 		Handler:      instrumentHandler(h),
 		ReadTimeout:  30 * time.Second,
 		WriteTimeout: 30 * time.Second,
 		IdleTimeout:  600 * time.Second,
 		TLSConfig:    tlsConfig,
+	}, nil
+}
+
+// Serve HTTP(S) content on the specified address. If config.TLS is
+// not nil, enable HTTPS and TLS authentication.
+//
+// This function will return an error if there are problems creating
+// the listener, otherwise it will handle graceful termination on
+// SIGINT or SIGTERM and return nil.
+func Serve(h http.Handler, config *ServerConfig, addr string) error {
+	srv, err := config.buildHTTPServer(h, addr)
+	if err != nil {
+		return err
 	}
 
 	// Install a signal handler for gentle process termination.
@@ -80,7 +100,7 @@ func Serve(h http.Handler, serverConfig *ServerConfig, addr string) (err error)
 	}()
 	signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
 
-	if tlsConfig != nil {
+	if srv.TLSConfig != nil {
 		err = srv.ListenAndServeTLS("", "")
 	} else {
 		err = srv.ListenAndServe()

vendor/git.autistici.org/ai3/go-common/serverutil/json.go

@@ -2,6 +2,7 @@ package serverutil
 
 import (
 	"encoding/json"
+	"log"
 	"net/http"
 )
 
@@ -28,10 +29,19 @@ func DecodeJSONRequest(w http.ResponseWriter, r *http.Request, obj interface{})
 
 // EncodeJSONResponse writes an application/json response to w.
 func EncodeJSONResponse(w http.ResponseWriter, obj interface{}) {
+	data, err := json.Marshal(obj)
+	if err != nil {
+		log.Printf("JSON serialization error: %v", err)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
 	w.Header().Set("Content-Type", "application/json")
 	w.Header().Set("Pragma", "no-cache")
 	w.Header().Set("Cache-Control", "no-store")
 	w.Header().Set("Expires", "-1")
 	w.Header().Set("X-Content-Type-Options", "nosniff")
-	_ = json.NewEncoder(w).Encode(obj)
+	if _, err = w.Write(data); err != nil {
+		log.Printf("error writing response: %v", err)
+	}
 }

server/device/addr.go → vendor/git.autistici.org/ai3/go-common/serverutil/proxy_headers.go

-package device
+package serverutil
 
 import (
 	"fmt"
 	"net"
 	"net/http"
-	"strings"
+
+	"github.com/gorilla/handlers"
 )
 
-func (m *Manager) getIPFromRequest(req *http.Request) net.IP {
-	// Parse the RemoteAddr Request field, for starters.
-	host, _, err := net.SplitHostPort(req.RemoteAddr)
+type proxyHeaders struct {
+	wrap, phWrap http.Handler
+	forwarders   []net.IPNet
+}
+
+func newProxyHeaders(h http.Handler, trustedForwarders []string) (http.Handler, error) {
+	f, err := parseIPNetList(trustedForwarders)
 	if err != nil {
-		host = req.RemoteAddr
-	}
-	ip := net.ParseIP(host)
-	if ip == nil {
-		return nil
+		return nil, err
 	}
+	return &proxyHeaders{
+		wrap:       h,
+		phWrap:     handlers.ProxyHeaders(h),
+		forwarders: f,
+	}, nil
+}
 
-	// See if it's a trusted forwarder, in which case go for the
-	// X-Forwarded-For header.
-	if matchIPNetList(ip, m.trustedForwarders) {
-		fwdAddr := req.Header.Get(m.remoteAddrHeader)
-		if fwdAddr == "" {
-			return nil
-		}
-		ip = net.ParseIP(strings.Split(fwdAddr, ", ")[0])
-		if ip == nil {
-			return nil
-		}
+func (p *proxyHeaders) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	host, _, err := net.SplitHostPort(r.RemoteAddr)
+	if err != nil {
+		host = r.RemoteAddr
 	}
-
-	return ip
+	ip := net.ParseIP(host)
+	if ip != nil && matchIPNetList(ip, p.forwarders) {
+		p.phWrap.ServeHTTP(w, r)
+		return
+	}
+	p.wrap.ServeHTTP(w, r)
 }
 
 func fullMask(ip net.IP) net.IPMask {

vendor/vendor.json

@@ -15,10 +15,10 @@
 			"revisionTime": "2017-12-17T20:32:41Z"
 		},
 		{
-			"checksumSHA1": "nlGRxexjZUxnHc/z/+ZqV/Xq51w=",
+			"checksumSHA1": "WZkkasKZTt4UoJHphxXqZauBGLk=",
 			"path": "git.autistici.org/ai3/go-common/serverutil",
-			"revision": "aa88011352b67032c19b0c14eaa06b3417176753",
-			"revisionTime": "2017-12-19T17:20:40Z"
+			"revision": "be4da336395850dd80cfaf36bfa0e364ec22a30f",
+			"revisionTime": "2018-01-14T09:57:18Z"
 		},
 		{
 			"checksumSHA1": "C1BVHHj8iBgBN5EWr3ucsGoEzew=",