Disable redirect of HTTP request to a HTTPS Minio server (#4454)

Fixes #4452

appveyor.yml

@@ -36,9 +36,9 @@ test_script:
   # Unit tests
   - ps: Add-AppveyorTest "Unit Tests" -Outcome Running
   - mkdir build\coverage
-  - go test -timeout 17m -race github.com/minio/minio/cmd...
-  - go test -race github.com/minio/minio/pkg...
-  - go test -coverprofile=build\coverage\coverage.txt -covermode=atomic github.com/minio/minio/cmd
+  - go test -v -timeout 17m -race github.com/minio/minio/cmd...
+  - go test -v -race github.com/minio/minio/pkg...
+  - go test -v -coverprofile=build\coverage\coverage.txt -covermode=atomic github.com/minio/minio/cmd
   - ps: Update-AppveyorTest "Unit Tests" -Outcome Passed
 
 after_test:

cmd/api-errors.go

@@ -149,6 +149,7 @@ const (
 	ErrAdminInvalidAccessKey
 	ErrAdminInvalidSecretKey
 	ErrAdminConfigNoQuorum
+	ErrInsecureClientRequest
 )
 
 // error code to APIError structure, these fields carry respective
@@ -618,6 +619,11 @@ var errorCodeResponse = map[APIErrorCode]APIError{
 		Description:    "Configuration update failed because server quorum was not met",
 		HTTPStatusCode: http.StatusServiceUnavailable,
 	},
+	ErrInsecureClientRequest: {
+		Code:           "XMinioInsecureClientRequest",
+		Description:    "Cannot respond to plain-text request from TLS-encrypted server",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 
 	// Add your error structure here.
 }

cmd/server-mux.go

@@ -449,25 +449,9 @@ func (m *ServerMux) ListenAndServe(certFile, keyFile string) (err error) {
 	// All http requests start to be processed by httpHandler
 	httpHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if tlsEnabled && r.TLS == nil {
-			// It is expected that r.Host might not have port
-			// for standard ports such as "80" and "443".
-			host, port, _ := net.SplitHostPort(r.Host)
-			if port == "" {
-				host = net.JoinHostPort(r.Host, globalMinioPort)
-			} else {
-				host = r.Host
-			}
-			// TLS is enabled but Request is not TLS configured
-			u := url.URL{
-				Scheme:   httpsScheme,
-				Opaque:   r.URL.Opaque,
-				User:     r.URL.User,
-				Host:     host,
-				Path:     r.URL.Path,
-				RawQuery: r.URL.RawQuery,
-				Fragment: r.URL.Fragment,
-			}
-			http.Redirect(w, r, u.String(), http.StatusTemporaryRedirect)
+			// TLS is enabled but request is not TLS
+			// configured - return error to client.
+			writeErrorResponse(w, ErrInsecureClientRequest, &url.URL{})
 		} else {
 
 			// Return ServiceUnavailable for clients which are sending requests
@@ -481,7 +465,7 @@ func (m *ServerMux) ListenAndServe(certFile, keyFile string) (err error) {
 			}
 
 			// Execute registered handlers, update currentReqs to keep
-			// tracks of current requests currently processed by the server
+			// track of concurrent requests processing on the server
 			atomic.AddInt32(&m.currentReqs, 1)
 			m.handler.ServeHTTP(w, r)
 			atomic.AddInt32(&m.currentReqs, -1)

cmd/server-mux_test.go

@@ -31,6 +31,7 @@ import (
 	"net/http"
 	"os"
 	"runtime"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -339,6 +340,11 @@ func TestServerListenAndServePlain(t *testing.T) {
 }
 
 func TestServerListenAndServeTLS(t *testing.T) {
+	_, err := newTestConfig(globalMinioDefaultRegion)
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+
 	wait := make(chan struct{})
 	addr := net.JoinHostPort("127.0.0.1", getFreePort())
 	errc := make(chan error)
@@ -354,7 +360,7 @@ func TestServerListenAndServeTLS(t *testing.T) {
 	}))
 
 	// Create a cert
-	err := createConfigDir()
+	err = createConfigDir()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -374,7 +380,6 @@ func TestServerListenAndServeTLS(t *testing.T) {
 
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
-	// Keep trying the server until it's accepting connections
 	go func() {
 		tr := &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
@@ -384,6 +389,7 @@ func TestServerListenAndServeTLS(t *testing.T) {
 			Transport: tr,
 		}
 		okTLS := false
+		// Keep trying the server until it's accepting connections
 		for !okTLS {
 			res, _ := client.Get("https://" + addr)
 			if res != nil && res.StatusCode == http.StatusOK {
@@ -391,14 +397,27 @@ func TestServerListenAndServeTLS(t *testing.T) {
 			}
 		}
 
-		okNoTLS := false
-		for !okNoTLS {
-			res, _ := client.Get("http://" + addr)
-			// Without TLS we expect a re-direction from http to https
-			// And also the request is not rejected.
-			if res != nil && res.StatusCode == http.StatusOK && res.Request.URL.Scheme == httpsScheme {
-				okNoTLS = true
-			}
+		// Once a request succeeds, subsequent requests should
+		// work fine.
+		res, err := client.Get("http://" + addr)
+		if err != nil {
+			t.Errorf("Got unexpected error: %v", err)
+		}
+		// Without TLS we expect a Bad-Request response from the server.
+		if !(res != nil && res.StatusCode == http.StatusBadRequest && res.Request.URL.Scheme == httpScheme) {
+			t.Fatalf("Plaintext request to TLS server did not have expected response!")
+		}
+
+		body, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			t.Errorf("Error reading body")
+		}
+
+		// Check that the expected error is received.
+		bodyStr := string(body)
+		apiErr := getAPIError(ErrInsecureClientRequest)
+		if !(strings.Contains(bodyStr, apiErr.Code) && strings.Contains(bodyStr, apiErr.Description)) {
+			t.Fatalf("Plaintext request to TLS server did not have expected response body!")
 		}
 		wg.Done()
 	}()