From 2c56788f8d94e75c3b191a36e98904b2d1deb2bd Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Thu, 8 Jun 2017 11:20:56 -0700 Subject: [PATCH] Validate gateway arguments (#4376) Fixes #4355 --- cmd/gateway-main.go | 43 +++++++++++++++++++++++++++++++--------- cmd/gateway-main_test.go | 33 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index 667869cac..82fc77711 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -21,6 +21,7 @@ import ( "fmt" "net/url" "os" + "runtime" "strings" "github.com/gorilla/mux" @@ -181,6 +182,36 @@ func parseGatewayEndpoint(arg string) (endPoint string, secure bool, err error) } } +// Validate gateway arguments. +func validateGatewayArguments(serverAddr, endpointAddr string) error { + if err := CheckLocalServerAddr(serverAddr); err != nil { + return err + } + + if runtime.GOOS == "darwin" { + _, port := mustSplitHostPort(serverAddr) + // On macOS, if a process already listens on LOCALIPADDR:PORT, net.Listen() falls back + // to IPv6 address i.e minio will start listening on IPv6 address whereas another + // (non-)minio process is listening on IPv4 of given port. + // To avoid this error situation we check for port availability only for macOS. + if err := checkPortAvailability(port); err != nil { + return err + } + } + + if endpointAddr != "" { + // Reject the endpoint if it points to the gateway handler itself. + sameTarget, err := sameLocalAddrs(endpointAddr, serverAddr) + if err != nil { + return err + } + if sameTarget { + return errors.New("endpoint points to the local gateway") + } + } + return nil +} + // Handler for 'minio gateway'. func gatewayMain(ctx *cli.Context) { if !ctx.Args().Present() || ctx.Args().First() == "help" { @@ -207,17 +238,11 @@ func gatewayMain(ctx *cli.Context) { backendType := ctx.Args().Get(0) // Second argument is the endpoint address (optional) endpointAddr := ctx.Args().Get(1) - // Third argument is the address flag + serverAddr := ctx.String("address") - if endpointAddr != "" { - // Reject the endpoint if it points to the gateway handler itself. - sameTarget, err := sameLocalAddrs(endpointAddr, serverAddr) - fatalIf(err, "Unable to compare server and endpoint addresses.") - if sameTarget { - fatalIf(errors.New("endpoint points to the local gateway"), "Endpoint url is not allowed") - } - } + err := validateGatewayArguments(serverAddr, endpointAddr) + fatalIf(err, "Invalid argument") // Second argument is endpoint. If no endpoint is specified then the // gateway implementation should use a default setting. diff --git a/cmd/gateway-main_test.go b/cmd/gateway-main_test.go index 1779725bf..b80d0292b 100644 --- a/cmd/gateway-main_test.go +++ b/cmd/gateway-main_test.go @@ -18,6 +18,7 @@ package cmd import ( "os" + "strings" "testing" ) @@ -73,3 +74,35 @@ func TestSetBrowserFromEnv(t *testing.T) { } os.Setenv("MINIO_BROWSER", browser) } + +// Test validateGatewayArguments +func TestValidateGatewayArguments(t *testing.T) { + nonLoopBackIPs := localIP4.FuncMatch(func(ip string, matchString string) bool { + return !strings.HasPrefix(ip, "127.") + }, "") + if len(nonLoopBackIPs) == 0 { + t.Fatalf("No non-loop back IP address found for this host") + } + nonLoopBackIP := nonLoopBackIPs.ToSlice()[0] + + testCases := []struct { + serverAddr string + endpointAddr string + valid bool + }{ + {":9000", "http://localhost:9001", true}, + {":9000", "http://google.com", true}, + {"123.123.123.123:9000", "http://localhost:9000", false}, + {":9000", "http://localhost:9000", false}, + {":9000", nonLoopBackIP + ":9000", false}, + } + for i, test := range testCases { + err := validateGatewayArguments(test.serverAddr, test.endpointAddr) + if test.valid && err != nil { + t.Errorf("Test %d expected not to return error but got %s", i+1, err) + } + if !test.valid && err == nil { + t.Errorf("Test %d expected to fail but it did not", i+1) + } + } +}