store only hash in the file, and compare sha256 hash of password

README.md

@@ -93,13 +93,17 @@ $ curl -v -X POST -d "@README.md" localhost:3535/api/v1/file-upload/testfile
 
 ## HTTP Basic Auth
 
-All API endpoints can be protected with HTTP Basic Auth. The secret needs to be set once, either via file or via API.
-If set via API, it will be persisted in a file specified in the config file.
+All API endpoints can be protected with HTTP Basic Auth.
+
+The API endpoints are initially unauthenticated, until a secret is configured
+either via file or via API. If the secret is configured via API, the SHA256
+hash is be stored in a file (specified in the config file) to enable basic auth protection
+across restarts.
 
 The config file ([systemapi-config.toml](./systemapi-config.toml)) includes a `basic_auth_secret_path`.
-- If this file is specified but doesn't exist, system-api will not start
-- If the file exists and is empty, then the APIs are unauthenticated until a secret is set
-- If the file exists and is not empty, then the APIs are authenticated with the secret in this file
+- If this file is specified but doesn't exist, system-api will not start and log an error.
+- If the file exists and is empty, then the APIs are unauthenticated until a secret is configured.
+- If the file exists and is not empty, then the APIs are authenticated for passwords that match the hash in this file.
 
 ```bash
 # Set `basic_auth_secret_path` in the config file and create it empty
@@ -110,7 +114,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

systemapi/middleware.go

@@ -1,20 +1,22 @@
 package systemapi
 
 import (
+	"crypto/sha256"
 	"crypto/subtle"
+	"encoding/hex"
 	"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
 			}
@@ -26,9 +28,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 := hex.EncodeToString(h.Sum(nil))
+
 			// 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
 			}

systemapi/server.go

@@ -4,6 +4,8 @@ package systemapi
 import (
 	"bufio"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -46,7 +48,7 @@ type Server struct {
 	events     []Event
 	eventsLock sync.RWMutex
 
-	basicAuthSecret string
+	basicAuthHash string
 }
 
 func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) {
@@ -70,11 +72,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 != "" {
@@ -102,7 +104,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)
@@ -312,13 +314,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) {
@@ -336,15 +338,20 @@ func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	// Create hash of the secret
+	h := sha256.New()
+	h.Write(secret)
+	secretHash := hex.EncodeToString(h.Sum(nil))
+
 	// 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)
 }

systemapi/server_test.go

@@ -2,6 +2,8 @@ package systemapi
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -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(basicAuthSecret)
+	basicAuthSecretHash := hex.EncodeToString(h.Sum(nil))
 
 	// Create the config
 	cfg := getTestConfig()
@@ -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)