diff --git a/cmd/admin/handlers/get.go b/cmd/admin/handlers/get.go index c101fd8b..beaabd7e 100644 --- a/cmd/admin/handlers/get.go +++ b/cmd/admin/handlers/get.go @@ -110,12 +110,15 @@ func (h *HandlersAdmin) CarvesDownloadHandler(w http.ResponseWriter, r *http.Req log.Info().Msg("empty carve session") return } - // Check if carve is archived already carve, err := h.Carves.GetBySession(carveSession) if err != nil { log.Err(err).Msgf("error getting carve") return } + if carve.EnvironmentID != env.ID { + log.Info().Msgf("carve env %d does not match requested env %d", carve.EnvironmentID, env.ID) + return + } var archived *carves.CarveResult if !carve.Archived { archived, err = h.Carves.Archive(carveSession, h.CarvesFolder) diff --git a/cmd/admin/oidc.go b/cmd/admin/oidc.go index 570ba566..4d8ea1a7 100644 --- a/cmd/admin/oidc.go +++ b/cmd/admin/oidc.go @@ -207,6 +207,15 @@ func oidcCallbackHandler(w http.ResponseWriter, r *http.Request) { // if JITProvision is enabled; reject otherwise. Threat T16, T25. func resolveOIDCUser(identity auth.ResolvedIdentity) (users.AdminUser, error) { if exists, existing := adminUsers.ExistsGet(identity.PreferredUsername); exists { + if existing.AuthSource == "" { + return users.AdminUser{}, fmt.Errorf("username %q is a local account and cannot be claimed by federated login", identity.PreferredUsername) + } + if existing.AuthSource != "oidc" { + if err := adminUsers.ChangeAuthSource(existing.Username, "oidc"); err != nil { + return users.AdminUser{}, fmt.Errorf("updating auth source: %w", err) + } + existing.AuthSource = "oidc" + } return existing, nil } if flagParams == nil || flagParams.OIDC == nil || !flagParams.OIDC.JITProvision { @@ -219,6 +228,7 @@ func resolveOIDCUser(identity auth.ResolvedIdentity) (users.AdminUser, error) { if err != nil { return users.AdminUser{}, fmt.Errorf("new user: %w", err) } + u.AuthSource = "oidc" if err := adminUsers.Create(u); err != nil { return users.AdminUser{}, fmt.Errorf("create user: %w", err) } diff --git a/cmd/admin/templates/node.html b/cmd/admin/templates/node.html index b65219e2..607bfbb3 100644 --- a/cmd/admin/templates/node.html +++ b/cmd/admin/templates/node.html @@ -564,7 +564,7 @@ targets: 1, render: function (data, type, row, meta) { if (type === 'display') { - return '
' + data + '
'; + return '
' + $('
').text(data).html() + '
'; } else { return data; } diff --git a/cmd/api/handlers/auth_logout.go b/cmd/api/handlers/auth_logout.go index d792ee7e..f987abd7 100644 --- a/cmd/api/handlers/auth_logout.go +++ b/cmd/api/handlers/auth_logout.go @@ -90,6 +90,7 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) { // tenant URL + client_id without an actual session to terminate // (pentest finding: unauthenticated IdP metadata disclosure). var authenticated bool + var userAuthSource string tokenCookie, err := r.Cookie("osctrl_token") if err == nil && tokenCookie.Value != "" && len(h.JWTSecret) > 0 { claims, valid := h.Users.CheckToken(string(h.JWTSecret), tokenCookie.Value) @@ -100,6 +101,9 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) { // client-side cookies and return the IdP URL. log.Warn().Err(cerr).Str("user", claims.Username).Msg("logout: ClearToken failed") } + if exists, u := h.Users.ExistsGet(claims.Username); exists { + userAuthSource = u.AuthSource + } } } @@ -159,7 +163,8 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) { // the OIDC callback (auth_oidc.go OIDCCallbackHandler). SAML and // password flows never touch it. So: // id_token cookie present + valid token → OIDC session - // id_token cookie absent → SAML or password session + // id_token cookie absent + auth_source="saml" → SAML session + // id_token cookie absent + auth_source="" → password session // // Anonymous callers always get the empty response (pentest // T-IDP-DISCLOSURE: no IdP scrape without auth). @@ -185,7 +190,7 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) { resp.IdPLogoutURL = oidcProvider.EndSessionURL() resp.IdPClientID = oidcClientID resp.IdPIDTokenHint = idTokenHint - case !isOIDCSession && samlProvider != nil: + case !isOIDCSession && samlProvider != nil && userAuthSource == "saml": resp.AuthSource = "saml" if samlLogoutURL != "" { resp.IdPLogoutURL = samlLogoutURL diff --git a/cmd/api/handlers/auth_resolve.go b/cmd/api/handlers/auth_resolve.go index 4ad8fc4b..ec92c77f 100644 --- a/cmd/api/handlers/auth_resolve.go +++ b/cmd/api/handlers/auth_resolve.go @@ -48,6 +48,18 @@ func (h *HandlersApi) resolveFederatedUser(identity auth.ResolvedIdentity, jitPr return users.AdminUser{}, fmt.Errorf("%w: empty username", ErrAuthUserRejected) } if exists, existing := h.Users.ExistsGet(identity.PreferredUsername); exists { + if existing.AuthSource == "" { + return users.AdminUser{}, fmt.Errorf("%w: username %q is a local account and cannot be claimed by federated login", + ErrAuthUserRejected, identity.PreferredUsername) + } + // Allow cross-protocol federated login (oidc↔saml) — same IdP + // may serve both protocols. Update the stamp to the current one. + if existing.AuthSource != authSource { + if err := h.Users.ChangeAuthSource(existing.Username, authSource); err != nil { + return users.AdminUser{}, fmt.Errorf("%w: updating auth source: %v", ErrAuthUserRejected, err) + } + existing.AuthSource = authSource + } return existing, nil } if !jitProvision { diff --git a/cmd/api/handlers/carves.go b/cmd/api/handlers/carves.go index 837b19fa..3ed3a822 100644 --- a/cmd/api/handlers/carves.go +++ b/cmd/api/handlers/carves.go @@ -254,10 +254,9 @@ func (h *HandlersApi) CarvesRunHandler(w http.ResponseWriter, r *http.Request) { // the splice site (single-quote escape, LIKE pattern escape) — no // allowlist gate here so legitimate paths containing spaces or // non-ASCII characters round-trip correctly. - // Make sure the user has permissions to run queries in the environments for _, e := range c.Environments { - if !h.Users.CheckPermissions(ctx[ctxUser], users.QueryLevel, e) { - apiErrorResponse(w, fmt.Sprintf("%s has insufficient permissions to run queries in environment %s", ctx[ctxUser], e), http.StatusForbidden, nil) + if !h.Users.CheckPermissions(ctx[ctxUser], users.CarveLevel, e) { + apiErrorResponse(w, fmt.Sprintf("%s has insufficient permissions to run carves in environment %s", ctx[ctxUser], e), http.StatusForbidden, nil) return } } diff --git a/cmd/api/handlers/environments.go b/cmd/api/handlers/environments.go index 17666bb4..3f4eee0a 100644 --- a/cmd/api/handlers/environments.go +++ b/cmd/api/handlers/environments.go @@ -169,25 +169,27 @@ func (h *HandlersApi) EnvironmentsHandler(w http.ResponseWriter, r *http.Request apiErrorResponse(w, "error getting environments", http.StatusInternalServerError, err) return } - var out []environments.TLSEnvironment + var out []any if h.Users.IsAdmin(requester) { - out = envAll + for _, e := range envAll { + out = append(out, e) + } } else { access, gerr := h.Users.GetAccess(requester) if gerr != nil { - // Treat as "no access" and return [] — fail closed. access = nil } for _, e := range envAll { ea := access[e.UUID] - if ea.User || ea.Admin { + if ea.Admin { out = append(out, e) + } else if ea.User { + out = append(out, projectEnvironmentView(e)) } } } if out == nil { - // Marshal as [] not null for the SPA. - out = []environments.TLSEnvironment{} + out = []any{} } log.Debug().Msgf("Returned %d environment(s) to %s", len(out), requester) h.AuditLog.Visit(requester, r.URL.Path, strings.Split(r.RemoteAddr, ":")[0], auditlog.NoEnvironment) diff --git a/cmd/api/handlers/nodes.go b/cmd/api/handlers/nodes.go index b374d172..c37df832 100644 --- a/cmd/api/handlers/nodes.go +++ b/cmd/api/handlers/nodes.go @@ -216,6 +216,10 @@ func (h *HandlersApi) DeleteNodeHandler(w http.ResponseWriter, r *http.Request) apiErrorResponse(w, "error parsing POST body", http.StatusInternalServerError, err) return } + if _, err := h.Nodes.GetByUUIDEnv(n.UUID, env.ID); err != nil { + apiErrorResponse(w, "node not found", http.StatusNotFound, err) + return + } if err := h.Nodes.ArchiveDeleteByUUID(n.UUID); err != nil { if err.Error() == "record not found" { apiErrorResponse(w, "node not found", http.StatusNotFound, err) diff --git a/cmd/tls/handlers/post.go b/cmd/tls/handlers/post.go index be3e9b75..2c82cf48 100644 --- a/cmd/tls/handlers/post.go +++ b/cmd/tls/handlers/post.go @@ -5,6 +5,7 @@ import ( "compress/gzip" "context" "crypto/sha256" + "crypto/subtle" "encoding/base64" "encoding/json" "errors" @@ -789,6 +790,11 @@ func (h *HandlersTLS) CarveBlockHandler(w http.ResponseWriter, r *http.Request) blockCarve := false // Check if provided session_id matches with the request_id (carve query name) if carve, err := h.Carves.GetCheckCarve(t.SessionID, t.RequestID); err == nil { + if carve.EnvironmentID != env.ID { + log.Warn().Msgf("CarveBlockHandler: carve env %d does not match URL env %d", carve.EnvironmentID, env.ID) + utils.HTTPResponse(w, utils.JSONApplicationUTF8, http.StatusOK, types.CarveBlockResponse{Success: false}) + return + } // Record ingested data requestSize.WithLabelValues(string(env.UUID), "CarveBlock").Observe(float64(len(body))) log.Info().Msgf("node %d in %s environment ingested %d bytes for CarveBlockHandler endpoint", carve.NodeID, env.Name, len(body)) @@ -1205,10 +1211,11 @@ func (h *HandlersTLS) OsqueryConfigEndpointHandler(w http.ResponseWriter, r *htt confirmed := false integrityCheck := false for _, confEndpoint := range *h.ConfigEndpoints { - if confEndpoint.Environment == envVar && confEndpoint.Secret == secretVar { + envMatch := confEndpoint.Environment == envVar + secretMatch := subtle.ConstantTimeCompare([]byte(confEndpoint.Secret), []byte(secretVar)) == 1 + if envMatch && secretMatch { confirmed = true integrityCheck = confEndpoint.IntegrityCheck - break } } if !confirmed { diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index bcbcb564..ca7297c2 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -312,16 +312,12 @@ export async function logout(): Promise { if (idpLogoutUrl) { const postLogout = `${window.location.origin}/login`; const params = new URLSearchParams(); - if (authSource === 'saml') { - // SAML IdP logout (e.g. Auth0 /v2/logout) uses `returnTo` - // instead of the OIDC `post_logout_redirect_uri`. params.set('returnTo', postLogout); if (idpClientId) { params.set('client_id', idpClientId); } } else { - // OIDC RP-initiated logout — standard parameter names. params.set('post_logout_redirect_uri', postLogout); if (idpIdTokenHint) { params.set('id_token_hint', idpIdTokenHint); diff --git a/pkg/auth/oidc/claims.go b/pkg/auth/oidc/claims.go index 67d1f9e9..66fcfcd5 100644 --- a/pkg/auth/oidc/claims.go +++ b/pkg/auth/oidc/claims.go @@ -17,6 +17,7 @@ type idTokenClaims struct { Subject string `json:"sub"` PreferredUsername string `json:"preferred_username"` Email string `json:"email"` + EmailVerified *bool `json:"email_verified"` Name string `json:"name"` GivenName string `json:"given_name"` FamilyName string `json:"family_name"` @@ -68,9 +69,13 @@ func pickUsername(c idTokenClaims, raw map[string]any, claim string) string { return c.PreferredUsername } case "email": - if c.Email != "" { + if c.Email != "" && c.EmailVerified != nil && *c.EmailVerified { return c.Email } + if c.Email != "" && (c.EmailVerified == nil || !*c.EmailVerified) { + log.Warn().Msgf("oidc: email claim %q used as username but email_verified is not true — rejecting", c.Email) + return c.Subject + } case "sub": return c.Subject } diff --git a/pkg/auth/oidc/claims_test.go b/pkg/auth/oidc/claims_test.go index 2ec4bb27..c560b0da 100644 --- a/pkg/auth/oidc/claims_test.go +++ b/pkg/auth/oidc/claims_test.go @@ -9,11 +9,14 @@ import ( // always-available fallback per OIDC spec; tests verify that the // pickUsername logic prefers the configured claim when present and // falls back to subject otherwise. +func boolPtr(b bool) *bool { return &b } + func TestPickUsername(t *testing.T) { claims := idTokenClaims{ Subject: "sub-uuid-1234", PreferredUsername: "alice", Email: "alice@example.com", + EmailVerified: boolPtr(true), Name: "Alice Tester", GivenName: "Alice", FamilyName: "Tester", @@ -42,6 +45,28 @@ func TestPickUsername(t *testing.T) { } } +// TestPickUsernameUnverifiedEmail — when email_verified is false or +// nil, the email claim must NOT be used as a username (audit finding 5). +func TestPickUsernameUnverifiedEmail(t *testing.T) { + unverified := idTokenClaims{ + Subject: "sub-uuid-1234", + Email: "alice@example.com", + EmailVerified: boolPtr(false), + } + got := pickUsername(unverified, nil, "email") + if got != "sub-uuid-1234" { + t.Fatalf("unverified email should fall back to sub, got %q", got) + } + nilVerified := idTokenClaims{ + Subject: "sub-uuid-1234", + Email: "alice@example.com", + } + got = pickUsername(nilVerified, nil, "email") + if got != "sub-uuid-1234" { + t.Fatalf("nil email_verified should fall back to sub, got %q", got) + } +} + // TestPickUsernameAbsentClaim — when the configured claim isn't on // the id_token, we fall back to subject. Test by clearing // PreferredUsername and asking for it. diff --git a/pkg/carves/s3.go b/pkg/carves/s3.go index f4896eec..934fa5a1 100644 --- a/pkg/carves/s3.go +++ b/pkg/carves/s3.go @@ -134,6 +134,9 @@ func (carveS3 *CarverS3) Archive(carve CarvedFile, blocks []CarvedBlock) (*Carve } var parts []awsTypes.CompletedPart for _, b := range blocks { + if b.BlockID < 0 || b.BlockID > 9998 { + return nil, fmt.Errorf("block_id %d out of valid range [0, 9998]", b.BlockID) + } etag, err := carveS3.Concatenate(S3URLtoKey(b.Data, carveS3.S3Config.Bucket), fkey, b.BlockID+1, uploadOutput.UploadId) if err != nil { return nil, fmt.Errorf("error concatenating - %w", err) diff --git a/pkg/carves/utils.go b/pkg/carves/utils.go index 213e494c..cbe80d5a 100644 --- a/pkg/carves/utils.go +++ b/pkg/carves/utils.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/base64" "fmt" + "path/filepath" "strings" "github.com/jmpsec/osctrl/pkg/utils" @@ -50,7 +51,7 @@ func S3URLtoKey(s3url, bucket string) string { // Function to generate a local file for carve archives func GenerateArchiveName(carve CarvedFile) string { cPath := strings.ReplaceAll(strings.ReplaceAll(carve.Path, "/", "-"), "\\", "-") - return fmt.Sprintf(LocalFile, carve.UUID, carve.SessionID, cPath) + return fmt.Sprintf(LocalFile, filepath.Base(carve.UUID), filepath.Base(carve.SessionID), filepath.Base(cPath)) } // Function to check if data is compressed using zstd diff --git a/pkg/config/types.go b/pkg/config/types.go index a07c796f..c154dbfb 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -266,7 +266,7 @@ type YAMLConfigurationSAML struct { MetaDataURL string `yaml:"metadataUrl"` RootURL string `yaml:"rootUrl"` LoginURL string `yaml:"loginUrl"` - LogoutURL string `yaml:"logoutUrl"` + LogoutURL string `yaml:"logoutUrl" mapstructure:"logoutUrl"` JITProvision bool `yaml:"jitProvision" mapstructure:"jitProvision"` // UsernameAttribute names the SAML attribute (by Name or // FriendlyName) whose value becomes the osctrl username. diff --git a/pkg/logging/process.go b/pkg/logging/process.go index 77aeb83c..c4a1205d 100644 --- a/pkg/logging/process.go +++ b/pkg/logging/process.go @@ -55,10 +55,12 @@ func (l *LoggerTLS) ProcessLogQueryResult(queriesWrite types.QueryWriteRequest, node, err := l.Nodes.GetByKey(queriesWrite.NodeKey) if err != nil { log.Err(err).Msg("error retrieving node") + return } - // Integrity check + // Integrity check — hard reject on env mismatch if envid != node.EnvironmentID { - log.Error().Msgf("ProcessLogQueryResult: EnvID[%d] does not match Node.EnvironmentID[%d]", envid, node.EnvironmentID) + log.Error().Msgf("ProcessLogQueryResult: EnvID[%d] does not match Node.EnvironmentID[%d] — dropping results", envid, node.EnvironmentID) + return } // Tap into results so we can update internal metrics for q, r := range queriesWrite.Queries { diff --git a/pkg/users/users.go b/pkg/users/users.go index e4a9b884..8d299854 100644 --- a/pkg/users/users.go +++ b/pkg/users/users.go @@ -158,6 +158,12 @@ func (m *UserManager) HashPasswordWithSalt(password string) (string, error) { // failure is non-fatal — login succeeds even if the rehash write // fails (next login retries). func (m *UserManager) CheckLoginCredentials(username, password string) (bool, AdminUser) { + if password == "" { + if dummyHash != nil { + _ = bcrypt.CompareHashAndPassword(dummyHash, []byte(password)) + } + return false, AdminUser{} + } // Check if we should include service users user, err := m.Get(username) if err != nil { @@ -309,9 +315,23 @@ func (m *UserManager) Create(user AdminUser) error { // New empty user func (m *UserManager) New(username, password, email, fullname string, admin, service bool) (AdminUser, error) { if !m.Exists(username) { - passhash, err := m.HashPasswordWithSalt(password) - if err != nil { - return AdminUser{}, err + var passhash string + if password == "" { + randomBytes := make([]byte, 32) + if _, err := cryptorand.Read(randomBytes); err != nil { + return AdminUser{}, fmt.Errorf("generate random token: %w", err) + } + h, err := m.HashPasswordWithSalt(hex.EncodeToString(randomBytes)) + if err != nil { + return AdminUser{}, err + } + passhash = h + } else { + h, err := m.HashPasswordWithSalt(password) + if err != nil { + return AdminUser{}, err + } + passhash = h } return AdminUser{ Username: username, @@ -389,6 +409,20 @@ func (m *UserManager) ChangeService(username string, service bool) error { return nil } +// ChangeAuthSource to modify the auth_source for a user +func (m *UserManager) ChangeAuthSource(username, authSource string) error { + user, err := m.Get(username) + if err != nil { + return fmt.Errorf("error getting user %w", err) + } + if authSource != user.AuthSource { + if err := m.DB.Model(&user).Updates(map[string]interface{}{"auth_source": authSource}).Error; err != nil { + return err + } + } + return nil +} + // All get all users func (m *UserManager) All() ([]AdminUser, error) { var users []AdminUser