Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password bypass vulnerability #12927

Closed
a1ien opened this issue Mar 27, 2019 · 6 comments
Closed

Password bypass vulnerability #12927

a1ien opened this issue Mar 27, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@a1ien
Copy link

a1ien commented Mar 27, 2019

We can authorize by any user. For that we can use jwt token with empty shared_secret.
It's happen because

case BearerAuthentication:
keyLookupFn := func(token *jwt.Token) (interface{}, error) {
// Check for expected signing method.
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
}
return []byte(h.Config.SharedSecret), nil
}
// Parse and validate the token.
token, err := jwt.Parse(creds.Token, keyLookupFn)
if err != nil {
h.httpError(w, err.Error(), http.StatusUnauthorized)
return
} else if !token.Valid {
h.httpError(w, "invalid token", http.StatusUnauthorized)
return
}
here we not check that h.Config.SharedSecret is not empty string.
In Authentication and authorization in InfluxDB document we not mention anything about shared-secret variable.

Also sems like we also not check exp state of token. And once generated token valid forewer.

@e-dard
Copy link
Contributor

e-dard commented Apr 1, 2019

Thanks for the report @a1ien. We are investigating this issue as a priority.

@dgnorton
Copy link
Contributor

dgnorton commented Apr 2, 2019

@a1ien again, thanks for reporting the issue. A PR to fix the shared secret issue has been made.

Also sems like we also not check exp state of token. And once generated token valid forever.

If the expiration claim is set on the token and it has expired, the parser will return an error here:

// Parse and validate the token.
token, err := jwt.Parse(creds.Token, keyLookupFn)
if err != nil {

It ensures an expiration was set here:

// Make sure an expiration was set on the token.
if exp, ok := claims["exp"].(float64); !ok || exp <= 0.0 {
h.httpError(w, "token expiration required", http.StatusUnauthorized)
return
}

And is tested here:

// Test handler with expired JWT token.
_, signedToken = MustJWTToken("user1", h.Config.SharedSecret, true)
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", signedToken))
w = httptest.NewRecorder()
h.ServeHTTP(w, req)
if w.Code != http.StatusUnauthorized {
t.Fatalf("unexpected status: %d: %s", w.Code, w.Body.String())
} else if !strings.Contains(w.Body.String(), `{"error":"Token is expired`) {
t.Fatalf("unexpected body: %s", w.Body.String())
}

@a1ien
Copy link
Author

a1ien commented Apr 2, 2019

i am not see any check that exp > time.now()

@dgnorton
Copy link
Contributor

dgnorton commented Apr 2, 2019

@a1ien the check isn't done by InfluxDB. It's done by the jwt library InfluxDB uses to parse tokens. The jwt.Parse function mentioned above will return an error if it sees that the token has expired.

@a1ien
Copy link
Author

a1ien commented Apr 2, 2019

@dgnorton Ah you are correct.

@e-dard
Copy link
Contributor

e-dard commented Apr 8, 2019

Fixed via #13088, #13132, #13133.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants