Don't truncate IPv6 addresses (#19573)

* Bugfix: Fix parsing of IPv6 addresses

Make sure that IPv6 addresses aren't truncated when parsing. Fixes #18924
* util: Change network address parsing funcs to return error
* pkg/api: Return NetworkAddress instead of host/port

pkg/services/auth/auth_token.go

@@ -54,8 +54,12 @@ func (s *UserAuthTokenService) ActiveTokenCount(ctx context.Context) (int64, err
 	return count, err
 }
 
-func (s *UserAuthTokenService) CreateToken(ctx context.Context, userId int64, clientIP, userAgent string) (*models.UserToken, error) {
+func (s *UserAuthTokenService) CreateToken(ctx context.Context, userId int64, clientAddr, userAgent string) (*models.UserToken, error) {
-	clientIP = util.ParseIPAddress(clientIP)
+	clientIP, err := util.ParseIPAddress(clientAddr)
+	if err != nil {
+		s.log.Debug("Failed to parse client IP address", "clientAddr", clientAddr, "err", err)
+		clientIP = ""
+	}
 	token, err := util.RandomHex(16)
 	if err != nil {
 		return nil, err
@@ -191,7 +195,8 @@ func (s *UserAuthTokenService) LookupToken(ctx context.Context, unhashedToken st
 	return &userToken, err
 }
 
-func (s *UserAuthTokenService) TryRotateToken(ctx context.Context, token *models.UserToken, clientIP, userAgent string) (bool, error) {
+func (s *UserAuthTokenService) TryRotateToken(ctx context.Context, token *models.UserToken,
+	clientAddr, userAgent string) (bool, error) {
 	if token == nil {
 		return false, nil
 	}
@@ -214,7 +219,12 @@ func (s *UserAuthTokenService) TryRotateToken(ctx context.Context, token *models
 
 	s.log.Debug("token needs rotation", "tokenId", model.Id, "authTokenSeen", model.AuthTokenSeen, "rotatedAt", rotatedAt)
 
-	clientIP = util.ParseIPAddress(clientIP)
+	clientIP, err := util.ParseIPAddress(clientAddr)
+	if err != nil {
+		s.log.Debug("Failed to parse client IP address", "clientAddr", clientAddr, "err", err)
+		clientIP = ""
+	}
+
 	newToken, err := util.RandomHex(16)
 	if err != nil {
 		return false, err

pkg/services/sqlstore/sqlstore.go

@@ -24,6 +24,7 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 	_ "github.com/grafana/grafana/pkg/tsdb/mssql"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/errutil"
 	_ "github.com/lib/pq"
 )
 
@@ -185,14 +186,18 @@ func (ss *SqlStore) buildConnectionString() (string, error) {
 
 		cnnstr += ss.buildExtraConnectionString('&')
 	case migrator.POSTGRES:
-		host, port := util.SplitHostPortDefault(ss.dbCfg.Host, "127.0.0.1", "5432")
+		addr, err := util.SplitHostPortDefault(ss.dbCfg.Host, "127.0.0.1", "5432")
+		if err != nil {
+			return "", errutil.Wrapf(err, "Invalid host specifier '%s'", ss.dbCfg.Host)
+		}
+
 		if ss.dbCfg.Pwd == "" {
 			ss.dbCfg.Pwd = "''"
 		}
 		if ss.dbCfg.User == "" {
 			ss.dbCfg.User = "''"
 		}
-		cnnstr = fmt.Sprintf("user=%s password=%s host=%s port=%s dbname=%s sslmode=%s sslcert=%s sslkey=%s sslrootcert=%s", ss.dbCfg.User, ss.dbCfg.Pwd, host, port, ss.dbCfg.Name, ss.dbCfg.SslMode, ss.dbCfg.ClientCertPath, ss.dbCfg.ClientKeyPath, ss.dbCfg.CaCertPath)
+		cnnstr = fmt.Sprintf("user=%s password=%s host=%s port=%s dbname=%s sslmode=%s sslcert=%s sslkey=%s sslrootcert=%s", ss.dbCfg.User, ss.dbCfg.Pwd, addr.Host, addr.Port, ss.dbCfg.Name, ss.dbCfg.SslMode, ss.dbCfg.ClientCertPath, ss.dbCfg.ClientKeyPath, ss.dbCfg.CaCertPath)
 
 		cnnstr += ss.buildExtraConnectionString(' ')
 	case migrator.SQLITE:

pkg/services/sqlstore/sqlstore_test.go

@@ -55,13 +55,13 @@ var sqlStoreTestCases = []sqlStoreTest{
 	{
 		name:          "MySQL IPv6 (Default Port)",
 		dbType:        "mysql",
-		dbHost:        "::1",
+		dbHost:        "[::1]",
-		connStrValues: []string{"tcp(::1)"},
+		connStrValues: []string{"tcp([::1])"},
 	},
 	{
 		name:          "Postgres IPv6 (Default Port)",
 		dbType:        "postgres",
-		dbHost:        "::1",
+		dbHost:        "[::1]",
 		connStrValues: []string{"host=::1", "port=5432"},
 	},
 }

pkg/tsdb/mssql/mssql.go

@@ -14,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/tsdb"
 	"github.com/grafana/grafana/pkg/tsdb/sqleng"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 func init() {
@@ -46,12 +47,15 @@ func newMssqlQueryEndpoint(datasource *models.DataSource) (tsdb.TsdbQueryEndpoin
 }
 
 func generateConnectionString(datasource *models.DataSource) (string, error) {
-	server, port := util.SplitHostPortDefault(datasource.Url, "localhost", "1433")
+	addr, err := util.SplitHostPortDefault(datasource.Url, "localhost", "1433")
+	if err != nil {
+		return "", errutil.Wrapf(err, "Invalid data source URL '%s'", datasource.Url)
+	}
 
 	encrypt := datasource.JsonData.Get("encrypt").MustString("false")
 	connStr := fmt.Sprintf("server=%s;port=%s;database=%s;user id=%s;password=%s;",
-		server,
+		addr.Host,
-		port,
+		addr.Port,
 		datasource.Database,
 		datasource.User,
 		datasource.DecryptedPassword(),

pkg/util/ip_address.go

@@ -1,54 +1,86 @@
 package util
 
 import (
+	"fmt"
 	"net"
 	"strings"
+
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 // ParseIPAddress parses an IP address and removes port and/or IPV6 format
-func ParseIPAddress(input string) string {
+func ParseIPAddress(input string) (string, error) {
-	host, _ := SplitHostPort(input)
+	addr, err := SplitHostPort(input)
-
+	if err != nil {
-	ip := net.ParseIP(host)
+		return "", errutil.Wrapf(err, "Failed to split network address '%s' by host and port",
+			input)
+	}
 
+	ip := net.ParseIP(addr.Host)
 	if ip == nil {
-		return host
+		return addr.Host, nil
 	}
 
 	if ip.IsLoopback() {
-		return "127.0.0.1"
+		if strings.Contains(addr.Host, ":") {
+			// IPv6
+			return "::1", nil
+		}
+		return "127.0.0.1", nil
 	}
 
-	return ip.String()
+	return ip.String(), nil
+}
+
+type NetworkAddress struct {
+	Host string
+	Port string
 }
 
 // SplitHostPortDefault splits ip address/hostname string by host and port. Defaults used if no match found
-func SplitHostPortDefault(input, defaultHost, defaultPort string) (host string, port string) {
+func SplitHostPortDefault(input, defaultHost, defaultPort string) (NetworkAddress, error) {
-	port = defaultPort
+	addr := NetworkAddress{
-	s := input
+		Host: defaultHost,
-	lastIndex := strings.LastIndex(input, ":")
+		Port: defaultPort,
-
+	}
-	if lastIndex != -1 {
+	if len(input) == 0 {
-		if lastIndex > 0 && input[lastIndex-1:lastIndex] != ":" {
+		return addr, fmt.Errorf("Input is empty")
-			s = input[:lastIndex]
-			port = input[lastIndex+1:]
-		} else if lastIndex == 0 {
-			s = defaultHost
-			port = input[lastIndex+1:]
-		}
-	} else {
-		port = defaultPort
 	}
 
-	s = strings.Replace(s, "[", "", -1)
+	start := 0
-	s = strings.Replace(s, "]", "", -1)
+	// Determine if IPv6 address, in which case IP address will be enclosed in square brackets
-	port = strings.Replace(port, "[", "", -1)
+	if strings.Index(input, "[") == 0 {
-	port = strings.Replace(port, "]", "", -1)
+		addrEnd := strings.LastIndex(input, "]")
+		if addrEnd < 0 {
+			// Malformed address
+			return addr, fmt.Errorf("Malformed IPv6 address: '%s'", input)
+		}
 
-	return s, port
+		start = addrEnd
+	}
+	if strings.LastIndex(input[start:], ":") < 0 {
+		// There's no port section of the input
+		// It's still useful to call net.SplitHostPort though, since it removes IPv6
+		// square brackets from the address
+		input = fmt.Sprintf("%s:%s", input, defaultPort)
+	}
+
+	host, port, err := net.SplitHostPort(input)
+	if err != nil {
+		return addr, errutil.Wrapf(err, "net.SplitHostPort failed for '%s'", input)
+	}
+
+	if len(host) > 0 {
+		addr.Host = host
+	}
+	if len(port) > 0 {
+		addr.Port = port
+	}
+
+	return addr, nil
 }
 
 // SplitHostPort splits ip address/hostname string by host and port
-func SplitHostPort(input string) (host string, port string) {
+func SplitHostPort(input string) (NetworkAddress, error) {
 	return SplitHostPortDefault(input, "", "")
 }

pkg/util/ip_address_test.go

@@ -4,95 +4,127 @@ import (
 	"testing"
 
 	. "github.com/smartystreets/goconvey/convey"
+	"golang.org/x/xerrors"
 )
 
 func TestParseIPAddress(t *testing.T) {
 	Convey("Test parse ip address", t, func() {
-		So(ParseIPAddress("192.168.0.140:456"), ShouldEqual, "192.168.0.140")
+		addr, err := ParseIPAddress("192.168.0.140:456")
-		So(ParseIPAddress("192.168.0.140"), ShouldEqual, "192.168.0.140")
+		So(err, ShouldBeNil)
-		So(ParseIPAddress("[::1:456]"), ShouldEqual, "127.0.0.1")
+		So(addr, ShouldEqual, "192.168.0.140")
-		So(ParseIPAddress("[::1]"), ShouldEqual, "127.0.0.1")
+
-		So(ParseIPAddress("::1"), ShouldEqual, "127.0.0.1")
+		addr, err = ParseIPAddress("192.168.0.140")
-		So(ParseIPAddress("::1:123"), ShouldEqual, "127.0.0.1")
+		So(err, ShouldBeNil)
+		So(addr, ShouldEqual, "192.168.0.140")
+
+		addr, err = ParseIPAddress("[::1]:456")
+		So(err, ShouldBeNil)
+		So(addr, ShouldEqual, "::1")
+
+		addr, err = ParseIPAddress("[::1]")
+		So(err, ShouldBeNil)
+		So(addr, ShouldEqual, "::1")
+	})
+
+	Convey("Invalid address", t, func() {
+		_, err := ParseIPAddress("[::1")
+		So(err, ShouldBeError, xerrors.Errorf(
+			"Failed to split network address '[::1' by host and port: Malformed IPv6 address: '[::1'"))
+
+		_, err = ParseIPAddress("::1]")
+		So(err, ShouldBeError, xerrors.Errorf(
+			"Failed to split network address '::1]' by host and port: net.SplitHostPort failed for '::1]': address ::1]: too many colons in address"))
+
+		_, err = ParseIPAddress("")
+		So(err, ShouldBeError, xerrors.Errorf(
+			"Failed to split network address '' by host and port: Input is empty"))
+	})
+
+	Convey("Loopback address", t, func() {
+		addr, err := ParseIPAddress("127.0.0.1")
+		So(err, ShouldBeNil)
+		So(addr, ShouldEqual, "127.0.0.1")
+
+		addr, err = ParseIPAddress("[::1]")
+		So(err, ShouldBeNil)
+		So(addr, ShouldEqual, "::1")
 	})
 }
 
 func TestSplitHostPortDefault(t *testing.T) {
 	Convey("Test split ip address to host and port", t, func() {
-		host, port := SplitHostPortDefault("192.168.0.140:456", "", "")
+		addr, err := SplitHostPortDefault("192.168.0.140:456", "", "")
-		So(host, ShouldEqual, "192.168.0.140")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "192.168.0.140")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPortDefault("192.168.0.140", "", "123")
+		addr, err = SplitHostPortDefault("192.168.0.140", "", "123")
-		So(host, ShouldEqual, "192.168.0.140")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "123")
+		So(addr.Host, ShouldEqual, "192.168.0.140")
+		So(addr.Port, ShouldEqual, "123")
 
-		host, port = SplitHostPortDefault("[::1:456]", "", "")
+		addr, err = SplitHostPortDefault("[::1]:456", "", "")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "::1")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPortDefault("[::1]", "", "123")
+		addr, err = SplitHostPortDefault("[::1]", "", "123")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "123")
+		So(addr.Host, ShouldEqual, "::1")
+		So(addr.Port, ShouldEqual, "123")
 
-		host, port = SplitHostPortDefault("::1:123", "", "")
+		addr, err = SplitHostPortDefault(":456", "1.2.3.4", "")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "123")
+		So(addr.Host, ShouldEqual, "1.2.3.4")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPortDefault("::1", "", "123")
+		addr, err = SplitHostPortDefault("xyz.rds.amazonaws.com", "", "123")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "123")
+		So(addr.Host, ShouldEqual, "xyz.rds.amazonaws.com")
+		So(addr.Port, ShouldEqual, "123")
 
-		host, port = SplitHostPortDefault(":456", "1.2.3.4", "")
+		addr, err = SplitHostPortDefault("xyz.rds.amazonaws.com:123", "", "")
-		So(host, ShouldEqual, "1.2.3.4")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "xyz.rds.amazonaws.com")
-
+		So(addr.Port, ShouldEqual, "123")
-		host, port = SplitHostPortDefault("xyz.rds.amazonaws.com", "", "123")
-		So(host, ShouldEqual, "xyz.rds.amazonaws.com")
-		So(port, ShouldEqual, "123")
-
-		host, port = SplitHostPortDefault("xyz.rds.amazonaws.com:123", "", "")
-		So(host, ShouldEqual, "xyz.rds.amazonaws.com")
-		So(port, ShouldEqual, "123")
 	})
 }
 
 func TestSplitHostPort(t *testing.T) {
 	Convey("Test split ip address to host and port", t, func() {
-		host, port := SplitHostPort("192.168.0.140:456")
+		addr, err := SplitHostPort("192.168.0.140:456")
-		So(host, ShouldEqual, "192.168.0.140")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "192.168.0.140")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPort("192.168.0.140")
+		addr, err = SplitHostPort("192.168.0.140")
-		So(host, ShouldEqual, "192.168.0.140")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "")
+		So(addr.Host, ShouldEqual, "192.168.0.140")
+		So(addr.Port, ShouldEqual, "")
 
-		host, port = SplitHostPort("[::1:456]")
+		addr, err = SplitHostPort("[::1]:456")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "::1")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPort("[::1]")
+		addr, err = SplitHostPort("[::1]")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "")
+		So(addr.Host, ShouldEqual, "::1")
+		So(addr.Port, ShouldEqual, "")
 
-		host, port = SplitHostPort("::1:123")
+		addr, err = SplitHostPort(":456")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "123")
+		So(addr.Host, ShouldEqual, "")
+		So(addr.Port, ShouldEqual, "456")
 
-		host, port = SplitHostPort("::1")
+		addr, err = SplitHostPort("xyz.rds.amazonaws.com")
-		So(host, ShouldEqual, "::1")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "")
+		So(addr.Host, ShouldEqual, "xyz.rds.amazonaws.com")
+		So(addr.Port, ShouldEqual, "")
 
-		host, port = SplitHostPort(":456")
+		addr, err = SplitHostPort("xyz.rds.amazonaws.com:123")
-		So(host, ShouldEqual, "")
+		So(err, ShouldBeNil)
-		So(port, ShouldEqual, "456")
+		So(addr.Host, ShouldEqual, "xyz.rds.amazonaws.com")
-
+		So(addr.Port, ShouldEqual, "123")
-		host, port = SplitHostPort("xyz.rds.amazonaws.com")
-		So(host, ShouldEqual, "xyz.rds.amazonaws.com")
-		So(port, ShouldEqual, "")
-
-		host, port = SplitHostPort("xyz.rds.amazonaws.com:123")
-		So(host, ShouldEqual, "xyz.rds.amazonaws.com")
-		So(port, ShouldEqual, "123")
 	})
 }