From 347544f9294dcfe008937f9010dae150625aec4a Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Mon, 18 Nov 2024 12:35:25 +0100 Subject: [PATCH] HTTP Basic auth support (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Summary Enables basic auth support for API requests. The basic auth password is configurable through API and/or file. If set via API, the salted hash is stored in the file to persist across reboots. Config-file updates: ```toml [general] # 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 ``` `basic_auth_secret_path` specifies the file to store the salted, hashed secret in. It's loaded (or created) on startup. - if the file is not empty, API requests need to include a http basic auth password that matches that sha256 hash (user `admin`) - if empty, no authentication is required for API requests until secret is configured through API or file. if `/api/v1/set-basic-auth` is called, it uses the payload as secret (immediately) and writes the hash of the secret it to the file (for reuse across restarts). - if file does not exist, it is created (empty) Only the salted SHA256 hash of the password is stored, both in the file as well as in memory. The secret can be overwritten (updated) via API call, if the request provides the previous http basic auth secret. Also added tests and updated the README. --- ## ✅ I have run these commands * [x] `make lint` * [x] `make test` * [x] `go mod tidy` --- .gitignore | 3 +- README.md | 46 ++++++++++- cmd/system-api/main.go | 7 +- systemapi-config.toml | 11 ++- systemapi/config.go | 15 +++- systemapi/middleware.go | 52 ++++++++++++ systemapi/server.go | 169 ++++++++++++++++++++++++++++++--------- systemapi/server_test.go | 154 +++++++++++++++++++++++++++++++++++ 8 files changed, 405 insertions(+), 52 deletions(-) create mode 100644 systemapi/middleware.go create mode 100644 systemapi/server_test.go 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 ac81663..fe1320b 100644 --- a/README.md +++ b/README.md @@ -12,10 +12,8 @@ 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 +- All actions show up in the event log --- @@ -93,3 +91,43 @@ $ 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 API endpoints are initially unauthenticated, until a secret is configured +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. + +The config file ([systemapi-config.toml](./systemapi-config.toml)) includes a `basic_auth_secret_path`. +- 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 +# 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 + +# Initially, requests are unauthenticated +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 + +# API calls work if correct basic auth credentials are provided +curl -v -u admin:foobar localhost:3535/livez + +# The update also shows up in the logs +curl -u admin:foobar localhost:3535/logs +``` diff --git a/cmd/system-api/main.go b/cmd/system-api/main.go index d32242c..e849644 100644 --- a/cmd/system-api/main.go +++ b/cmd/system-api/main.go @@ -91,12 +91,9 @@ 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 } go server.Start() diff --git a/systemapi-config.toml b/systemapi-config.toml index 21c7fb9..1b8b291 100644 --- a/systemapi-config.toml +++ b/systemapi-config.toml @@ -1,14 +1,23 @@ [general] listen_addr = "0.0.0.0:3535" pipe_file = "pipe.fifo" +pprof = true log_json = false log_debug = true +# 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 c034135..cd00cfa 100644 --- a/systemapi/config.go +++ b/systemapi/config.go @@ -7,10 +7,17 @@ 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"` + 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 new file mode 100644 index 0000000..c7ee1d7 --- /dev/null +++ b/systemapi/middleware.go @@ -0,0 +1,52 @@ +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, 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 + hashedCredentials := getHashedCredentials() + + // If no credentials are set, just pass through (unauthenticated) + if len(hashedCredentials) == 0 { + next.ServeHTTP(w, r) + return + } + + // Load credentials from request + user, pass, ok := r.BasicAuth() + if !ok { + basicAuthFailed(w, realm) + return + } + + // 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 + credPassHash, credUserOk := hashedCredentials[user] + if !credUserOk || subtle.ConstantTimeCompare([]byte(userPassHash), []byte(credPassHash)) != 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..6d888e0 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" @@ -21,58 +23,91 @@ import ( "github.com/go-chi/httplog/v2" ) -type HTTPServerConfig struct { - Config *SystemAPIConfig - Log *httplog.Logger - EnablePprof bool - - 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 eventsLock sync.RWMutex + + 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), } - if cfg.Config.General.PipeFile != "" { - os.Remove(cfg.Config.General.PipeFile) - err := syscall.Mknod(cfg.Config.General.PipeFile, syscall.S_IFIFO|0o666, 0) + // Load (or create) file with basic auth secret hash + err = server.loadBasicAuthSecretFromFile() + if err != nil { + return nil, err + } + + // Setup the pipe file + 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 + 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 server, nil +} + +func (s *Server) loadBasicAuthSecretFromFile() error { + if s.cfg.General.BasicAuthSecretPath == "" { + return nil + } + + // Create if the file does not exist + 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.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.General.BasicAuthSecretPath) + if err != nil { + return fmt.Errorf("failed to read basic auth secret file: %w", err) } - srv.srv = &http.Server{ - Addr: cfg.Config.General.ListenAddr, - Handler: srv.getRouter(), - ReadTimeout: cfg.ReadTimeout, - WriteTimeout: cfg.WriteTimeout, + 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) } - return srv, nil + 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 { + s.log.Info("Basic auth enabled", "file", s.cfg.General.BasicAuthSecretPath) + } + return nil } func (s *Server) getRouter() http.Handler { @@ -81,24 +116,38 @@ func (s *Server) getRouter() http.Handler { mux.Use(httplog.RequestLogger(s.log)) mux.Use(middleware.Recoverer) + // Enable a custom HTTP Basic Auth middleware + mux.Use(BasicAuth("system-api", s.cfg.General.BasicAuthSecretSalt, 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.General.EnablePprof { mux.Mount("/debug", middleware.Profiler()) + s.log.Info("pprof API enabled: /debug/pprof/") } return mux } 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 @@ -120,7 +169,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) } @@ -133,8 +182,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") @@ -185,6 +234,7 @@ func (s *Server) writeEventsAsText(w http.ResponseWriter) { return } } + w.WriteHeader(http.StatusOK) } func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { @@ -195,7 +245,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") @@ -205,6 +255,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) { @@ -215,12 +266,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 @@ -247,12 +298,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 @@ -285,3 +336,47 @@ 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) getBasicAuthHashedCredentials() map[string]string { + // dynamic because can be set at runtime + hashedCredentials := make(map[string]string) + if s.basicAuthHash != "" { + hashedCredentials["admin"] = s.basicAuthHash + } + return hashedCredentials +} + +func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) { + if s.cfg.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 + } + + // 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.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.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 new file mode 100644 index 0000000..0c4f782 --- /dev/null +++ b/systemapi/server_test.go @@ -0,0 +1,154 @@ +package systemapi + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "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 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 +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, requestBody) + require.NoError(t, err) + if basicAuthUser != "" { + req.SetBasicAuth(basicAuthUser, basicAuthPass) + } + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + 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 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) + } +} + +func TestGeneralHandlers(t *testing.T) { + // Instantiate the server + srv := newTestServer(t) + router := srv.getRouter() + + // Test /livez + code, _ := execRequest(t, router, http.MethodGet, "/livez", nil) + require.Equal(t, http.StatusOK, code) + + // /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)) + + // Add an event + 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) { + 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 := NewSystemAPIConfig() + cfg.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" + cfg.General.BasicAuthSecretSalt = basicAuthSalt + + // Create the server instance + srv, err := NewServer(getTestLogger(), cfg) + require.NoError(t, err) + + // Ensure the basic auth secret file was created + _, err = os.Stat(cfg.General.BasicAuthSecretPath) + require.NoError(t, err) + + // Get the router + router := srv.getRouter() + + // Prepare request helpers + 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) + 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 + code, _ = reqSetBasicAuthSecret("", "", bytes.NewReader(basicAuthSecret)) + require.Equal(t, http.StatusOK, code) + + // Ensure hash was written to file and is reproducible + secretFromFile, err := os.ReadFile(cfg.General.BasicAuthSecretPath) + require.NoError(t, err) + require.Equal(t, basicAuthSecretHash, string(secretFromFile)) + + // From here on, /livez shoud fail without basic auth + code, _ = reqGetLiveZ("", "", nil) + require.Equal(t, http.StatusUnauthorized, code) + + // /livez should work with basic auth + 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) +}