From 8c01c6cc0e476bc9da3b6cc5dce60392666b32ee Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Wed, 13 Nov 2024 13:01:07 +0100 Subject: [PATCH 1/7] start basic auth --- README.md | 33 +++++++++-- cmd/system-api/main.go | 1 + systemapi-config.toml | 3 + systemapi/config.go | 2 + systemapi/middleware.go | 41 ++++++++++++++ systemapi/server.go | 63 +++++++++++++++++++++ systemapi/server_test.go | 116 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 255 insertions(+), 4 deletions(-) create mode 100644 systemapi/middleware.go create mode 100644 systemapi/server_test.go diff --git a/README.md b/README.md index ac81663..30fc988 100644 --- a/README.md +++ b/README.md @@ -12,10 +12,7 @@ It currently does the following things: hashes, etc. - **Actions**: Ability to execute shell commands via API - **Configuration** through file uploads - -Future features: - -- Set a password for http-basic-auth (persisted, for all future requests) +- **HTTP Basic Auth** for API endpoints --- @@ -93,3 +90,31 @@ $ go run cmd/system-api/main.go --config systemapi-config.toml # Execute the example action $ 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. + +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 + +```bash +# Set `basic_auth_secret_path` in the config file and create it empty +touch .basic-auth-secret +vi systemapi-config.toml + +# Start the server, +$ go run cmd/system-api/main.go --config systemapi-config.toml + +# Initially, requests are unauthenticated +$ curl localhost:3535/api/v1/livez + +# Set the basic auth secret +$ curl -d "foobar" localhost:3535/api/v1/set-basic-auth + +# Now requests are authenticated +$ curl -u admin:foobar -v localhost:3535/livez +``` diff --git a/cmd/system-api/main.go b/cmd/system-api/main.go index d32242c..1d513f2 100644 --- a/cmd/system-api/main.go +++ b/cmd/system-api/main.go @@ -97,6 +97,7 @@ func runCli(cCtx *cli.Context) (err error) { } server, err := systemapi.NewServer(cfg) if err != nil { + log.Error("Error creating server", "err", err) return err } go server.Start() diff --git a/systemapi-config.toml b/systemapi-config.toml index 21c7fb9..7ba7d00 100644 --- a/systemapi-config.toml +++ b/systemapi-config.toml @@ -4,6 +4,9 @@ pipe_file = "pipe.fifo" log_json = false log_debug = true +# The path to the secret file used for basic auth +basic_auth_secret_path = "/tmp/basic_auth_secret" + [actions] # reboot = "reboot" # rbuilder_restart = "/etc/init.d/rbuilder restart" diff --git a/systemapi/config.go b/systemapi/config.go index c034135..1a1f351 100644 --- a/systemapi/config.go +++ b/systemapi/config.go @@ -11,6 +11,8 @@ type systemAPIConfigGeneral struct { PipeFile string `toml:"pipe_file"` LogJSON bool `toml:"log_json"` LogDebug bool `toml:"log_debug"` + + BasicAuthSecretPath string `toml:"basic_auth_secret_path"` } type SystemAPIConfig struct { diff --git a/systemapi/middleware.go b/systemapi/middleware.go new file mode 100644 index 0000000..d852ca3 --- /dev/null +++ b/systemapi/middleware.go @@ -0,0 +1,41 @@ +package systemapi + +import ( + "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 { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + creds := getCreds() + + if len(creds) == 0 { + // if no credentials are set, just pass through + next.ServeHTTP(w, r) + return + } + + user, pass, ok := r.BasicAuth() + if !ok { + basicAuthFailed(w, realm) + return + } + + credPass, credUserOk := creds[user] + if !credUserOk || subtle.ConstantTimeCompare([]byte(pass), []byte(credPass)) != 1 { + basicAuthFailed(w, realm) + return + } + + next.ServeHTTP(w, r) + }) + } +} + +func basicAuthFailed(w http.ResponseWriter, realm string) { + w.Header().Add("WWW-Authenticate", fmt.Sprintf(`Basic realm="%s"`, realm)) + w.WriteHeader(http.StatusUnauthorized) +} diff --git a/systemapi/server.go b/systemapi/server.go index b3c47d7..03389bd 100644 --- a/systemapi/server.go +++ b/systemapi/server.go @@ -45,6 +45,8 @@ type Server struct { events []Event eventsLock sync.RWMutex + + basicAuthSecret string } func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { @@ -55,6 +57,26 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { events: make([]Event, 0), } + if cfg.Config.General.BasicAuthSecretPath != "" { + // Abort if the file does not exist + if _, err := os.Stat(cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { + return nil, fmt.Errorf("basic auth secret file does not exist: %s", cfg.Config.General.BasicAuthSecretPath) + } + + // Read the secret from the file + secret, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) + if err != nil { + return nil, fmt.Errorf("failed to read basic auth secret file: %w", err) + } + + if len(secret) == 0 { + cfg.Log.Info("Empty basic auth file loaded", "file", cfg.Config.General.BasicAuthSecretPath) + } else { + cfg.Log.Info("Basic auth enabled", "file", cfg.Config.General.BasicAuthSecretPath) + } + srv.basicAuthSecret = string(secret) + } + if cfg.Config.General.PipeFile != "" { os.Remove(cfg.Config.General.PipeFile) err := syscall.Mknod(cfg.Config.General.PipeFile, syscall.S_IFIFO|0o666, 0) @@ -80,12 +102,16 @@ func (s *Server) getRouter() http.Handler { mux.Use(httplog.RequestLogger(s.log)) mux.Use(middleware.Recoverer) + mux.Use(BasicAuth("system-api", s.getBasicAuthCreds)) mux.Get("/", s.handleLivenessCheck) mux.Get("/livez", s.handleLivenessCheck) mux.Get("/api/v1/new_event", s.handleNewEvent) mux.Get("/api/v1/events", s.handleGetEvents) mux.Get("/logs", s.handleGetLogs) + + mux.Post("/api/v1/set-basic-auth", s.handleSetBasicAuthCreds) + mux.Get("/api/v1/actions/{action}", s.handleAction) mux.Post("/api/v1/file-upload/{file}", s.handleFileUpload) @@ -285,3 +311,40 @@ func (s *Server) handleFileUpload(w http.ResponseWriter, r *http.Request) { s.addInternalEvent(fmt.Sprintf("file upload success: %s = %s - content: %d bytes", fileArg, filename, len(content))) w.WriteHeader(http.StatusOK) } + +func (s *Server) getBasicAuthCreds() map[string]string { + // dynamic because can be set at runtime + resp := make(map[string]string) + if s.basicAuthSecret != "" { + resp["admin"] = s.basicAuthSecret + } + return resp +} + +func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) { + if s.cfg.Config.General.BasicAuthSecretPath == "" { + s.log.Warn("Basic auth secret path not set") + w.WriteHeader(http.StatusNotImplemented) + return + } + + // read secret from payload + secret, err := io.ReadAll(r.Body) + if err != nil { + s.log.Error("Failed to read secret from payload", "err", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + + // write secret to file + err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, secret, 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.log.Info("Basic auth secret updated") + w.WriteHeader(http.StatusOK) +} diff --git a/systemapi/server_test.go b/systemapi/server_test.go new file mode 100644 index 0000000..25dd6b6 --- /dev/null +++ b/systemapi/server_test.go @@ -0,0 +1,116 @@ +package systemapi + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/flashbots/system-api/common" + "github.com/go-chi/httplog/v2" + "github.com/stretchr/testify/require" +) + +func getTestLogger() *httplog.Logger { + return common.SetupLogger(&common.LoggingOpts{ + Debug: true, + JSON: false, + }) +} + +func getTestConfig() *HTTPServerConfig { + return &HTTPServerConfig{ + Log: getTestLogger(), + Config: NewSystemAPIConfig(), + } +} + +func TestGeneralHandlers(t *testing.T) { + // Create the config + cfg := getTestConfig() + + // Instantiate the server + srv, err := NewServer(cfg) + require.NoError(t, err) + router := srv.getRouter() + + // Test /livez + req, err := http.NewRequest(http.MethodGet, "/livez", nil) + require.NoError(t, err) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + + // Test /api/v1/events + req, err = http.NewRequest(http.MethodGet, "/api/v1/events", nil) + require.NoError(t, err) + rr = httptest.NewRecorder() + router.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + body, err := io.ReadAll(rr.Body) + require.NoError(t, err) + require.Equal(t, "[]\n", string(body)) + + // Add an event + req, err = http.NewRequest(http.MethodGet, "/api/v1/new_event?message=foo", nil) + require.NoError(t, err) + rr = httptest.NewRecorder() + router.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + require.Len(t, srv.events, 1) +} + +func TestBasicAuth(t *testing.T) { + basicAuthSecret := []byte("secret") + tempDir := t.TempDir() + + // Create the config + cfg := getTestConfig() + cfg.Config.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" + + // Create the temporary file to store the basic auth secret + err := os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) + require.NoError(t, err) + + // Instantiate the server + srv, err := NewServer(cfg) + require.NoError(t, err) + router := srv.getRouter() + + getLiveZ := func(basicAuthUser, basicAuthPass string) int { + req, err := http.NewRequest(http.MethodGet, "/livez", nil) + if basicAuthUser != "" { + req.SetBasicAuth(basicAuthUser, basicAuthPass) + } + require.NoError(t, err) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + return rr.Code + } + + // Initially, /livez should work without basic auth + require.Equal(t, http.StatusOK, getLiveZ("", "")) + + // Set a basic auth secret + req, err := http.NewRequest(http.MethodPost, "/api/v1/set-basic-auth", bytes.NewReader(basicAuthSecret)) + require.NoError(t, err) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + + // Verify secretFromFile was written to file + secretFromFile, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) + require.NoError(t, err) + require.Equal(t, basicAuthSecret, secretFromFile) + + // From here on, /livez shoud fail without basic auth + require.Equal(t, http.StatusUnauthorized, getLiveZ("", "")) + + // /livez should work with basic auth + require.Equal(t, http.StatusOK, getLiveZ("admin", string(basicAuthSecret))) + + // /livez should now work with invalid basic auth credentials + require.Equal(t, http.StatusUnauthorized, getLiveZ("admin1", string(basicAuthSecret))) +} From 65d71da5bb8ba8e0c411af282d532d73e858f8d7 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Wed, 13 Nov 2024 14:20:23 +0100 Subject: [PATCH 2/7] simplify tests --- systemapi/middleware.go | 5 ++++- systemapi/server_test.go | 37 ++++++++++++++++--------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/systemapi/middleware.go b/systemapi/middleware.go index d852ca3..10d535e 100644 --- a/systemapi/middleware.go +++ b/systemapi/middleware.go @@ -10,20 +10,23 @@ import ( func BasicAuth(realm string, getCreds 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() + // If no credentials are set, just pass through (unauthenticated) if len(creds) == 0 { - // if no credentials are set, just pass through next.ServeHTTP(w, r) return } + // Load credentials from request user, pass, ok := r.BasicAuth() if !ok { basicAuthFailed(w, realm) return } + // Compare to allowed credentials credPass, credUserOk := creds[user] if !credUserOk || subtle.ConstantTimeCompare([]byte(pass), []byte(credPass)) != 1 { basicAuthFailed(w, realm) diff --git a/systemapi/server_test.go b/systemapi/server_test.go index 25dd6b6..e0cbf51 100644 --- a/systemapi/server_test.go +++ b/systemapi/server_test.go @@ -27,37 +27,34 @@ func getTestConfig() *HTTPServerConfig { } } -func TestGeneralHandlers(t *testing.T) { - // Create the config - cfg := getTestConfig() +func execRequest(t *testing.T, router http.Handler, method, url string, body io.Reader) *httptest.ResponseRecorder { + t.Helper() + req, err := http.NewRequest(method, url, body) + require.NoError(t, err) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + return rr +} +func TestGeneralHandlers(t *testing.T) { // Instantiate the server - srv, err := NewServer(cfg) + srv, err := NewServer(getTestConfig()) require.NoError(t, err) router := srv.getRouter() // Test /livez - req, err := http.NewRequest(http.MethodGet, "/livez", nil) - require.NoError(t, err) - rr := httptest.NewRecorder() - router.ServeHTTP(rr, req) + rr := execRequest(t, router, http.MethodGet, "/livez", nil) require.Equal(t, http.StatusOK, rr.Code) // Test /api/v1/events - req, err = http.NewRequest(http.MethodGet, "/api/v1/events", nil) - require.NoError(t, err) - rr = httptest.NewRecorder() - router.ServeHTTP(rr, req) + rr = execRequest(t, router, http.MethodGet, "/api/v1/events", nil) require.Equal(t, http.StatusOK, rr.Code) body, err := io.ReadAll(rr.Body) require.NoError(t, err) require.Equal(t, "[]\n", string(body)) // Add an event - req, err = http.NewRequest(http.MethodGet, "/api/v1/new_event?message=foo", nil) - require.NoError(t, err) - rr = httptest.NewRecorder() - router.ServeHTTP(rr, req) + rr = execRequest(t, router, http.MethodGet, "/api/v1/new_event?message=foo", nil) require.Equal(t, http.StatusOK, rr.Code) require.Len(t, srv.events, 1) } @@ -79,6 +76,7 @@ func TestBasicAuth(t *testing.T) { require.NoError(t, err) router := srv.getRouter() + // Helper to get /livez with and without basic auth getLiveZ := func(basicAuthUser, basicAuthPass string) int { req, err := http.NewRequest(http.MethodGet, "/livez", nil) if basicAuthUser != "" { @@ -94,13 +92,10 @@ func TestBasicAuth(t *testing.T) { require.Equal(t, http.StatusOK, getLiveZ("", "")) // Set a basic auth secret - req, err := http.NewRequest(http.MethodPost, "/api/v1/set-basic-auth", bytes.NewReader(basicAuthSecret)) - require.NoError(t, err) - rr := httptest.NewRecorder() - router.ServeHTTP(rr, req) + rr := execRequest(t, router, http.MethodPost, "/api/v1/set-basic-auth", bytes.NewReader(basicAuthSecret)) require.Equal(t, http.StatusOK, rr.Code) - // Verify secretFromFile was written to file + // Ensure secretFromFile was written to file secretFromFile, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) require.NoError(t, err) require.Equal(t, basicAuthSecret, secretFromFile) From f5cedd57190219bb0708fdf62a84fea669b102e0 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 16:00:13 +0100 Subject: [PATCH 3/7] test updates --- systemapi/server_test.go | 91 +++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 33 deletions(-) diff --git a/systemapi/server_test.go b/systemapi/server_test.go index e0cbf51..0527d0f 100644 --- a/systemapi/server_test.go +++ b/systemapi/server_test.go @@ -27,13 +27,34 @@ func getTestConfig() *HTTPServerConfig { } } -func execRequest(t *testing.T, router http.Handler, method, url string, body io.Reader) *httptest.ResponseRecorder { +// Helper to execute an API request with optional basic auth +func execRequestAuth(t *testing.T, router http.Handler, method, url string, requestBody io.Reader, basicAuthUser, basicAuthPass string) (statusCode int, responsePayload []byte) { t.Helper() - req, err := http.NewRequest(method, url, body) + req, err := http.NewRequest(method, url, requestBody) require.NoError(t, err) + if basicAuthUser != "" { + req.SetBasicAuth(basicAuthUser, basicAuthPass) + } rr := httptest.NewRecorder() router.ServeHTTP(rr, req) - return rr + + responseBody, err := io.ReadAll(rr.Body) + require.NoError(t, err) + return rr.Code, responseBody +} + +// Helper to execute an API request without basic auth +func execRequest(t *testing.T, router http.Handler, method, url string, requestBody io.Reader) (statusCode int, responsePayload []byte) { + t.Helper() + return execRequestAuth(t, router, method, url, requestBody, "", "") +} + +// Helper to create prepared executors for specific API endpoints +func makeRequestExecutor(t *testing.T, router http.Handler, method, url string) func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { + t.Helper() + return func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { + return execRequestAuth(t, router, method, url, requestBody, basicAuthUser, basicAuthPass) + } } func TestGeneralHandlers(t *testing.T) { @@ -43,19 +64,17 @@ func TestGeneralHandlers(t *testing.T) { router := srv.getRouter() // Test /livez - rr := execRequest(t, router, http.MethodGet, "/livez", nil) - require.Equal(t, http.StatusOK, rr.Code) + code, _ := execRequest(t, router, http.MethodGet, "/livez", nil) + require.Equal(t, http.StatusOK, code) // Test /api/v1/events - rr = execRequest(t, router, http.MethodGet, "/api/v1/events", nil) - require.Equal(t, http.StatusOK, rr.Code) - body, err := io.ReadAll(rr.Body) - require.NoError(t, err) - require.Equal(t, "[]\n", string(body)) + code, respBody := execRequest(t, router, http.MethodGet, "/api/v1/events", nil) + require.Equal(t, http.StatusOK, code) + require.Equal(t, "[]\n", string(respBody)) // Add an event - rr = execRequest(t, router, http.MethodGet, "/api/v1/new_event?message=foo", nil) - require.Equal(t, http.StatusOK, rr.Code) + code, _ = execRequest(t, router, http.MethodGet, "/api/v1/new_event?message=foo", nil) + require.Equal(t, http.StatusOK, code) require.Len(t, srv.events, 1) } @@ -67,33 +86,34 @@ func TestBasicAuth(t *testing.T) { cfg := getTestConfig() cfg.Config.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" + // Server should fail to start if the basic auth secret file does not exist + _, err := NewServer(cfg) + require.Error(t, err) + // Create the temporary file to store the basic auth secret - err := os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) + err = os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) require.NoError(t, err) - // Instantiate the server + // Server will work now srv, err := NewServer(cfg) require.NoError(t, err) router := srv.getRouter() - // Helper to get /livez with and without basic auth - getLiveZ := func(basicAuthUser, basicAuthPass string) int { - req, err := http.NewRequest(http.MethodGet, "/livez", nil) - if basicAuthUser != "" { - req.SetBasicAuth(basicAuthUser, basicAuthPass) - } - require.NoError(t, err) - rr := httptest.NewRecorder() - router.ServeHTTP(rr, req) - return rr.Code - } + // Prepare request helpers + reqGetLiveZ := makeRequestExecutor(t, router, http.MethodGet, "/livez") + reqSetBasicAuthSecret := makeRequestExecutor(t, router, http.MethodPost, "/api/v1/set-basic-auth") // Initially, /livez should work without basic auth - require.Equal(t, http.StatusOK, getLiveZ("", "")) + code, _ := reqGetLiveZ("", "", nil) + require.Equal(t, http.StatusOK, code) + + // should work even if invalid basic auth credentials are provided + code, _ = reqGetLiveZ("admin", "foo", nil) + require.Equal(t, http.StatusOK, code) // Set a basic auth secret - rr := execRequest(t, router, http.MethodPost, "/api/v1/set-basic-auth", bytes.NewReader(basicAuthSecret)) - require.Equal(t, http.StatusOK, rr.Code) + code, _ = reqSetBasicAuthSecret("", "", bytes.NewReader(basicAuthSecret)) + require.Equal(t, http.StatusOK, code) // Ensure secretFromFile was written to file secretFromFile, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) @@ -101,11 +121,16 @@ func TestBasicAuth(t *testing.T) { require.Equal(t, basicAuthSecret, secretFromFile) // From here on, /livez shoud fail without basic auth - require.Equal(t, http.StatusUnauthorized, getLiveZ("", "")) + code, _ = reqGetLiveZ("", "", nil) + require.Equal(t, http.StatusUnauthorized, code) // /livez should work with basic auth - require.Equal(t, http.StatusOK, getLiveZ("admin", string(basicAuthSecret))) - - // /livez should now work with invalid basic auth credentials - require.Equal(t, http.StatusUnauthorized, getLiveZ("admin1", string(basicAuthSecret))) + code, _ = reqGetLiveZ("admin", string(basicAuthSecret), nil) + require.Equal(t, http.StatusOK, code) + + // /livez should not work with invalid basic auth credentials + code, _ = reqGetLiveZ("admin1", string(basicAuthSecret), nil) + require.Equal(t, http.StatusUnauthorized, code) + code, _ = reqGetLiveZ("admin", "foo", nil) + require.Equal(t, http.StatusUnauthorized, code) } From f1216c971c634530c48b346bd2fd5541a8c3444a Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 17:20:23 +0100 Subject: [PATCH 4/7] store only hash in the file, and compare sha256 hash of password --- README.md | 16 ++++++++++------ systemapi/middleware.go | 17 ++++++++++++----- systemapi/server.go | 29 ++++++++++++++++++----------- systemapi/server_test.go | 13 ++++++++++--- 4 files changed, 50 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 30fc988..3dfb73c 100644 --- a/README.md +++ b/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 diff --git a/systemapi/middleware.go b/systemapi/middleware.go index 10d535e..0fc7310 100644 --- a/systemapi/middleware.go +++ b/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 } diff --git a/systemapi/server.go b/systemapi/server.go index 03389bd..560443f 100644 --- a/systemapi/server.go +++ b/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) } diff --git a/systemapi/server_test.go b/systemapi/server_test.go index 0527d0f..287afbe 100644 --- a/systemapi/server_test.go +++ b/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) From 262a86bdbd466f2e9b40d83c95ab0be7117488ec Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 17:31:36 +0100 Subject: [PATCH 5/7] cleanup --- .gitignore | 3 +- README.md | 33 +++++++++++------ systemapi-config.toml | 5 ++- systemapi/config.go | 9 +++-- systemapi/server.go | 79 ++++++++++++++++++++++++++++------------ systemapi/server_test.go | 29 +++++++++++---- 6 files changed, 108 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index d19c6ea..508c1a8 100644 --- a/.gitignore +++ b/.gitignore @@ -21,4 +21,5 @@ /build /cert.pem /key.pem -/pipe.fifo \ No newline at end of file +/pipe.fifo +/basic-auth-secret.txt \ No newline at end of file diff --git a/README.md b/README.md index 3dfb73c..3d64de1 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ It currently does the following things: - **Actions**: Ability to execute shell commands via API - **Configuration** through file uploads - **HTTP Basic Auth** for API endpoints +- All actions show up in the event log --- @@ -101,24 +102,32 @@ hash is be stored in a file (specified in the config file) to enable basic auth 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 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. +- If the file exists and is empty, then the APIs are unauthenticated until a secret is configured. +- If this file is specified but doesn't exist, system-api will create it (empty). ```bash -# Set `basic_auth_secret_path` in the config file and create it empty -touch .basic-auth-secret -vi systemapi-config.toml +# The included systemapi-config.toml uses basic-auth-secret.txt for basic_auth_secret_path +cat systemapi-config.toml -# Start the server, -$ go run cmd/system-api/main.go --config systemapi-config.toml +# Start the server +go run cmd/system-api/main.go --config systemapi-config.toml # Initially, requests are unauthenticated -$ curl localhost:3535/livez +curl -v localhost:3535/livez + +# Set the basic auth secret. From here on, authentication is required for all API requests. +curl -d "foobar" localhost:3535/api/v1/set-basic-auth + +# Check that hash was written to the file +cat basic-auth-secret.txt + +# API calls with no basic auth credentials are provided fail now, with '401 Unauthorized' because +curl -v localhost:3535/livez -# Set the basic auth secret -$ curl -d "foobar" localhost:3535/api/v1/set-basic-auth +# API calls work if correct basic auth credentials are provided +curl -v -u admin:foobar localhost:3535/livez -# Now requests are authenticated -$ curl -u admin:foobar -v localhost:3535/livez +# The update also shows up in the logs +curl -u admin:foobar localhost:3535/logs ``` diff --git a/systemapi-config.toml b/systemapi-config.toml index 7ba7d00..612dcaa 100644 --- a/systemapi-config.toml +++ b/systemapi-config.toml @@ -1,11 +1,12 @@ [general] listen_addr = "0.0.0.0:3535" pipe_file = "pipe.fifo" +pprof = true log_json = false log_debug = true -# The path to the secret file used for basic auth -basic_auth_secret_path = "/tmp/basic_auth_secret" +# Enable HTTP Basic auth by setting a file for the hashed secret +basic_auth_secret_path = "basic-auth-secret.txt" [actions] # reboot = "reboot" diff --git a/systemapi/config.go b/systemapi/config.go index 1a1f351..ff02b41 100644 --- a/systemapi/config.go +++ b/systemapi/config.go @@ -7,10 +7,11 @@ import ( ) type systemAPIConfigGeneral struct { - ListenAddr string `toml:"listen_addr"` - PipeFile string `toml:"pipe_file"` - LogJSON bool `toml:"log_json"` - LogDebug bool `toml:"log_debug"` + ListenAddr string `toml:"listen_addr"` + PipeFile string `toml:"pipe_file"` + LogJSON bool `toml:"log_json"` + LogDebug bool `toml:"log_debug"` + EnablePprof bool `toml:"pprof"` // Enables pprof endpoints BasicAuthSecretPath string `toml:"basic_auth_secret_path"` } diff --git a/systemapi/server.go b/systemapi/server.go index 560443f..efe3a79 100644 --- a/systemapi/server.go +++ b/systemapi/server.go @@ -24,9 +24,8 @@ import ( ) type HTTPServerConfig struct { - Config *SystemAPIConfig - Log *httplog.Logger - EnablePprof bool + Config *SystemAPIConfig + Log *httplog.Logger DrainDuration time.Duration GracefulShutdownDuration time.Duration @@ -59,26 +58,13 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { events: make([]Event, 0), } - if cfg.Config.General.BasicAuthSecretPath != "" { - // Abort if the file does not exist - if _, err := os.Stat(cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { - return nil, fmt.Errorf("basic auth secret file does not exist: %s", cfg.Config.General.BasicAuthSecretPath) - } - - // Read the secret from the file - secret, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) - if err != nil { - return nil, fmt.Errorf("failed to read basic auth secret file: %w", err) - } - - if len(secret) == 0 { - 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.basicAuthHash = string(secret) + // Load (or create) file with basic auth secret hash + err = srv.loadBasicAuthSecretFromFile() + if err != nil { + return nil, err } + // Setup the pipe file if cfg.Config.General.PipeFile != "" { os.Remove(cfg.Config.General.PipeFile) err := syscall.Mknod(cfg.Config.General.PipeFile, syscall.S_IFIFO|0o666, 0) @@ -89,6 +75,7 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { go srv.readPipeInBackground() } + // Create the HTTP server srv.srv = &http.Server{ Addr: cfg.Config.General.ListenAddr, Handler: srv.getRouter(), @@ -99,27 +86,68 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { return srv, nil } +func (s *Server) loadBasicAuthSecretFromFile() error { + if s.cfg.Config.General.BasicAuthSecretPath == "" { + return nil + } + + // Create if the file does not exist + if _, err := os.Stat(s.cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { + err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) + if err != nil { + return fmt.Errorf("failed to create basic auth secret file: %w", err) + } + s.cfg.Log.Info("Basic auth file created, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + s.basicAuthHash = "" + return nil + } + + // Read the secret from the file + secret, err := os.ReadFile(s.cfg.Config.General.BasicAuthSecretPath) + if err != nil { + return fmt.Errorf("failed to read basic auth secret file: %w", err) + } + + if len(secret) == 0 { + s.cfg.Log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + } else { + s.cfg.Log.Info("Basic auth enabled", "file", s.cfg.Config.General.BasicAuthSecretPath) + } + s.basicAuthHash = string(secret) + return nil +} + func (s *Server) getRouter() http.Handler { mux := chi.NewRouter() mux.Use(httplog.RequestLogger(s.log)) mux.Use(middleware.Recoverer) + + // Enable a custom HTTP Basic Auth middleware mux.Use(BasicAuth("system-api", s.getBasicAuthHashedCredentials)) + // Common APIs mux.Get("/", s.handleLivenessCheck) mux.Get("/livez", s.handleLivenessCheck) + + // Event (log) APIs mux.Get("/api/v1/new_event", s.handleNewEvent) mux.Get("/api/v1/events", s.handleGetEvents) mux.Get("/logs", s.handleGetLogs) + // API to set the basic auth secret mux.Post("/api/v1/set-basic-auth", s.handleSetBasicAuthCreds) + // API to trigger an action mux.Get("/api/v1/actions/{action}", s.handleAction) + + // API to upload a file mux.Post("/api/v1/file-upload/{file}", s.handleFileUpload) - if s.cfg.EnablePprof { - s.log.Info("pprof API enabled") + // Optionally, pprof + if s.cfg.Config.General.EnablePprof { mux.Mount("/debug", middleware.Profiler()) + s.log.Info("pprof API enabled: /debug/pprof/") } return mux @@ -213,6 +241,7 @@ func (s *Server) writeEventsAsText(w http.ResponseWriter) { return } } + w.WriteHeader(http.StatusOK) } func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { @@ -223,7 +252,7 @@ func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { return } - // write events as JSON response + // Send events as JSON response s.eventsLock.RLock() defer s.eventsLock.RUnlock() w.Header().Set("Content-Type", "application/json") @@ -233,6 +262,7 @@ func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) return } + w.WriteHeader(http.StatusOK) } func (s *Server) handleGetLogs(w http.ResponseWriter, r *http.Request) { @@ -353,5 +383,6 @@ func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) s.basicAuthHash = secretHash s.log.Info("Basic auth secret updated") + s.addInternalEvent("basic auth secret updated. new hash: " + secretHash) w.WriteHeader(http.StatusOK) } diff --git a/systemapi/server_test.go b/systemapi/server_test.go index 287afbe..1dd37dd 100644 --- a/systemapi/server_test.go +++ b/systemapi/server_test.go @@ -51,8 +51,8 @@ func execRequest(t *testing.T, router http.Handler, method, url string, requestB return execRequestAuth(t, router, method, url, requestBody, "", "") } -// Helper to create prepared executors for specific API endpoints -func makeRequestExecutor(t *testing.T, router http.Handler, method, url string) func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { +// Helper to create prepared test runners for specific API endpoints +func createRequestRunner(t *testing.T, router http.Handler, method, url string) func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { t.Helper() return func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { return execRequestAuth(t, router, method, url, requestBody, basicAuthUser, basicAuthPass) @@ -69,7 +69,7 @@ func TestGeneralHandlers(t *testing.T) { code, _ := execRequest(t, router, http.MethodGet, "/livez", nil) require.Equal(t, http.StatusOK, code) - // Test /api/v1/events + // /api/v1/events is initially empty code, respBody := execRequest(t, router, http.MethodGet, "/api/v1/events", nil) require.Equal(t, http.StatusOK, code) require.Equal(t, "[]\n", string(respBody)) @@ -78,6 +78,17 @@ func TestGeneralHandlers(t *testing.T) { code, _ = execRequest(t, router, http.MethodGet, "/api/v1/new_event?message=foo", nil) require.Equal(t, http.StatusOK, code) require.Len(t, srv.events, 1) + require.Equal(t, "foo", srv.events[0].Message) + + // /api/v1/events now has an entry + code, respBody = execRequest(t, router, http.MethodGet, "/api/v1/events", nil) + require.Equal(t, http.StatusOK, code) + require.Contains(t, string(respBody), "foo") + + // /logs should also work + code, respBody = execRequest(t, router, http.MethodGet, "/logs", nil) + require.Equal(t, http.StatusOK, code) + require.Contains(t, string(respBody), "foo\n") } func TestBasicAuth(t *testing.T) { @@ -93,9 +104,13 @@ func TestBasicAuth(t *testing.T) { cfg := getTestConfig() cfg.Config.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" - // Server should fail to start if the basic auth secret file does not exist + // Create the server instance _, err := NewServer(cfg) - require.Error(t, err) + require.NoError(t, err) + + // Ensure the basic auth secret file was created + _, err = os.Stat(cfg.Config.General.BasicAuthSecretPath) + require.NoError(t, err) // Create the temporary file to store the basic auth secret err = os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) @@ -107,8 +122,8 @@ func TestBasicAuth(t *testing.T) { router := srv.getRouter() // Prepare request helpers - reqGetLiveZ := makeRequestExecutor(t, router, http.MethodGet, "/livez") - reqSetBasicAuthSecret := makeRequestExecutor(t, router, http.MethodPost, "/api/v1/set-basic-auth") + reqGetLiveZ := createRequestRunner(t, router, http.MethodGet, "/livez") + reqSetBasicAuthSecret := createRequestRunner(t, router, http.MethodPost, "/api/v1/set-basic-auth") // Initially, /livez should work without basic auth code, _ := reqGetLiveZ("", "", nil) From 6b1e6ca0724f922cd8c30c4d68ee1eb83fc74fdc Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 18:36:15 +0100 Subject: [PATCH 6/7] salt --- README.md | 2 +- cmd/system-api/main.go | 6 +-- systemapi-config.toml | 11 ++++-- systemapi/config.go | 4 ++ systemapi/middleware.go | 3 +- systemapi/server.go | 82 ++++++++++++++++++---------------------- systemapi/server_test.go | 34 ++++++++--------- 7 files changed, 67 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index 3d64de1..fe1320b 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ $ curl -v -X POST -d "@README.md" localhost:3535/api/v1/file-upload/testfile 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 +either via file or via API. If the secret is configured via API, the salted SHA256 hash is be stored in a file (specified in the config file) to enable basic auth protection across restarts. diff --git a/cmd/system-api/main.go b/cmd/system-api/main.go index 1d513f2..e849644 100644 --- a/cmd/system-api/main.go +++ b/cmd/system-api/main.go @@ -91,11 +91,7 @@ func runCli(cCtx *cli.Context) (err error) { ) // Setup and start the server (in the background) - cfg := &systemapi.HTTPServerConfig{ - Log: log, - Config: config, - } - server, err := systemapi.NewServer(cfg) + server, err := systemapi.NewServer(log, config) if err != nil { log.Error("Error creating server", "err", err) return err diff --git a/systemapi-config.toml b/systemapi-config.toml index 612dcaa..1b8b291 100644 --- a/systemapi-config.toml +++ b/systemapi-config.toml @@ -5,14 +5,19 @@ pprof = true log_json = false log_debug = true -# Enable HTTP Basic auth by setting a file for the hashed secret -basic_auth_secret_path = "basic-auth-secret.txt" +# HTTP Basic Auth +basic_auth_secret_path = "basic-auth-secret.txt" # basic auth is supported if a path is provided +basic_auth_secret_salt = "D;%yL9TS:5PalS/d" # use a random string for the salt + +# HTTP server timeouts +# http_read_timeout_ms = 2500 +# http_write_timeout_ms = 2500 [actions] +echo_test = "echo test" # reboot = "reboot" # rbuilder_restart = "/etc/init.d/rbuilder restart" # rbuilder_stop = "/etc/init.d/rbuilder stop" -echo_test = "echo test" [file_uploads] testfile = "/tmp/testfile.txt" diff --git a/systemapi/config.go b/systemapi/config.go index ff02b41..cd00cfa 100644 --- a/systemapi/config.go +++ b/systemapi/config.go @@ -14,6 +14,10 @@ type systemAPIConfigGeneral struct { EnablePprof bool `toml:"pprof"` // Enables pprof endpoints BasicAuthSecretPath string `toml:"basic_auth_secret_path"` + BasicAuthSecretSalt string `toml:"basic_auth_secret_salt"` + + HTTPReadTimeoutMillis int `toml:"http_read_timeout_ms"` + HTTPWriteTimeoutMillis int `toml:"http_write_timeout_ms"` } type SystemAPIConfig struct { diff --git a/systemapi/middleware.go b/systemapi/middleware.go index 0fc7310..c7ee1d7 100644 --- a/systemapi/middleware.go +++ b/systemapi/middleware.go @@ -9,7 +9,7 @@ import ( ) // BasicAuth implements a simple middleware handler for adding basic http auth to a route. -func BasicAuth(realm string, getHashedCredentials func() map[string]string) func(next http.Handler) http.Handler { +func BasicAuth(realm, salt 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 @@ -31,6 +31,7 @@ func BasicAuth(realm string, getHashedCredentials func() map[string]string) func // Hash the password and see if credentials are allowed h := sha256.New() h.Write([]byte(pass)) + h.Write([]byte(salt)) userPassHash := hex.EncodeToString(h.Sum(nil)) // Compare to allowed credentials diff --git a/systemapi/server.go b/systemapi/server.go index efe3a79..9002a12 100644 --- a/systemapi/server.go +++ b/systemapi/server.go @@ -23,25 +23,14 @@ import ( "github.com/go-chi/httplog/v2" ) -type HTTPServerConfig struct { - Config *SystemAPIConfig - Log *httplog.Logger - - DrainDuration time.Duration - GracefulShutdownDuration time.Duration - ReadTimeout time.Duration - WriteTimeout time.Duration -} - type Event struct { ReceivedAt time.Time `json:"received_at"` Message string `json:"message"` } type Server struct { - cfg *HTTPServerConfig + cfg *SystemAPIConfig log *httplog.Logger - srv *http.Server events []Event @@ -50,70 +39,70 @@ type Server struct { basicAuthHash string } -func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { - srv = &Server{ +func NewServer(log *httplog.Logger, cfg *SystemAPIConfig) (server *Server, err error) { + server = &Server{ cfg: cfg, - log: cfg.Log, + log: log, srv: nil, events: make([]Event, 0), } // Load (or create) file with basic auth secret hash - err = srv.loadBasicAuthSecretFromFile() + err = server.loadBasicAuthSecretFromFile() if err != nil { return nil, err } // Setup the pipe file - if cfg.Config.General.PipeFile != "" { - os.Remove(cfg.Config.General.PipeFile) - err := syscall.Mknod(cfg.Config.General.PipeFile, syscall.S_IFIFO|0o666, 0) + if cfg.General.PipeFile != "" { + os.Remove(cfg.General.PipeFile) + err := syscall.Mknod(cfg.General.PipeFile, syscall.S_IFIFO|0o666, 0) if err != nil { return nil, err } - go srv.readPipeInBackground() + go server.readPipeInBackground() } // Create the HTTP server - srv.srv = &http.Server{ - Addr: cfg.Config.General.ListenAddr, - Handler: srv.getRouter(), - ReadTimeout: cfg.ReadTimeout, - WriteTimeout: cfg.WriteTimeout, + server.srv = &http.Server{ + Addr: cfg.General.ListenAddr, + Handler: server.getRouter(), + ReadTimeout: time.Duration(cfg.General.HTTPReadTimeoutMillis) * time.Millisecond, + WriteTimeout: time.Duration(cfg.General.HTTPWriteTimeoutMillis) * time.Millisecond, } - return srv, nil + return server, nil } func (s *Server) loadBasicAuthSecretFromFile() error { - if s.cfg.Config.General.BasicAuthSecretPath == "" { + if s.cfg.General.BasicAuthSecretPath == "" { return nil } // Create if the file does not exist - if _, err := os.Stat(s.cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { - err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) + if _, err := os.Stat(s.cfg.General.BasicAuthSecretPath); os.IsNotExist(err) { + err = os.WriteFile(s.cfg.General.BasicAuthSecretPath, []byte{}, 0o600) if err != nil { return fmt.Errorf("failed to create basic auth secret file: %w", err) } - s.cfg.Log.Info("Basic auth file created, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + s.log.Info("Basic auth file created, auth disabled until secret is configured", "file", s.cfg.General.BasicAuthSecretPath) s.basicAuthHash = "" return nil } // Read the secret from the file - secret, err := os.ReadFile(s.cfg.Config.General.BasicAuthSecretPath) + secret, err := os.ReadFile(s.cfg.General.BasicAuthSecretPath) if err != nil { return fmt.Errorf("failed to read basic auth secret file: %w", err) } + s.basicAuthHash = string(secret) if len(secret) == 0 { - s.cfg.Log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + s.log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", s.cfg.General.BasicAuthSecretPath) } else { - s.cfg.Log.Info("Basic auth enabled", "file", s.cfg.Config.General.BasicAuthSecretPath) + s.log.Info("Basic auth enabled", "file", s.cfg.General.BasicAuthSecretPath) } - s.basicAuthHash = string(secret) return nil } @@ -124,7 +113,7 @@ func (s *Server) getRouter() http.Handler { mux.Use(middleware.Recoverer) // Enable a custom HTTP Basic Auth middleware - mux.Use(BasicAuth("system-api", s.getBasicAuthHashedCredentials)) + mux.Use(BasicAuth("system-api", s.cfg.General.BasicAuthSecretSalt, s.getBasicAuthHashedCredentials)) // Common APIs mux.Get("/", s.handleLivenessCheck) @@ -145,7 +134,7 @@ func (s *Server) getRouter() http.Handler { mux.Post("/api/v1/file-upload/{file}", s.handleFileUpload) // Optionally, pprof - if s.cfg.Config.General.EnablePprof { + if s.cfg.General.EnablePprof { mux.Mount("/debug", middleware.Profiler()) s.log.Info("pprof API enabled: /debug/pprof/") } @@ -154,7 +143,7 @@ func (s *Server) getRouter() http.Handler { } func (s *Server) readPipeInBackground() { - file, err := os.OpenFile(s.cfg.Config.General.PipeFile, os.O_CREATE, os.ModeNamedPipe) + file, err := os.OpenFile(s.cfg.General.PipeFile, os.O_CREATE, os.ModeNamedPipe) if err != nil { s.log.Error("Open named pipe file error:", "error", err) return @@ -176,7 +165,7 @@ func (s *Server) readPipeInBackground() { } func (s *Server) Start() { - s.log.Info("Starting HTTP server", "listenAddress", s.cfg.Config.General.ListenAddr) + s.log.Info("Starting HTTP server", "listenAddress", s.cfg.General.ListenAddr) if err := s.srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { s.log.Error("HTTP server failed", "err", err) } @@ -189,8 +178,8 @@ func (s *Server) Shutdown(ctx context.Context) error { s.log.Error("HTTP server shutdown failed", "err", err) } - if s.cfg.Config.General.PipeFile != "" { - os.Remove(s.cfg.Config.General.PipeFile) + if s.cfg.General.PipeFile != "" { + os.Remove(s.cfg.General.PipeFile) } s.log.Info("HTTP server shutdown") @@ -273,12 +262,12 @@ func (s *Server) handleAction(w http.ResponseWriter, r *http.Request) { action := chi.URLParam(r, "action") s.log.Info("Received action", "action", action) - if s.cfg.Config == nil { + if s.cfg == nil { w.WriteHeader(http.StatusNotImplemented) return } - cmd, ok := s.cfg.Config.Actions[action] + cmd, ok := s.cfg.Actions[action] if !ok { w.WriteHeader(http.StatusBadRequest) return @@ -305,12 +294,12 @@ func (s *Server) handleFileUpload(w http.ResponseWriter, r *http.Request) { log := s.log.With("file", fileArg) log.Info("Receiving file upload") - if s.cfg.Config == nil { + if s.cfg == nil { w.WriteHeader(http.StatusNotImplemented) return } - filename, ok := s.cfg.Config.FileUploads[fileArg] + filename, ok := s.cfg.FileUploads[fileArg] if !ok { w.WriteHeader(http.StatusBadRequest) return @@ -354,7 +343,7 @@ func (s *Server) getBasicAuthHashedCredentials() map[string]string { } func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) { - if s.cfg.Config.General.BasicAuthSecretPath == "" { + if s.cfg.General.BasicAuthSecretPath == "" { s.log.Warn("Basic auth secret path not set") w.WriteHeader(http.StatusNotImplemented) return @@ -371,10 +360,11 @@ func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) // Create hash of the secret h := sha256.New() h.Write(secret) + h.Write([]byte(s.cfg.General.BasicAuthSecretSalt)) secretHash := hex.EncodeToString(h.Sum(nil)) // write secret to file - err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, []byte(secretHash), 0o600) + err = os.WriteFile(s.cfg.General.BasicAuthSecretPath, []byte(secretHash), 0o600) if err != nil { s.log.Error("Failed to write secret to file", "err", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/systemapi/server_test.go b/systemapi/server_test.go index 1dd37dd..0c4f782 100644 --- a/systemapi/server_test.go +++ b/systemapi/server_test.go @@ -22,11 +22,11 @@ func getTestLogger() *httplog.Logger { }) } -func getTestConfig() *HTTPServerConfig { - return &HTTPServerConfig{ - Log: getTestLogger(), - Config: NewSystemAPIConfig(), - } +func newTestServer(t *testing.T) *Server { + t.Helper() + srv, err := NewServer(getTestLogger(), NewSystemAPIConfig()) + require.NoError(t, err) + return srv } // Helper to execute an API request with optional basic auth @@ -61,8 +61,7 @@ func createRequestRunner(t *testing.T, router http.Handler, method, url string) func TestGeneralHandlers(t *testing.T) { // Instantiate the server - srv, err := NewServer(getTestConfig()) - require.NoError(t, err) + srv := newTestServer(t) router := srv.getRouter() // Test /livez @@ -94,31 +93,28 @@ func TestGeneralHandlers(t *testing.T) { func TestBasicAuth(t *testing.T) { tempDir := t.TempDir() basicAuthSecret := []byte("secret") + basicAuthSalt := "salt" // Create a hash of the basic auth secret h := sha256.New() h.Write(basicAuthSecret) + h.Write([]byte(basicAuthSalt)) basicAuthSecretHash := hex.EncodeToString(h.Sum(nil)) // Create the config - cfg := getTestConfig() - cfg.Config.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" + cfg := NewSystemAPIConfig() + cfg.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" + cfg.General.BasicAuthSecretSalt = basicAuthSalt // Create the server instance - _, err := NewServer(cfg) + srv, err := NewServer(getTestLogger(), cfg) require.NoError(t, err) // Ensure the basic auth secret file was created - _, err = os.Stat(cfg.Config.General.BasicAuthSecretPath) + _, err = os.Stat(cfg.General.BasicAuthSecretPath) require.NoError(t, err) - // Create the temporary file to store the basic auth secret - err = os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) - require.NoError(t, err) - - // Server will work now - srv, err := NewServer(cfg) - require.NoError(t, err) + // Get the router router := srv.getRouter() // Prepare request helpers @@ -138,7 +134,7 @@ func TestBasicAuth(t *testing.T) { require.Equal(t, http.StatusOK, code) // Ensure hash was written to file and is reproducible - secretFromFile, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) + secretFromFile, err := os.ReadFile(cfg.General.BasicAuthSecretPath) require.NoError(t, err) require.Equal(t, basicAuthSecretHash, string(secretFromFile)) From 918547ac3d75a5fad6fbd77c87707b8a99aeaa0b Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 19:02:10 +0100 Subject: [PATCH 7/7] warn if file content looks off --- systemapi/server.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/systemapi/server.go b/systemapi/server.go index 9002a12..6d888e0 100644 --- a/systemapi/server.go +++ b/systemapi/server.go @@ -98,6 +98,10 @@ func (s *Server) loadBasicAuthSecretFromFile() error { } s.basicAuthHash = string(secret) + if len(s.basicAuthHash) != 64 { + return fmt.Errorf("basic auth secret in %s does not look like a SHA256 hash (must be 64 characters)", s.cfg.General.BasicAuthSecretPath) + } + if len(secret) == 0 { s.log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", s.cfg.General.BasicAuthSecretPath) } else {