jwt,browser: allow short-expiry tokens for GETs (#4684)

This commit fixes a potential security issue, whereby a full-access
token to the server would be available in the GET URL of a download
request. This fixes that issue by introducing short-expiry tokens, which
are only valid for one minute, and are regenerated for every download
request.

This commit specifically introduces the short-lived tokens, adds tests
for the tokens, adds an RPC call for generating a token given a
full-access token, updates the browser to use the new tokens for
requests where the token is passed as a GET parameter, and adds some
tests with the new temporary tokens.

Refs: https://github.com/minio/minio/pull/4673

browser/app/js/components/Browse.js

@@ -150,7 +150,16 @@ export default class Browse extends React.Component {
       if (prefix === currentPath) return
       browserHistory.push(utils.pathJoin(currentBucket, encPrefix))
     } else {
-      window.location = `${window.location.origin}/minio/download/${currentBucket}/${encPrefix}?token=${storage.getItem('token')}`
+      // Download the selected file.
+      web.CreateURLToken()
+        .then(res => {
+          let url = `${window.location.origin}/minio/download/${currentBucket}/${encPrefix}?token=${res.token}`
+          window.location = url
+        })
+        .catch(err => dispatch(actions.showAlert({
+          type: 'danger',
+          message: err.message
+        })))
     }
   }
 
@@ -406,16 +415,24 @@ export default class Browse extends React.Component {
   }
 
   downloadSelected() {
-    const {dispatch} = this.props
+    const {dispatch, web} = this.props
     let req = {
       bucketName: this.props.currentBucket,
       objects: this.props.checkedObjects,
       prefix: this.props.currentPath
     }
-    let requestUrl = location.origin + "/minio/zip?token=" + localStorage.token
+
+    web.CreateURLToken()
+      .then(res => {
+        let requestUrl = location.origin + "/minio/zip?token=" + res.token
 
         this.xhr = new XMLHttpRequest()
         dispatch(actions.downloadSelected(requestUrl, req, this.xhr))
+      })
+      .catch(err => dispatch(actions.showAlert({
+        type: 'danger',
+        message: err.message
+      })))
   }
 
   clearSelected() {

browser/app/js/web.js

@@ -112,6 +112,9 @@ export default class Web {
         return res
       })
   }
+  CreateURLToken() {
+    return this.makeCall('CreateURLToken')
+  }
   GetBucketPolicy(args) {
     return this.makeCall('GetBucketPolicy', args)
   }

cmd/jwt.go

@@ -34,6 +34,9 @@ const (
 
 	// Inter-node JWT token expiry is 100 years approx.
 	defaultInterNodeJWTExpiry = 100 * 365 * 24 * time.Hour
+
+	// URL JWT token expiry is one minute (might be exposed).
+	defaultURLJWTExpiry = time.Minute
 )
 
 var (
@@ -77,6 +80,10 @@ func authenticateWeb(accessKey, secretKey string) (string, error) {
 	return authenticateJWT(accessKey, secretKey, defaultJWTExpiry)
 }
 
+func authenticateURL(accessKey, secretKey string) (string, error) {
+	return authenticateJWT(accessKey, secretKey, defaultURLJWTExpiry)
+}
+
 func keyFuncCallback(jwtToken *jwtgo.Token) (interface{}, error) {
 	if _, ok := jwtToken.Method.(*jwtgo.SigningMethodHMAC); !ok {
 		return nil, fmt.Errorf("Unexpected signing method: %v", jwtToken.Header["alg"])

cmd/jwt_test.go

@@ -60,6 +60,8 @@ func testAuthenticate(authType string, t *testing.T) {
 			_, err = authenticateNode(testCase.accessKey, testCase.secretKey)
 		} else if authType == "web" {
 			_, err = authenticateWeb(testCase.accessKey, testCase.secretKey)
+		} else if authType == "url" {
+			_, err = authenticateURL(testCase.accessKey, testCase.secretKey)
 		}
 
 		if testCase.expectedErr != nil {
@@ -83,6 +85,10 @@ func TestAuthenticateWeb(t *testing.T) {
 	testAuthenticate("web", t)
 }
 
+func TestAuthenticateURL(t *testing.T) {
+	testAuthenticate("url", t)
+}
+
 func BenchmarkAuthenticateNode(b *testing.B) {
 	testPath, err := newTestConfig(globalMinioDefaultRegion)
 	if err != nil {

cmd/web-handlers.go

@@ -467,6 +467,30 @@ func (web *webAPIHandlers) GetAuth(r *http.Request, args *WebGenericArgs, reply
 	return nil
 }
 
+// URLTokenReply contains the reply for CreateURLToken.
+type URLTokenReply struct {
+	Token     string `json:"token"`
+	UIVersion string `json:"uiVersion"`
+}
+
+// CreateURLToken creates a URL token (short-lived) for GET requests.
+func (web *webAPIHandlers) CreateURLToken(r *http.Request, args *WebGenericArgs, reply *URLTokenReply) error {
+	if !isHTTPRequestValid(r) {
+		return toJSONError(errAuthentication)
+	}
+
+	creds := serverConfig.GetCredential()
+
+	token, err := authenticateURL(creds.AccessKey, creds.SecretKey)
+	if err != nil {
+		return toJSONError(err)
+	}
+
+	reply.Token = token
+	reply.UIVersion = browser.UIVersion
+	return nil
+}
+
 // Upload - file upload handler.
 func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) {
 	objectAPI := web.ObjectAPI()

cmd/web-handlers_test.go

@@ -662,6 +662,45 @@ func testGetAuthWebHandler(obj ObjectLayer, instanceType string, t TestErrHandle
 	}
 }
 
+func TestWebCreateURLToken(t *testing.T) {
+	ExecObjectLayerTest(t, testCreateURLToken)
+}
+
+func testCreateURLToken(obj ObjectLayer, instanceType string, t TestErrHandler) {
+	apiRouter := initTestWebRPCEndPoint(obj)
+	credentials := serverConfig.GetCredential()
+
+	authorization, err := getWebRPCToken(apiRouter, credentials.AccessKey, credentials.SecretKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	args := WebGenericArgs{}
+	tokenReply := &URLTokenReply{}
+
+	req, err := newTestWebRPCRequest("Web.CreateURLToken", authorization, args)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rec := httptest.NewRecorder()
+	apiRouter.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code)
+	}
+
+	err = getTestWebRPCResponse(rec, &tokenReply)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Ensure the token is valid now. It will expire later.
+	if !isAuthTokenValid(tokenReply.Token) {
+		t.Fatalf("token is not valid")
+	}
+}
+
 // Wrapper for calling Upload Handler
 func TestWebHandlerUpload(t *testing.T) {
 	ExecObjectLayerTest(t, testUploadWebHandler)
@@ -815,6 +854,32 @@ func testDownloadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandl
 		t.Fatalf("The downloaded file is corrupted")
 	}
 
+	// Temporary token should succeed.
+	tmpToken, err := authenticateURL(credentials.AccessKey, credentials.SecretKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	code, bodyContent = test(tmpToken)
+
+	if code != http.StatusOK {
+		t.Fatalf("Expected the response status to be 200, but instead found `%d`", code)
+	}
+
+	if !bytes.Equal(bodyContent, content) {
+		t.Fatalf("The downloaded file is corrupted")
+	}
+
+	// Old token should fail.
+	code, bodyContent = test("eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1MDAzMzIwOTUsImlhdCI6MTUwMDMzMjAzNSwic3ViIjoiRFlLSU01VlRZNDBJMVZQSE5VMTkifQ.tXQ45GJc8eOFet_a4VWVyeqJEOPWybotQYNr2zVxBpEOICkGbu_YWGhd9TkLLe1E65oeeiLHPdXSN8CzcbPoRA")
+	if code != http.StatusForbidden {
+		t.Fatalf("Expected the response status to be 403, but instead found `%d`", code)
+	}
+
+	if !bytes.Equal(bodyContent, bytes.NewBufferString("Authentication failed, check your access credentials").Bytes()) {
+		t.Fatalf("Expected authentication error message, got %v", bodyContent)
+	}
+
 	// Unauthenticated download should fail.
 	code, _ = test("")
 	if code != http.StatusForbidden {
@@ -848,7 +913,7 @@ func testWebHandlerDownloadZip(obj ObjectLayer, instanceType string, t TestErrHa
 	apiRouter := initTestWebRPCEndPoint(obj)
 	credentials := serverConfig.GetCredential()
 
-	authorization, err := getWebRPCToken(apiRouter, credentials.AccessKey, credentials.SecretKey)
+	authorization, err := authenticateURL(credentials.AccessKey, credentials.SecretKey)
 	if err != nil {
 		t.Fatal("Cannot authenticate")
 	}

cmd/web-router.go

@@ -83,6 +83,9 @@ func registerWebRouter(mux *router.Router) error {
 	// RPC handler at URI - /minio/webrpc
 	webBrowserRouter.Methods("POST").Path("/webrpc").Handler(webRPC)
 	webBrowserRouter.Methods("PUT").Path("/upload/{bucket}/{object:.+}").HandlerFunc(web.Upload)
+
+	// These methods use short-expiry tokens in the URLs. These tokens may unintentionally
+	// be logged, so a new one must be generated for each request.
 	webBrowserRouter.Methods("GET").Path("/download/{bucket}/{object:.+}").Queries("token", "{token:.*}").HandlerFunc(web.Download)
 	webBrowserRouter.Methods("POST").Path("/zip").Queries("token", "{token:.*}").HandlerFunc(web.DownloadZip)