Skip to content

Commit

Permalink
store only hash in the file, and compare sha256 hash of password
Browse files Browse the repository at this point in the history
  • Loading branch information
metachris committed Nov 14, 2024
1 parent f5cedd5 commit c538c07
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 20 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ vi systemapi-config.toml
$ go run cmd/system-api/main.go --config systemapi-config.toml

# Initially, requests are unauthenticated
$ curl localhost:3535/api/v1/livez
$ curl localhost:3535/livez

# Set the basic auth secret
$ curl -d "foobar" localhost:3535/api/v1/set-basic-auth
Expand Down
16 changes: 11 additions & 5 deletions systemapi/middleware.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
package systemapi

import (
"crypto/sha256"
"crypto/subtle"
"fmt"
"net/http"
)

// BasicAuth implements a simple middleware handler for adding basic http auth to a route.
func BasicAuth(realm string, getCreds func() map[string]string) func(next http.Handler) http.Handler {
func BasicAuth(realm string, getHashedCredentials func() map[string]string) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Loading credentials dynamically because they can be updated at runtime
creds := getCreds()
hashedCredentials := getHashedCredentials()

// If no credentials are set, just pass through (unauthenticated)
if len(creds) == 0 {
if len(hashedCredentials) == 0 {
next.ServeHTTP(w, r)
return
}
Expand All @@ -26,9 +27,14 @@ func BasicAuth(realm string, getCreds func() map[string]string) func(next http.H
return
}

// Hash the password and see if credentials are allowed
h := sha256.New()
h.Write([]byte(pass))
userPassHash := fmt.Sprintf("%x", h.Sum(nil))

Check failure on line 33 in systemapi/middleware.go

View workflow job for this annotation

GitHub Actions / Lint

fmt.Sprintf can be replaced with faster hex.EncodeToString (perfsprint)

// Compare to allowed credentials
credPass, credUserOk := creds[user]
if !credUserOk || subtle.ConstantTimeCompare([]byte(pass), []byte(credPass)) != 1 {
credPassHash, credUserOk := hashedCredentials[user]
if !credUserOk || subtle.ConstantTimeCompare([]byte(userPassHash), []byte(credPassHash)) != 1 {
basicAuthFailed(w, realm)
return
}
Expand Down
28 changes: 17 additions & 11 deletions systemapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package systemapi
import (
"bufio"
"context"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -46,7 +47,7 @@ type Server struct {
events []Event
eventsLock sync.RWMutex

basicAuthSecret string
basicAuthHash string
}

func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) {
Expand All @@ -70,11 +71,11 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) {
}

if len(secret) == 0 {
cfg.Log.Info("Empty basic auth file loaded", "file", cfg.Config.General.BasicAuthSecretPath)
cfg.Log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", cfg.Config.General.BasicAuthSecretPath)
} else {
cfg.Log.Info("Basic auth enabled", "file", cfg.Config.General.BasicAuthSecretPath)
}
srv.basicAuthSecret = string(secret)
srv.basicAuthHash = string(secret)
}

if cfg.Config.General.PipeFile != "" {
Expand Down Expand Up @@ -102,7 +103,7 @@ func (s *Server) getRouter() http.Handler {

mux.Use(httplog.RequestLogger(s.log))
mux.Use(middleware.Recoverer)
mux.Use(BasicAuth("system-api", s.getBasicAuthCreds))
mux.Use(BasicAuth("system-api", s.getBasicAuthHashedCredentials))

mux.Get("/", s.handleLivenessCheck)
mux.Get("/livez", s.handleLivenessCheck)
Expand Down Expand Up @@ -312,13 +313,13 @@ func (s *Server) handleFileUpload(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}

func (s *Server) getBasicAuthCreds() map[string]string {
func (s *Server) getBasicAuthHashedCredentials() map[string]string {
// dynamic because can be set at runtime
resp := make(map[string]string)
if s.basicAuthSecret != "" {
resp["admin"] = s.basicAuthSecret
hashedCredentials := make(map[string]string)
if s.basicAuthHash != "" {
hashedCredentials["admin"] = s.basicAuthHash
}
return resp
return hashedCredentials
}

func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) {
Expand All @@ -336,15 +337,20 @@ func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request)
return
}

// Create hash of the secret
h := sha256.New()
h.Write([]byte(secret))

Check failure on line 342 in systemapi/server.go

View workflow job for this annotation

GitHub Actions / Lint

unnecessary conversion (unconvert)
secretHash := fmt.Sprintf("%x", h.Sum(nil))

Check failure on line 343 in systemapi/server.go

View workflow job for this annotation

GitHub Actions / Lint

fmt.Sprintf can be replaced with faster hex.EncodeToString (perfsprint)

// write secret to file
err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, secret, 0o600)
err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, []byte(secretHash), 0o600)
if err != nil {
s.log.Error("Failed to write secret to file", "err", err)
w.WriteHeader(http.StatusInternalServerError)
return
}

s.basicAuthSecret = string(secret)
s.basicAuthHash = secretHash
s.log.Info("Basic auth secret updated")
w.WriteHeader(http.StatusOK)
}
13 changes: 10 additions & 3 deletions systemapi/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package systemapi

import (
"bytes"
"crypto/sha256"
"fmt"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -79,8 +81,13 @@ func TestGeneralHandlers(t *testing.T) {
}

func TestBasicAuth(t *testing.T) {
basicAuthSecret := []byte("secret")
tempDir := t.TempDir()
basicAuthSecret := []byte("secret")

// Create a hash of the basic auth secret
h := sha256.New()
h.Write([]byte(basicAuthSecret))

Check failure on line 89 in systemapi/server_test.go

View workflow job for this annotation

GitHub Actions / Lint

unnecessary conversion (unconvert)
basicAuthSecretHash := fmt.Sprintf("%x", h.Sum(nil))

Check failure on line 90 in systemapi/server_test.go

View workflow job for this annotation

GitHub Actions / Lint

fmt.Sprintf can be replaced with faster hex.EncodeToString (perfsprint)

// Create the config
cfg := getTestConfig()
Expand Down Expand Up @@ -115,10 +122,10 @@ func TestBasicAuth(t *testing.T) {
code, _ = reqSetBasicAuthSecret("", "", bytes.NewReader(basicAuthSecret))
require.Equal(t, http.StatusOK, code)

// Ensure secretFromFile was written to file
// Ensure hash was written to file and is reproducible
secretFromFile, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath)
require.NoError(t, err)
require.Equal(t, basicAuthSecret, secretFromFile)
require.Equal(t, basicAuthSecretHash, string(secretFromFile))

// From here on, /livez shoud fail without basic auth
code, _ = reqGetLiveZ("", "", nil)
Expand Down

0 comments on commit c538c07

Please sign in to comment.