From 325f1753fbfa0259ea4a6cf258ea7658bffb8091 Mon Sep 17 00:00:00 2001 From: Corey Ogburn Date: Tue, 1 Aug 2023 10:38:59 -0600 Subject: [PATCH 1/2] Add AuthCheck to Import endpoint Removed call to GetMembers which required nodes/read permission that only superuser has. Added events/write Auth checks to previously added salt store actions. UI no longer shuffles users to the login screen when they get a 401 from an API request. --- html/js/app.js | 2 +- server/gridmembershandler.go | 25 ++++++++++--------------- server/modules/salt/saltstore.go | 10 ++++++++-- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/html/js/app.js b/html/js/app.js index 9adf43449..da26ec4d4 100644 --- a/html/js/app.js +++ b/html/js/app.js @@ -711,7 +711,7 @@ $(document).ready(function() { if (response) { const redirectCookie = this.getCookie('AUTH_REDIRECT'); if ((response.headers && response.headers['content-type'] == "text/html") || - (response.status == 401) || + (response.status == 401 && !response.request.responseURL.indexOf('/api/')) || (redirectCookie != null && redirectCookie.length > 0)) { this.deleteCookie('AUTH_REDIRECT'); this.showLogin(); diff --git a/server/gridmembershandler.go b/server/gridmembershandler.go index 6f9eb2492..d194e7847 100644 --- a/server/gridmembershandler.go +++ b/server/gridmembershandler.go @@ -76,26 +76,12 @@ func (h *GridMembersHandler) postImport(w http.ResponseWriter, r *http.Request) return } - members, err := h.server.GridMembersstore.GetMembers(ctx) - if err != nil { - web.Respond(w, r, http.StatusInternalServerError, err) - return - } - - _, gmExists := lo.Find(members, func(m *model.GridMember) bool { - return strings.EqualFold(m.Id, id) - }) - if !gmExists { - web.Respond(w, r, http.StatusNotFound, errors.New("grid member not found")) - return - } - uploadLimit := int64(h.server.Config.ClientParams.GridParams.MaxUploadSize) if uploadLimit == 0 { uploadLimit = 25 * 1024 * 1024 // 25 MiB } - err = r.ParseMultipartForm(uploadLimit) + err := r.ParseMultipartForm(uploadLimit) if err != nil { web.Respond(w, r, http.StatusBadRequest, err) return @@ -160,6 +146,15 @@ func (h *GridMembersHandler) postImport(w http.ResponseWriter, r *http.Request) ext = ext[1:] + // auth check before we do anything async, before we even process the upload, + // if the user is authorized to complete this action + // TODO: When we re-evaluate the permissions needed to SendFile, we should + // add that same permission check here + if err := h.server.CheckAuthorized(ctx, "write", "events"); err != nil { + web.Respond(w, r, http.StatusUnauthorized, err) + return + } + baseTargetDir := h.server.Config.ImportUploadDir // "/opt/sensoroni/uploads/" targetDir := filepath.Join(baseTargetDir, "processing", id) diff --git a/server/modules/salt/saltstore.go b/server/modules/salt/saltstore.go index 8ef46172c..3ee67020c 100644 --- a/server/modules/salt/saltstore.go +++ b/server/modules/salt/saltstore.go @@ -986,7 +986,11 @@ func (store *Saltstore) ManageMember(ctx context.Context, operation string, id s } func (store *Saltstore) SendFile(ctx context.Context, node string, from string, to string, cleanup bool) error { - // TODO: Auth Check + // TODO: re-evaluate necessary permissions when this feature is used for more + // than importing pcap/evtx. + if err := store.server.CheckAuthorized(ctx, "write", "events"); err != nil { + return err + } args := map[string]string{ "command": "send-file", @@ -1005,7 +1009,9 @@ func (store *Saltstore) SendFile(ctx context.Context, node string, from string, } func (store *Saltstore) Import(ctx context.Context, node string, file string, importer string) (*string, error) { - // TODO: Auth Check + if err := store.server.CheckAuthorized(ctx, "write", "events"); err != nil { + return nil, err + } args := map[string]string{ "command": "import-file", From d9b1b0733bfaa1bd454c6248cc14a372625efe31 Mon Sep 17 00:00:00 2001 From: Corey Ogburn Date: Tue, 1 Aug 2023 12:05:14 -0600 Subject: [PATCH 2/2] Add test for Auth Check --- server/gridmembershandler_test.go | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 server/gridmembershandler_test.go diff --git a/server/gridmembershandler_test.go b/server/gridmembershandler_test.go new file mode 100644 index 000000000..b2e4facd3 --- /dev/null +++ b/server/gridmembershandler_test.go @@ -0,0 +1,68 @@ +package server + +import ( + "bytes" + "context" + "encoding/hex" + "mime/multipart" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi" + "github.com/security-onion-solutions/securityonion-soc/config" + "github.com/security-onion-solutions/securityonion-soc/model" + "github.com/security-onion-solutions/securityonion-soc/web" + "github.com/stretchr/testify/assert" +) + +type rejectAuthorizer struct{} + +func (auth *rejectAuthorizer) CheckContextOperationAuthorized(ctx context.Context, operation string, target string) error { + return model.NewUnauthorized("", operation, target) +} +func (auth *rejectAuthorizer) CheckUserOperationAuthorized(user *model.User, operation string, target string) error { + return model.NewUnauthorized("", operation, target) +} + +func TestImportAuth(t *testing.T) { + h := &GridMembersHandler{ + server: &Server{ + Authorizer: &rejectAuthorizer{}, + Config: &config.ServerConfig{}, + }, + } + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + ff, err := writer.CreateFormFile("attachment", "file.pcap") + if err != nil { + t.Fatal(err) + } + + content, err := hex.DecodeString("a1b2c3d4000000") + assert.NoError(t, err) + + _, err = ff.Write(content) + assert.NoError(t, err) + + assert.NoError(t, writer.Close()) + + w := httptest.NewRecorder() + + r := httptest.NewRequest("POST", "/1_standalone/import", bytes.NewReader(body.Bytes())) + r.Header.Add("Content-Type", "multipart/form-data; boundary="+writer.Boundary()) + + c := chi.NewRouteContext() + c.URLParams.Add("id", "1_standalone") + ctx := context.WithValue(context.Background(), chi.RouteCtxKey, c) + ctx = context.WithValue(ctx, web.ContextKeyRequestStart, time.Now()) + + r = r.WithContext(ctx) + + h.postImport(w, r) + + assert.Equal(t, 401, w.Code) + assert.Equal(t, web.GENERIC_ERROR_MESSAGE, w.Body.String()) +}