Auth: Improve IP sanitization and security logs #808

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/entity/auth_session_login.go

@@ -122,7 +122,7 @@ func AuthLocal(user *User, f form.Login, s *Session, c *gin.Context) (provider a
 			message := authn.ErrInvalidUsername.Error()
 
 			if s != nil {
-				event.AuditErr([]string{clientIp, "session %s", "login as %s with app password", message}, s.RefID, clean.LogQuote(username))
+				event.AuditErr([]string{clientIp, "session %s", "login as %s", "app password", message}, s.RefID, clean.LogQuote(username))
 				event.LoginError(clientIp, "api", username, s.UserAgent, message)
 				s.Status = http.StatusUnauthorized
 			}
@@ -137,7 +137,7 @@ func AuthLocal(user *User, f form.Login, s *Session, c *gin.Context) (provider a
 			}
 
 			if s != nil {
-				event.AuditErr([]string{clientIp, "session %s", "login as %s with app password", message}, s.RefID, clean.LogQuote(username))
+				event.AuditErr([]string{clientIp, "session %s", "login as %s", "app password", message}, s.RefID, clean.LogQuote(username))
 				event.LoginError(clientIp, "api", username, s.UserAgent, message)
 				s.Status = http.StatusUnauthorized
 			}
@@ -165,7 +165,7 @@ func AuthLocal(user *User, f form.Login, s *Session, c *gin.Context) (provider a
 					s.SessExpires = authSess.SessExpires
 				}
 
-				event.AuditInfo([]string{clientIp, "session %s", "login as %s with app password", authn.Succeeded}, s.RefID, clean.LogQuote(username))
+				event.AuditInfo([]string{clientIp, "session %s", "login as %s", "app password", authn.Succeeded}, s.RefID, clean.LogQuote(username))
 				event.LoginInfo(clientIp, "api", username, s.UserAgent)
 			}
 
@@ -195,7 +195,6 @@ func AuthLocal(user *User, f form.Login, s *Session, c *gin.Context) (provider a
 
 			if s != nil {
 				event.AuditInfo([]string{clientIp, "session %s", "login as %s", err.Error()}, s.RefID, clean.LogQuote(username))
-				event.LoginError(clientIp, "api", username, s.UserAgent, err.Error())
 				s.Status = http.StatusUnauthorized
 			}
 

pkg/clean/clip.go

@@ -4,6 +4,7 @@ import "strings"
 
 const (
 	ClipShortType = 8
+	ClipIPv6      = 39
 	ClipType      = 64
 )
 

pkg/clean/ip.go

@@ -0,0 +1,34 @@
+package clean
+
+import (
+	"net"
+	"regexp"
+)
+
+// IpRegExp matches characters allowed in IPv4 or IPv6 network addresses.
+var IpRegExp = regexp.MustCompile(`[^a-zA-Z0-9:.]`)
+
+// IP returns the sanitized and normalized network address if it is valid, or the default otherwise.
+func IP(s, defaultIp string) string {
+	// Return default if invalid.
+	if s == "" || len(s) > MaxLength || s == defaultIp {
+		return defaultIp
+	}
+
+	// Remove invalid characters, including whitespace.
+	if s = IpRegExp.ReplaceAllString(s, ""); s == "" {
+		return defaultIp
+	}
+
+	// Limit string length to 39 characters.
+	if len(s) > ClipIPv6 {
+		s = s[:ClipIPv6]
+	}
+
+	// Parse IP address and return it as string.
+	if ip := net.ParseIP(s); ip == nil {
+		return defaultIp
+	} else {
+		return ip.String()
+	}
+}

pkg/clean/ip_test.go

@@ -0,0 +1,43 @@
+package clean
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIP(t *testing.T) {
+	t.Run("Empty", func(t *testing.T) {
+		assert.Equal(t, "0.0.0.0", IP("", "0.0.0.0"))
+	})
+	t.Run("Unknown", func(t *testing.T) {
+		assert.Equal(t, "0.0.0.0", IP("0.0.0.0", "0.0.0.0"))
+	})
+	t.Run("Localhost", func(t *testing.T) {
+		assert.Equal(t, "127.0.0.1", IP("127.0.0.1", "0.0.0.0"))
+	})
+	t.Run("IPv6", func(t *testing.T) {
+		assert.Equal(t, "2001:0:130f::9c0:876a:130b", IP("2001:0000:130F:0000:0000:09C0:876A:130B", "0.0.0.0"))
+	})
+	t.Run("IPv6", func(t *testing.T) {
+		assert.Equal(t, "2001:0:130f::9c0:876a:130b", IP("    2001:0000:130F:0000:0000:09C0:876A:130B    ", "0.0.0.0"))
+	})
+	t.Run("PublicIPv4", func(t *testing.T) {
+		assert.Equal(t, "8.8.8.8", IP("8.8.8.8", "0.0.0.0"))
+	})
+	t.Run("PrivateIPv4", func(t *testing.T) {
+		assert.Equal(t, "192.168.1.128", IP("192.168.1.128", "0.0.0.0"))
+	})
+	t.Run("UUID", func(t *testing.T) {
+		assert.Equal(t, "0.0.0.0", IP("123e4567-e89b-12d3-A456-426614174000", "0.0.0.0"))
+	})
+	t.Run("Hello", func(t *testing.T) {
+		assert.Equal(t, "0.0.0.0", IP("Hello", "0.0.0.0"))
+	})
+	t.Run("Default", func(t *testing.T) {
+		assert.Equal(t, "default", IP("Hello", "default"))
+	})
+	t.Run("EmptyDefault", func(t *testing.T) {
+		assert.Equal(t, "", IP("Hello", ""))
+	})
+}

pkg/header/request.go

@@ -2,10 +2,13 @@ package header
 
 import (
 	"github.com/gin-gonic/gin"
+
+	"github.com/photoprism/photoprism/pkg/clean"
 )
 
 const (
 	UnknownIP = "0.0.0.0"
+	LocalIP   = "127.0.0.1"
 )
 
 // ClientIP returns the client IP address from the request context or a placeholder if it is unknown.
@@ -16,9 +19,9 @@ func ClientIP(c *gin.Context) (ip string) {
 	} else if c.Request == nil {
 		return UnknownIP
 	} else if ip = c.ClientIP(); ip != "" {
-		return ip
+		return clean.IP(ip, UnknownIP)
 	} else if ip = c.RemoteIP(); ip != "" {
-		return ip
+		return clean.IP(ip, UnknownIP)
 	}
 
 	// Tests may not specify an IP address.