Skip to content

Commit

Permalink
Merge pull request #13132 from influxdata/dn-backport-fix-blank-secre…
Browse files Browse the repository at this point in the history
…t-1_7

Fix security vulnerability when shared secret is blank
  • Loading branch information
dgnorton committed Apr 4, 2019
2 parents 93b5632 + 5f75cc7 commit 2934bd4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
15 changes: 15 additions & 0 deletions services/httpd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ const (
MaxDebugRequestsInterval = 6 * time.Hour
)

var (
// ErrBearerAuthDisabled is returned when client specifies bearer auth in
// a request but bearer auth is disabled.
ErrBearerAuthDisabled = errors.New("bearer auth disabld")
)

// AuthenticationMethod defines the type of authentication used.
type AuthenticationMethod int

Expand Down Expand Up @@ -240,6 +246,10 @@ func (h *Handler) Open() {
}
h.accessLogFilters = StatusFilters(h.Config.AccessLogStatusFilters)

if h.Config.AuthEnabled && h.Config.SharedSecret == "" {
h.Logger.Info("Auth is enabled but shared-secret is blank. BearerAuthentication is disabled.")
}

if h.Config.FluxEnabled {
h.registered = true
prom.MustRegister(h.Controller.PrometheusCollectors()...)
Expand Down Expand Up @@ -1581,6 +1591,11 @@ func authenticate(inner func(http.ResponseWriter, *http.Request, meta.User), h *
return
}
case BearerAuthentication:
if h.Config.SharedSecret == "" {
atomic.AddInt64(&h.stats.AuthenticationFailures, 1)
h.httpError(w, ErrBearerAuthDisabled.Error(), http.StatusUnauthorized)
return
}
keyLookupFn := func(token *jwt.Token) (interface{}, error) {
// Check for expected signing method.
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
Expand Down
19 changes: 19 additions & 0 deletions services/httpd/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,25 @@ func TestHandler_Query_Auth(t *testing.T) {
t.Fatalf("unexpected body: %s", body)
}

// Test that auth fails if shared secret is blank.
origSecret := h.Config.SharedSecret
h.Config.SharedSecret = ""
token, _ = MustJWTToken("user1", h.Config.SharedSecret, false)
signedToken, err = token.SignedString([]byte(h.Config.SharedSecret))
if err != nil {
t.Fatal(err)
}
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 body := strings.TrimSpace(w.Body.String()); body != `{"error":"bearer auth disabled"}` {
} else if !strings.Contains(w.Body.String(), httpd.ErrBearerAuthDisabled.Error()) {
t.Fatalf("unexpected body: %s", w.Body.String())
}
h.Config.SharedSecret = origSecret

// Test the handler with valid user and password in the url and invalid in
// basic auth (prioritize url).
w = httptest.NewRecorder()
Expand Down

0 comments on commit 2934bd4

Please sign in to comment.