diff --git a/daemon/access.go b/daemon/access.go index d04a2d413c8..7734772f8d3 100644 --- a/daemon/access.go +++ b/daemon/access.go @@ -38,6 +38,7 @@ import ( "github.com/snapcore/snapd/overlord/ifacestate" "github.com/snapcore/snapd/polkit" "github.com/snapcore/snapd/sandbox/cgroup" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/strutil" ) @@ -77,21 +78,12 @@ func checkPolkitActionImpl(r *http.Request, ucred *ucrednet, action string) *api // An access checker will either allow a request, deny it, or return // accessUnknown, which indicates the decision should be delegated to // the next access checker. +// +// The CheckAccess method returns an *apiError if the request is denied, +// a seclog.AuthzChecks struct describing the result of each authorization +// check performed, and the intended access level for the check. type accessChecker interface { - CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError -} - -// requireSockets ensures the request was received via one of the specified sockets. -func requireSockets(ucred *ucrednet, sockets []string) *apiError { - if ucred == nil { - return Forbidden("access denied") - } - - if !strutil.ListContains(sockets, ucred.Socket) { - return Forbidden("access denied") - } - - return nil + CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) } type accessLevel string @@ -100,6 +92,9 @@ const ( accessLevelRoot accessLevel = "root" accessLevelAuthenticated accessLevel = "authenticated" accessLevelOpen accessLevel = "open" + // accessLevelNotEvaluated is returned when byActionAccess fails before + // delegation; no authorization checks ran so the level is unknown. + accessLevelNotEvaluated accessLevel = "not-evaluated" ) type accessOptions struct { @@ -130,56 +125,162 @@ func (o accessOptions) validate() error { return nil } -func checkAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState, opts accessOptions) *apiError { - if err := opts.validate(); err != nil { - return InternalError(err.Error()) +// checkPrerequisites runs prerequisite authorization checks that must all pass +// before access level checks are evaluated. These include peer credentials, +// socket restrictions, and interface requirements. +func checkPrerequisites(d *Daemon, r *http.Request, ucred *ucrednet, checks *seclog.AuthzChecks, opts accessOptions) *apiError { + // Mark applicable checks as AuthzNotReached (will become Pass/Fail during evaluation). + // AccessOptions is intentionally not reset here: it is set by the caller after + // opts.validate() has run. + checks.PeerCreds = seclog.AuthzNotReached + if len(opts.Sockets) != 0 { + checks.Socket = seclog.AuthzNotReached + } + if opts.InterfaceAccess != nil { + checks.Interface = seclog.AuthzNotReached } - if rspe := requireSockets(ucred, opts.Sockets); rspe != nil { - return rspe + // Peer credentials check + if ucred == nil { + checks.PeerCreds = seclog.AuthzFail + return Forbidden("access denied") } + checks.PeerCreds = seclog.AuthzPass + // Socket check + if len(opts.Sockets) != 0 { + if !strutil.ListContains(opts.Sockets, ucred.Socket) { + checks.Socket = seclog.AuthzFail + return Forbidden("access denied") + } + checks.Socket = seclog.AuthzPass + } + + // Interface check if opts.InterfaceAccess != nil { - // No interface checks are made if request is coming from snapd.socket - // to account for the snapd-control interface. rspe := requireInterfaceApiAccess(d, r, ucred, *opts.InterfaceAccess) if rspe != nil { + checks.Interface = seclog.AuthzFail return rspe } + checks.Interface = seclog.AuthzPass + } + + return nil +} + +// checkAccessLevelAuthorization determines if the user is authorized at the +// required access level by evaluating multiple authorization methods. Any single +// method can grant access: open access, user authentication, root UID, or polkit. +func checkAccessLevelAuthorization(r *http.Request, ucred *ucrednet, user *auth.UserState, checks *seclog.AuthzChecks, opts accessOptions) *apiError { + // Mark applicable checks as AuthzNotReached (will become Pass/Fail during evaluation). + // Checks that do not apply to the current access level remain AuthzNotApplicable. + switch opts.AccessLevel { + case accessLevelOpen: + checks.OpenAccess = seclog.AuthzNotReached + case accessLevelAuthenticated: + checks.UserAuth = seclog.AuthzNotReached + checks.Root = seclog.AuthzNotReached + if opts.PolkitAction != "" { + checks.Polkit = seclog.AuthzNotReached + } + case accessLevelRoot: + checks.Root = seclog.AuthzNotReached + if opts.PolkitAction != "" { + checks.Polkit = seclog.AuthzNotReached + } } + // Access level checks. All except the final polkit check returns on success. + if opts.AccessLevel == accessLevelOpen { + checks.OpenAccess = seclog.AuthzPass return nil } + // accessLevelOpen cannot fail + // Snapd local macaroon check - snapd user authentication if opts.AccessLevel == accessLevelAuthenticated && user != nil { - // user != nil means we have an authenticated user + checks.UserAuth = seclog.AuthzPass return nil } + if opts.AccessLevel == accessLevelAuthenticated { + checks.UserAuth = seclog.AuthzFail + // Even though snapd user authentication failed, + // we still accept root or polkit authorization as + // permitted alternatives. + } + // System UID based root check for privileged actions. if ucred.Uid == 0 { + checks.Root = seclog.AuthzPass return nil } + checks.Root = seclog.AuthzFail - // We check polkit last because it may result in the user - // being prompted for authorisation. This should be avoided if - // access is otherwise granted. + // Polkit check - system user authentication/authorization (policy dependant) for privileged actions. + // This happens last because it may prompt the user. if opts.PolkitAction != "" { - return checkPolkitAction(r, ucred, opts.PolkitAction) + rspe := checkPolkitAction(r, ucred, opts.PolkitAction) + if rspe == nil { + checks.Polkit = seclog.AuthzPass + return nil + } + checks.Polkit = seclog.AuthzFail + return rspe } - // XXX: when to 403 vs 401? + // If we reach here, all access level checks failed if opts.AccessLevel == accessLevelAuthenticated || opts.InterfaceAccess != nil { + // At this point, if authenticated access and/or interface access is required, + // it means that accessLevelRoot (root or polkit) was either implicitly or + // explicitly required and failed return Unauthorized("access denied") } + // Explicitly required accessLevelRoot, via system UID or polkit checks was not authorized. return Forbidden("access denied") } +func checkAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState, opts accessOptions) (*apiError, seclog.AuthzChecks, accessLevel) { + // All checks default to AuthzNotApplicable + checks := seclog.NewAuthzChecks() + level := opts.AccessLevel + + if err := opts.validate(); err != nil { + checks.AccessOptions = seclog.AuthzFail + return InternalError(err.Error()), checks, level + } + checks.AccessOptions = seclog.AuthzPass + + if err := checkPrerequisites(d, r, ucred, &checks, opts); err != nil { + return err, checks, level + } + + if err := checkAccessLevelAuthorization(r, ucred, user, &checks, opts); err != nil { + return err, checks, level + } + + return nil, checks, level +} + +// isAdministrativeAccess reports whether authz audit events should be emitted: +// the intended access level is authenticated or root and any authorization +// checks were evaluated. Dispatch-only failures return accessLevelNotEvaluated +// with empty checks and are excluded. +func isAdministrativeAccess(level accessLevel, checks seclog.AuthzChecks) bool { + switch level { + case accessLevelAuthenticated, accessLevelRoot: + return checks.AnyPerformed() + default: + return false + } +} + // openAccess allows requests without authentication, provided they // have peer credentials and were not received on snapd-snap.socket type openAccess struct{} -func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelOpen, Sockets: []string{dirs.SnapdSocket}, @@ -202,7 +303,7 @@ type authenticatedAccess struct { Polkit string } -func (ac authenticatedAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac authenticatedAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelAuthenticated, Sockets: []string{dirs.SnapdSocket}, @@ -215,7 +316,7 @@ func (ac authenticatedAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucr // were not received on snapd-snap.socket type rootAccess struct{} -func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelRoot, Sockets: []string{dirs.SnapdSocket}, @@ -226,7 +327,7 @@ func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, us // snapAccess allows requests from the snapd-snap.socket only. type snapAccess struct{} -func (ac snapAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac snapAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelOpen, Sockets: []string{dirs.SnapSocket}, @@ -323,7 +424,7 @@ type interfaceOpenAccess struct { Interfaces []string } -func (ac interfaceOpenAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac interfaceOpenAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelOpen, Sockets: []string{dirs.SnapdSocket, dirs.SnapSocket}, @@ -348,7 +449,7 @@ type interfaceAuthenticatedAccess struct { Polkit string } -func (ac interfaceAuthenticatedAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac interfaceAuthenticatedAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelAuthenticated, Sockets: []string{dirs.SnapdSocket, dirs.SnapSocket}, @@ -368,7 +469,7 @@ type interfaceProviderRootAccess struct { Interfaces []string } -func (ac interfaceProviderRootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac interfaceProviderRootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelRoot, Sockets: []string{dirs.SnapdSocket, dirs.SnapSocket}, @@ -398,7 +499,7 @@ type interfaceRootAccess struct { Polkit string } -func (ac interfaceRootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (ac interfaceRootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { opts := accessOptions{ AccessLevel: accessLevelRoot, Sockets: []string{dirs.SnapdSocket, dirs.SnapSocket}, @@ -432,7 +533,10 @@ type byActionAccess struct { const maxBodySize = 4 * 1024 * 1024 // 4MB -func (ac byActionAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +// CheckAccess routes by JSON "action" to a delegated checker. Failures before +// delegation are dispatch errors (BadRequest/InternalError), not authorization; +// they return empty AuthzChecks and accessLevelNotEvaluated. +func (ac byActionAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { switch ac.Default.(type) { // TODO: If less strict interfaces are needed as defaults then // we might need to introduce access checker sorting so that the @@ -440,11 +544,11 @@ func (ac byActionAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet // action access checker. case rootAccess, interfaceRootAccess, interfaceProviderRootAccess: default: - return InternalError("internal error: default access checker must have root-level access: got %T", ac.Default) + return InternalError("internal error: default access checker must have root-level access: got %T", ac.Default), seclog.NewAuthzChecks(), accessLevelNotEvaluated } if contentType := r.Header.Get("Content-Type"); contentType != "application/json" { - return BadRequest("unexpected content type: %q", contentType) + return BadRequest("unexpected content type: %q", contentType), seclog.NewAuthzChecks(), accessLevelNotEvaluated } req := actionRequest{} @@ -461,13 +565,13 @@ func (ac byActionAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet err := decoder.Decode(&req) if err != nil { if (errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF)) && lr.N <= 0 { - return BadRequest("body size limit exceeded") + return BadRequest("body size limit exceeded"), seclog.NewAuthzChecks(), accessLevelNotEvaluated } // Content type is JSON, but it's invalid - return BadRequest(err.Error()) + return BadRequest(err.Error()), seclog.NewAuthzChecks(), accessLevelNotEvaluated } if decoder.More() { - return BadRequest("unexpected data after request body") + return BadRequest("unexpected data after request body"), seclog.NewAuthzChecks(), accessLevelNotEvaluated } r.Body.Close() diff --git a/daemon/access_test.go b/daemon/access_test.go index 7e2cf19ea7d..7edfd2f5859 100644 --- a/daemon/access_test.go +++ b/daemon/access_test.go @@ -33,9 +33,15 @@ import ( "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/overlord/auth" "github.com/snapcore/snapd/polkit" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/testutil" ) +// errOnly discards the AuthzChecks and AccessLevel return values from CheckAccess. +func errOnly(e *daemon.APIError, _ seclog.AuthzChecks, _ daemon.AccessLevel) *daemon.APIError { + return e +} + type accessSuite struct { apiBaseSuite } @@ -49,16 +55,16 @@ var ( func (s *accessSuite) TestAccessOptionsValidation(c *C) { opts := daemon.AccessOptions{} - c.Check(daemon.CheckAccess(nil, nil, nil, nil, opts), ErrorMatches, `unexpected access level "" \(api 500\)`) + c.Check(errOnly(daemon.CheckAccess(nil, nil, nil, nil, opts)), ErrorMatches, `unexpected access level "" \(api 500\)`) opts = daemon.AccessOptions{AccessLevel: "some-level"} - c.Check(daemon.CheckAccess(nil, nil, nil, nil, opts), ErrorMatches, `unexpected access level "some-level" \(api 500\)`) + c.Check(errOnly(daemon.CheckAccess(nil, nil, nil, nil, opts)), ErrorMatches, `unexpected access level "some-level" \(api 500\)`) opts = daemon.AccessOptions{AccessLevel: "root"} - c.Check(daemon.CheckAccess(nil, nil, nil, nil, opts), ErrorMatches, `no sockets specified \(api 500\)`) + c.Check(errOnly(daemon.CheckAccess(nil, nil, nil, nil, opts)), ErrorMatches, `no sockets specified \(api 500\)`) opts = daemon.AccessOptions{AccessLevel: "root", Sockets: []string{"some-socket"}} - c.Check(daemon.CheckAccess(nil, nil, nil, nil, opts), ErrorMatches, `unexpected socket "some-socket" \(api 500\)`) + c.Check(errOnly(daemon.CheckAccess(nil, nil, nil, nil, opts)), ErrorMatches, `unexpected socket "some-socket" \(api 500\)`) } func (s *accessSuite) TestOpenAccess(c *C) { @@ -66,15 +72,15 @@ func (s *accessSuite) TestOpenAccess(c *C) { // openAccess denies access from snapd-snap.socket ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), DeepEquals, errForbidden) // Access allowed from snapd.socket ucred.Socket = dirs.SnapdSocket - c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), IsNil) // Access forbidden without peer credentials. This will need // to be revisited if the API is ever exposed over TCP. - c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, nil, nil)), DeepEquals, errForbidden) } func (s *accessSuite) TestAuthenticatedAccess(c *C) { @@ -92,26 +98,26 @@ func (s *accessSuite) TestAuthenticatedAccess(c *C) { // authenticatedAccess denies access from snapd-snap.socket ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(nil, req, ucred, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, req, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, user)), DeepEquals, errForbidden) // the same for unknown sockets ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: "unexpected.socket"} - c.Check(ac.CheckAccess(nil, req, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), DeepEquals, errForbidden) // With macaroon auth, a normal user is granted access ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(nil, req, ucred, user), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, user)), IsNil) // Macaroon access requires peer credentials - c.Check(ac.CheckAccess(nil, req, nil, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, nil, user)), DeepEquals, errForbidden) // Without macaroon auth, normal users are unauthorized - c.Check(ac.CheckAccess(nil, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), DeepEquals, errUnauthorized) // The root user is granted access without a macaroon ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(nil, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), IsNil) } func (s *accessSuite) TestAuthenticatedAccessPolkit(c *C) { @@ -130,9 +136,9 @@ func (s *accessSuite) TestAuthenticatedAccessPolkit(c *C) { return daemon.Forbidden("access denied") }) defer restore() - c.Check(ac.CheckAccess(nil, req, nil, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, req, nil, user), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, nil, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, nil, user)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), IsNil) // polkit is checked for regular users without macaroon auth restore = daemon.MockCheckPolkitAction(func(r *http.Request, u *daemon.Ucrednet, action string) *daemon.APIError { @@ -143,7 +149,7 @@ func (s *accessSuite) TestAuthenticatedAccessPolkit(c *C) { }) defer restore() ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(nil, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, ucred, nil)), IsNil) } func (s *accessSuite) TestCheckPolkitActionImpl(c *C) { @@ -211,22 +217,22 @@ func (s *accessSuite) TestRootAccess(c *C) { user := &auth.UserState{} // rootAccess denies access without ucred - c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, nil, nil, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, nil, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, nil, user)), DeepEquals, errForbidden) // rootAccess denies access from snapd-snap.socket ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, nil, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, user)), DeepEquals, errForbidden) // Non-root users are forbidden, even with macaroon auth ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, nil, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, user)), DeepEquals, errForbidden) // Root is granted access ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), IsNil) } func (s *accessSuite) TestSnapAccess(c *C) { @@ -234,12 +240,12 @@ func (s *accessSuite) TestSnapAccess(c *C) { // snapAccess allows access from snapd-snap.socket ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), IsNil) // access is forbidden on the main socket or without peer creds ucred.Socket = dirs.SnapdSocket - c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, ucred, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, nil, nil, nil)), DeepEquals, errForbidden) } func (s *accessSuite) TestRequireInterfaceApiAccessImpl(c *C) { @@ -271,26 +277,26 @@ plugs: var ac daemon.AccessChecker = daemon.InterfaceOpenAccess{Interfaces: []string{"snap-themes-control", "snap-refresh-control"}} // Access with no ucred data is forbidden - c.Check(ac.CheckAccess(d, nil, nil, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(d, nil, nil, nil)), DeepEquals, errForbidden) // Access from snapd.socket is allowed ucred := &daemon.Ucrednet{Uid: 1000, Pid: 1001, Socket: dirs.SnapdSocket} req := http.Request{RemoteAddr: ucred.String()} - c.Check(ac.CheckAccess(d, nil, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(d, nil, ucred, nil)), IsNil) c.Check(req.RemoteAddr, Equals, ucred.String()) // Access from unknown sockets is forbidden ucred = &daemon.Ucrednet{Uid: 1000, Pid: 1001, Socket: "unknown.socket"} - c.Check(ac.CheckAccess(d, nil, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(d, nil, ucred, nil)), DeepEquals, errForbidden) // Access from pids that cannot be mapped to a snap on // snapd-snap.socket are rejected ucred = &daemon.Ucrednet{Uid: 1000, Pid: 1001, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(d, nil, ucred, nil), DeepEquals, daemon.Forbidden("could not determine snap name for pid: not a snap")) + c.Check(errOnly(ac.CheckAccess(d, nil, ucred, nil)), DeepEquals, daemon.Forbidden("could not determine snap name for pid: not a snap")) // Access from snapd-snap.socket is rejected by default ucred = &daemon.Ucrednet{Uid: 1000, Pid: 42, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(d, nil, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(d, nil, ucred, nil)), DeepEquals, errForbidden) // Now connect the marker interface st := d.Overlord().State() @@ -304,7 +310,7 @@ plugs: // Access is allowed now that the snap has the plug connected req = http.Request{RemoteAddr: ucred.String()} - c.Check(ac.CheckAccess(s.d, &req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, &req, ucred, nil)), IsNil) // Interface is attached to RemoteAddr c.Check(req.RemoteAddr, Equals, fmt.Sprintf("%siface=snap-themes-control;", ucred)) @@ -320,7 +326,7 @@ plugs: }) st.Unlock() req = http.Request{RemoteAddr: ucred.String()} - c.Check(ac.CheckAccess(s.d, &req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, &req, ucred, nil)), IsNil) // Check that both interfaces are attached to RemoteAddr. // Since conns is a map, order is not guaranteed. c.Check(req.RemoteAddr, Matches, fmt.Sprintf("^%siface=(snap-themes-control&snap-refresh-control|snap-refresh-control&snap-themes-control);$", ucred)) @@ -335,7 +341,7 @@ plugs: }) st.Unlock() req = http.Request{RemoteAddr: ucred.String()} - c.Check(ac.CheckAccess(d, nil, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(d, nil, ucred, nil)), DeepEquals, errForbidden) c.Check(req.RemoteAddr, Equals, ucred.String()) } @@ -357,7 +363,7 @@ func (s *accessSuite) TestInterfaceOpenAccess(c *C) { return nil }) defer restore() - c.Check(ac.CheckAccess(s.d, nil, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, nil, ucred, nil)), IsNil) // Access is forbidden if requireInterfaceApiAccess() fails restore = daemon.MockRequireInterfaceApiAccess(func( @@ -366,7 +372,7 @@ func (s *accessSuite) TestInterfaceOpenAccess(c *C) { return errForbidden }) defer restore() - c.Check(ac.CheckAccess(s.d, nil, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, nil, ucred, nil)), DeepEquals, errForbidden) } func (s *accessSuite) TestInterfaceAuthenticatedAccess(c *C) { @@ -396,8 +402,8 @@ func (s *accessSuite) TestInterfaceAuthenticatedAccess(c *C) { return errForbidden }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errForbidden) - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errForbidden) // If requireInterfaceApiAccess succeeds, root is granted access restore = daemon.MockRequireInterfaceApiAccess(func( @@ -406,14 +412,14 @@ func (s *accessSuite) TestInterfaceAuthenticatedAccess(c *C) { return nil }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), IsNil) // Macaroon auth will grant a normal user access too ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapSocket} - c.Check(ac.CheckAccess(s.d, req, ucred, user), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), IsNil) // Without macaroon auth, normal users are unauthorized - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errUnauthorized) } func (s *accessSuite) TestInterfaceAuthenticatedAccessPolkit(c *C) { @@ -444,9 +450,9 @@ func (s *accessSuite) TestInterfaceAuthenticatedAccessPolkit(c *C) { return errForbidden }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), IsNil) ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket} - c.Check(ac.CheckAccess(s.d, req, ucred, user), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), IsNil) // polkit is checked for regular users without macaroon auth restore = daemon.MockCheckPolkitAction(func(r *http.Request, u *daemon.Ucrednet, action string) *daemon.APIError { @@ -456,7 +462,7 @@ func (s *accessSuite) TestInterfaceAuthenticatedAccessPolkit(c *C) { return nil }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), IsNil) } func (s *accessSuite) TestInterfaceProviderRootAccessCallsWithCorrectArgs(c *C) { @@ -491,7 +497,7 @@ func (s *accessSuite) TestInterfaceProviderRootAccessCallsWithCorrectArgs(c *C) return errForbidden }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errForbidden) c.Assert(called, Equals, 1) } @@ -555,7 +561,7 @@ plugs: req := &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errForbidden) // Now connect both interfaces st := d.Overlord().State() @@ -575,45 +581,45 @@ plugs: req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), IsNil) // connected-fwupd-caller, but on the plug side ucred = &daemon.Ucrednet{Uid: 0, Pid: 1042, Socket: dirs.SnapSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errForbidden) // disconnected-fwupd-caller ucred = &daemon.Ucrednet{Uid: 0, Pid: 10042, Socket: dirs.SnapSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errForbidden) // normal user has no access even with a Macaroon auth ucred = &daemon.Ucrednet{Uid: 42, Pid: 42, Socket: dirs.SnapSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errUnauthorized) // Without macaroon auth, normal users are unauthorized - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errUnauthorized) // on snapd socket, non-root is unauthorized ucred = &daemon.Ucrednet{Uid: 42, Pid: 123, Socket: dirs.SnapdSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errUnauthorized) // but root is ucred = &daemon.Ucrednet{Uid: 0, Pid: 123, Socket: dirs.SnapdSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), IsNil) } func (s *accessSuite) TestInterfaceRootAccessCallsWithCorrectArgs(c *C) { @@ -648,7 +654,7 @@ func (s *accessSuite) TestInterfaceRootAccessCallsWithCorrectArgs(c *C) { return errForbidden }) defer restore() - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errForbidden) c.Assert(called, Equals, 1) } @@ -700,7 +706,7 @@ plugs: req := &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errForbidden) // Now connect connected-fwupd-caller (plug) to fwupd-app (slot) st := d.Overlord().State() @@ -717,38 +723,38 @@ plugs: req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), IsNil) // fwupd-app, connected on the slot side ucred = &daemon.Ucrednet{Uid: 0, Pid: 42, Socket: dirs.SnapSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errForbidden) // normal user has no access even with a Macaroon auth ucred = &daemon.Ucrednet{Uid: 42, Pid: 1042, Socket: dirs.SnapSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, user), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, user)), DeepEquals, errUnauthorized) // Without macaroon auth, normal users are unauthorized - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errUnauthorized) // on snapd socket, non-root is unauthorized ucred = &daemon.Ucrednet{Uid: 42, Pid: 123, Socket: dirs.SnapdSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), DeepEquals, errUnauthorized) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), DeepEquals, errUnauthorized) // but root is ucred = &daemon.Ucrednet{Uid: 0, Pid: 123, Socket: dirs.SnapdSocket} req = &http.Request{ RemoteAddr: ucred.String(), } - c.Check(ac.CheckAccess(s.d, req, ucred, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, ucred, nil)), IsNil) } func (s *accessSuite) TestInterfaceRootAccessPolkit(c *C) { @@ -809,13 +815,13 @@ plugs: }) defer restore() // ucred is missing - c.Check(ac.CheckAccess(nil, req, nil, nil), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(nil, req, nil, nil)), DeepEquals, errForbidden) // user is root (on snapd.socket) - c.Check(ac.CheckAccess(nil, req, &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket}, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket}, nil)), IsNil) // snap request (as root) with relevant connected plug (on snapd-snap.socket) - c.Check(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 0, Pid: 1042, Socket: dirs.SnapSocket}, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 0, Pid: 1042, Socket: dirs.SnapSocket}, nil)), IsNil) // snap request without relevant connected plug (on snapd-snap.socket) - c.Check(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 0, Pid: 42, Socket: dirs.SnapSocket}, user), DeepEquals, errForbidden) + c.Check(errOnly(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 0, Pid: 42, Socket: dirs.SnapSocket}, user)), DeepEquals, errForbidden) // polkit is checked for snaps with connected plug called := 0 @@ -827,11 +833,11 @@ plugs: }) defer restore() // regular user (on snapd.socket) - c.Check(ac.CheckAccess(nil, req, &daemon.Ucrednet{Uid: 1001, Pid: 100, Socket: dirs.SnapdSocket}, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(nil, req, &daemon.Ucrednet{Uid: 1001, Pid: 100, Socket: dirs.SnapdSocket}, nil)), IsNil) // snap request (with macaroon) with relevant connected plug (on snapd-snap.socket) - c.Check(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 1001, Pid: 1042, Socket: dirs.SnapSocket}, user), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 1001, Pid: 1042, Socket: dirs.SnapSocket}, user)), IsNil) // snap request (without macaroon) with relevant connected plug (on snapd-snap.socket) - c.Check(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 1001, Pid: 1042, Socket: dirs.SnapSocket}, nil), IsNil) + c.Check(errOnly(ac.CheckAccess(s.d, req, &daemon.Ucrednet{Uid: 1001, Pid: 1042, Socket: dirs.SnapSocket}, nil)), IsNil) c.Check(called, Equals, 3) } @@ -989,7 +995,7 @@ func (s *accessSuite) TestByActionAccess(c *C) { for action := range byAction { cmt := Commentf("sub-test tcs[%d] failed for action %q", idx, action) - err := ac.CheckAccess(nil, reqWithAction(c, action, !tc.notJSON, tc.malformed), &tc.ucred, user) + err := errOnly(ac.CheckAccess(nil, reqWithAction(c, action, !tc.notJSON, tc.malformed), &tc.ucred, user)) if expectedErr := tc.expectedErr[action]; err != nil { c.Check(err, DeepEquals, expectedErr, cmt) } else { @@ -998,13 +1004,45 @@ func (s *accessSuite) TestByActionAccess(c *C) { } cmt := Commentf("sub-test tcs[%d] failed for default action", idx) - err := ac.CheckAccess(nil, reqWithAction(c, "default", !tc.notJSON, tc.malformed), &tc.ucred, user) + err := errOnly(ac.CheckAccess(nil, reqWithAction(c, "default", !tc.notJSON, tc.malformed), &tc.ucred, user)) if expectedErr := tc.expectedErr["default"]; err != nil { c.Check(err, DeepEquals, expectedErr, cmt) } else { c.Check(err, IsNil, cmt) } } + + // Delegated access levels depend on the sub-checker that actually runs. + ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket} + + err, _, level := ac.CheckAccess(nil, reqWithAction(c, "action-3", true, false), ucred, nil) + c.Check(err, IsNil) + c.Check(level, Equals, daemon.AccessLevelOpen) + + err, _, level = ac.CheckAccess(nil, reqWithAction(c, "default", true, false), ucred, nil) + c.Check(err, DeepEquals, errForbidden) + c.Check(level, Equals, daemon.AccessLevelRoot) +} + +func (s *accessSuite) TestByActionAccessReturnsNotEvaluatedLevelOnDispatchError(c *C) { + ac := daemon.ByActionAccess{Default: daemon.RootAccess{}} + ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} + + err, checks, level := ac.CheckAccess(nil, reqWithAction(c, "action-1", false, false), ucred, nil) + c.Check(err, DeepEquals, daemon.BadRequest(`unexpected content type: ""`)) + c.Check(checks, DeepEquals, seclog.NewAuthzChecks()) + c.Check(level, Equals, daemon.AccessLevelNotEvaluated) + + err, checks, level = ac.CheckAccess(nil, reqWithAction(c, "action-1", true, true), ucred, nil) + c.Check(err, DeepEquals, daemon.BadRequest("invalid character '}' looking for beginning of value")) + c.Check(checks, DeepEquals, seclog.NewAuthzChecks()) + c.Check(level, Equals, daemon.AccessLevelNotEvaluated) + + ac = daemon.ByActionAccess{Default: daemon.ByActionAccess{Default: daemon.RootAccess{}}} + err, checks, level = ac.CheckAccess(nil, reqWithAction(c, "unknown", true, false), ucred, nil) + c.Check(err, ErrorMatches, "internal error: default access checker must have root-level access.*") + c.Check(checks, DeepEquals, seclog.NewAuthzChecks()) + c.Check(level, Equals, daemon.AccessLevelNotEvaluated) } func (s *accessSuite) TestByActionAccessDefaultMustBeRoot(c *C) { @@ -1035,7 +1073,7 @@ func (s *accessSuite) TestByActionAccessDefaultMustBeRoot(c *C) { ac := daemon.ByActionAccess{Default: tc.ac} ucred := daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} - err = ac.CheckAccess(nil, req, &ucred, nil) + err = errOnly(ac.CheckAccess(nil, req, &ucred, nil)) if tc.canBeDefault { c.Assert(err, IsNil) } else { @@ -1054,7 +1092,7 @@ func (s *accessSuite) TestByActionAccessLargeJSON(c *C) { ac := daemon.ByActionAccess{Default: daemon.RootAccess{}} ucred := daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} - err = ac.CheckAccess(nil, req, &ucred, nil) + err = errOnly(ac.CheckAccess(nil, req, &ucred, nil)) c.Assert(err, DeepEquals, daemon.BadRequest("body size limit exceeded")) } @@ -1067,6 +1105,335 @@ func (s *accessSuite) TestByActionAccessDataAfterJOSN(c *C) { ac := daemon.ByActionAccess{Default: daemon.RootAccess{}} ucred := daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket} - err = ac.CheckAccess(nil, req, &ucred, nil) + err = errOnly(ac.CheckAccess(nil, req, &ucred, nil)) c.Assert(err, DeepEquals, daemon.BadRequest("unexpected data after request body")) } + +func (s *accessSuite) TestCheckPrerequisites(c *C) { + d := s.daemon(c) + + tests := []struct { + name string + ucred *daemon.Ucrednet + opts daemon.AccessOptions + mockInterface func() (restore func()) + expectErr *daemon.APIError + checkResults func(c *C, checks seclog.AuthzChecks) + }{ + { + name: "nil ucred - peer creds fail", + ucred: nil, + opts: daemon.AccessOptions{AccessLevel: "root", Sockets: []string{dirs.SnapdSocket}}, + expectErr: daemon.Forbidden("access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.PeerCreds, Equals, seclog.AuthzFail) + }, + }, + { + name: "wrong socket - socket check fails", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapSocket}, + opts: daemon.AccessOptions{AccessLevel: "root", Sockets: []string{dirs.SnapdSocket}}, + expectErr: daemon.Forbidden("access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.PeerCreds, Equals, seclog.AuthzPass) + c.Check(checks.Socket, Equals, seclog.AuthzFail) + }, + }, + { + name: "interface check fails", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + InterfaceAccess: &daemon.InterfaceAccessReqs{ + Interfaces: []string{"snap-themes-control"}, + Plug: true, + }, + }, + mockInterface: func() func() { + return daemon.MockRequireInterfaceApiAccess(func(d *daemon.Daemon, r *http.Request, ucred *daemon.Ucrednet, reqs daemon.InterfaceAccessReqs) *daemon.APIError { + return daemon.Forbidden("interface access denied") + }) + }, + expectErr: daemon.Forbidden("interface access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.PeerCreds, Equals, seclog.AuthzPass) + c.Check(checks.Socket, Equals, seclog.AuthzPass) + c.Check(checks.Interface, Equals, seclog.AuthzFail) + }, + }, + { + name: "all prerequisites pass", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + InterfaceAccess: &daemon.InterfaceAccessReqs{ + Interfaces: []string{"snap-themes-control"}, + Plug: true, + }, + }, + mockInterface: func() func() { + return daemon.MockRequireInterfaceApiAccess(func(d *daemon.Daemon, r *http.Request, ucred *daemon.Ucrednet, reqs daemon.InterfaceAccessReqs) *daemon.APIError { + return nil // success + }) + }, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.PeerCreds, Equals, seclog.AuthzPass) + c.Check(checks.Socket, Equals, seclog.AuthzPass) + c.Check(checks.Interface, Equals, seclog.AuthzPass) + }, + }, + { + name: "no interface access required - passes", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + }, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.PeerCreds, Equals, seclog.AuthzPass) + c.Check(checks.Socket, Equals, seclog.AuthzPass) + // Interface should still be NotApplicable (not checked) + c.Check(checks.Interface, Equals, seclog.AuthzNotApplicable) + }, + }, + } + + for _, tc := range tests { + c.Logf("Test case: %s", tc.name) + checks := seclog.NewAuthzChecks() + + var restore func() + if tc.mockInterface != nil { + restore = tc.mockInterface() + } + + req := httptest.NewRequest("GET", "/", nil) + err := daemon.CheckPrerequisites(d, req, tc.ucred, &checks, tc.opts) + + if tc.expectErr != nil { + c.Check(err, DeepEquals, tc.expectErr, Commentf("test case: %s", tc.name)) + } else { + c.Check(err, IsNil, Commentf("test case: %s", tc.name)) + } + + if tc.checkResults != nil { + tc.checkResults(c, checks) + } + + if restore != nil { + restore() + } + } +} + +func (s *accessSuite) TestCheckAccessLevelAuthorization(c *C) { + tests := []struct { + name string + ucred *daemon.Ucrednet + user *auth.UserState + opts daemon.AccessOptions + mockPolkit func() (restore func()) + expectErr *daemon.APIError + checkResults func(c *C, checks seclog.AuthzChecks) + }{ + { + name: "open access - succeeds immediately", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{AccessLevel: "open", Sockets: []string{dirs.SnapdSocket}}, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzPass) + }, + }, + { + name: "authenticated with user - succeeds", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: &auth.UserState{ID: 1, Email: "user@example.com"}, + opts: daemon.AccessOptions{AccessLevel: "authenticated", Sockets: []string{dirs.SnapdSocket}}, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzPass) + }, + }, + { + name: "authenticated without user, root uid - succeeds via root", + ucred: &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{AccessLevel: "authenticated", Sockets: []string{dirs.SnapdSocket}}, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzFail) + c.Check(checks.Root, Equals, seclog.AuthzPass) + }, + }, + { + name: "authenticated without user, non-root, with polkit - succeeds via polkit", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + PolkitAction: "io.snapcraft.snapd.manage", + }, + mockPolkit: func() func() { + return daemon.MockCheckPolkitAction(func(r *http.Request, ucred *daemon.Ucrednet, action string) *daemon.APIError { + return nil // authorized + }) + }, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzFail) + c.Check(checks.Root, Equals, seclog.AuthzFail) + c.Check(checks.Polkit, Equals, seclog.AuthzPass) + }, + }, + { + name: "authenticated fails all checks - returns Unauthorized", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{AccessLevel: "authenticated", Sockets: []string{dirs.SnapdSocket}}, + expectErr: daemon.Unauthorized("access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzFail) + c.Check(checks.Root, Equals, seclog.AuthzFail) + }, + }, + { + name: "root access level with root uid - succeeds", + ucred: &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{AccessLevel: "root", Sockets: []string{dirs.SnapdSocket}}, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzNotApplicable) + c.Check(checks.Root, Equals, seclog.AuthzPass) + c.Check(checks.Polkit, Equals, seclog.AuthzNotApplicable) + }, + }, + { + name: "root access level with polkit - succeeds via polkit", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{ + AccessLevel: "root", + Sockets: []string{dirs.SnapdSocket}, + PolkitAction: "io.snapcraft.snapd.manage", + }, + mockPolkit: func() func() { + return daemon.MockCheckPolkitAction(func(r *http.Request, ucred *daemon.Ucrednet, action string) *daemon.APIError { + return nil // authorized + }) + }, + expectErr: nil, + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzNotApplicable) + c.Check(checks.Root, Equals, seclog.AuthzFail) + c.Check(checks.Polkit, Equals, seclog.AuthzPass) + }, + }, + { + name: "root access level fails all checks - returns Forbidden", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{AccessLevel: "root", Sockets: []string{dirs.SnapdSocket}}, + expectErr: daemon.Forbidden("access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.OpenAccess, Equals, seclog.AuthzNotApplicable) + c.Check(checks.UserAuth, Equals, seclog.AuthzNotApplicable) + c.Check(checks.Root, Equals, seclog.AuthzFail) + c.Check(checks.Polkit, Equals, seclog.AuthzNotApplicable) + }, + }, + { + name: "polkit check fails - returns polkit error", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + PolkitAction: "io.snapcraft.snapd.manage", + }, + mockPolkit: func() func() { + return daemon.MockCheckPolkitAction(func(r *http.Request, ucred *daemon.Ucrednet, action string) *daemon.APIError { + return daemon.AuthCancelled("user cancelled") + }) + }, + expectErr: daemon.AuthCancelled("user cancelled"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.Polkit, Equals, seclog.AuthzFail) + }, + }, + { + name: "authenticated with interface access fails - returns Unauthorized", + ucred: &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: dirs.SnapdSocket}, + user: nil, + opts: daemon.AccessOptions{ + AccessLevel: "authenticated", + Sockets: []string{dirs.SnapdSocket}, + InterfaceAccess: &daemon.InterfaceAccessReqs{ + Interfaces: []string{"snap-themes-control"}, + Plug: true, + }, + }, + expectErr: daemon.Unauthorized("access denied"), + checkResults: func(c *C, checks seclog.AuthzChecks) { + c.Check(checks.UserAuth, Equals, seclog.AuthzFail) + c.Check(checks.Root, Equals, seclog.AuthzFail) + }, + }, + } + + for _, tc := range tests { + c.Logf("Test case: %s", tc.name) + checks := seclog.NewAuthzChecks() + + var restore func() + if tc.mockPolkit != nil { + restore = tc.mockPolkit() + } + + req := httptest.NewRequest("GET", "/", nil) + err := daemon.CheckAccessLevelAuthorization(req, tc.ucred, tc.user, &checks, tc.opts) + + if tc.expectErr != nil { + c.Check(err, DeepEquals, tc.expectErr, Commentf("test case: %s", tc.name)) + } else { + c.Check(err, IsNil, Commentf("test case: %s", tc.name)) + } + + if tc.checkResults != nil { + tc.checkResults(c, checks) + } + + if restore != nil { + restore() + } + } +} + +func (s *accessSuite) TestIsAdministrativeAccess(c *C) { + empty := seclog.NewAuthzChecks() + + c.Check(daemon.IsAdministrativeAccess(daemon.AccessLevelOpen, empty), Equals, false) + c.Check(daemon.IsAdministrativeAccess(daemon.AccessLevelNotEvaluated, empty), Equals, false) + + c.Check(daemon.IsAdministrativeAccess(daemon.AccessLevelAuthenticated, empty), Equals, false) + checks := seclog.NewAuthzChecks() + checks.PeerCreds = seclog.AuthzPass + c.Check(daemon.IsAdministrativeAccess(daemon.AccessLevelAuthenticated, checks), Equals, true) + + checks = seclog.NewAuthzChecks() + checks.Root = seclog.AuthzFail + c.Check(daemon.IsAdministrativeAccess(daemon.AccessLevelRoot, checks), Equals, true) +} diff --git a/daemon/api_users.go b/daemon/api_users.go index 3428c40f1c5..983df6ad380 100644 --- a/daemon/api_users.go +++ b/daemon/api_users.go @@ -21,6 +21,7 @@ package daemon import ( "encoding/json" + "fmt" "net/http" "regexp" "time" @@ -87,11 +88,8 @@ var isEmailish = regexp.MustCompile(`.@.*\..`).MatchString // apiLoginError logs a login failure to the security audit log and returns resp // unchanged. It is a convenience wrapper so that each error return path in // loginUser can log with a single call. -func apiLoginError(resp *apiError, snapdUser seclog.SnapdUser, code string) *apiError { - seclog.LogLoginFailure(snapdUser, seclog.Reason{ - Code: code, - Message: resp.Message, - }) +func apiLoginError(resp *apiError, snapdUser seclog.SnapdUser) *apiError { + seclog.LogLoginFailure(snapdUser, resp.reason()) return resp } @@ -105,8 +103,11 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response { decoder := json.NewDecoder(r.Body) if err := decoder.Decode(&loginData); err != nil { - return apiLoginError(BadRequest("cannot decode login data from request body: %v", err), - seclog.SnapdUser{}, seclog.ReasonInvalidAuthData) + return apiLoginError(&apiError{ + Status: 400, + Message: fmt.Sprintf("cannot decode login data from request body: %v", err), + Kind: client.ErrorKindInvalidAuthData, + }, seclog.SnapdUser{}) } if loginData.Email == "" && isEmailish(loginData.Username) { @@ -129,7 +130,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response { }, seclog.SnapdUser{ StoreUserName: loginData.Username, StoreUserEmail: loginData.Email, - }, seclog.ReasonInvalidAuthData) + }) } // Build the user identity for security audit logging. At this point we know @@ -150,13 +151,13 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response { Status: 401, Message: err.Error(), Kind: client.ErrorKindTwoFactorRequired, - }, snapdUser, seclog.ReasonTwoFactorRequired) + }, snapdUser) case store.Err2faFailed: return apiLoginError(&apiError{ Status: 401, Message: err.Error(), Kind: client.ErrorKindTwoFactorFailed, - }, snapdUser, seclog.ReasonTwoFactorFailed) + }, snapdUser) default: switch err := err.(type) { case store.InvalidAuthDataError: @@ -165,20 +166,24 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response { Message: err.Error(), Kind: client.ErrorKindInvalidAuthData, Value: err, - }, snapdUser, seclog.ReasonInvalidAuthData) + }, snapdUser) case store.PasswordPolicyError: return apiLoginError(&apiError{ Status: 401, Message: err.Error(), Kind: client.ErrorKindPasswordPolicy, Value: err, - }, snapdUser, seclog.ReasonPasswordPolicy) + }, snapdUser) } - reason := seclog.ReasonInternal + kind := client.ErrorKind(seclog.ReasonInternal) if err == store.ErrInvalidCredentials { - reason = seclog.ReasonInvalidCredentials + kind = client.ErrorKind(seclog.ReasonInvalidCredentials) } - return apiLoginError(Unauthorized(err.Error()), snapdUser, reason) + return apiLoginError(&apiError{ + Status: 401, + Message: err.Error(), + Kind: kind, + }, snapdUser) case nil: // continue } @@ -200,7 +205,11 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response { } st.Unlock() if err != nil { - return apiLoginError(InternalError("cannot persist authentication details: %v", err), snapdUser, seclog.ReasonInternal) + return apiLoginError(&apiError{ + Status: 500, + Message: fmt.Sprintf("cannot persist authentication details: %v", err), + Kind: client.ErrorKind(seclog.ReasonInternal), + }, snapdUser) } snapdUser.ID = int64(user.ID) diff --git a/daemon/daemon.go b/daemon/daemon.go index 2c27e096b91..ab1e30c4361 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -48,6 +48,7 @@ import ( "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/overlord/standby" "github.com/snapcore/snapd/overlord/state" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/snapdenv" "github.com/snapcore/snapd/store" "github.com/snapcore/snapd/systemd" @@ -159,7 +160,13 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if rspe := access.CheckAccess(c.d, r, ucred, user); rspe != nil { + rspe, checks, level := access.CheckAccess(c.d, r, ucred, user) + admin := isAdministrativeAccess(level, checks) + + if rspe != nil { + if admin { + logUnauthorizedAccess(c, r, ucred, user, access, level, rspe, checks) + } rspe.ServeHTTP(w, r) return } @@ -184,32 +191,142 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) { } rsp.ServeHTTP(w, r) + + // Log after the handler runs: authz_admin means the access gate passed, + // not that the API operation succeeded. + if admin { + logAdminActivity(c, r, ucred, user, access, level, checks) + } } -func traceSnapdAPI(c *Command, w http.ResponseWriter, r *http.Request) { - if osutil.GetenvBool("SNAPD_TRACE") { - loggedWithAction := false - if r.Method == "POST" && (r.Header.Get("Content-Type") == "application/json" || r.Header.Get("Content-Type") == "") { - r.Body = http.MaxBytesReader(w, r.Body, 3*1024*1024) // 3 MB limit - bodyBytes, err := io.ReadAll(r.Body) - if err != nil { - logger.Trace("endpoint-error", "body-read", err) - } - var data struct { - Action string `json:"action"` - } - if err := json.Unmarshal(bodyBytes, &data); err == nil { - if data.Action != "" { - loggedWithAction = true - logger.Trace("endpoint", "method", r.Method, "path", c.Path, "action", data.Action) - } - } - r.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) +func accessCheckerName(ac accessChecker) string { + switch ac.(type) { + case openAccess: + return "open" + case authenticatedAccess: + return "authenticated" + case rootAccess: + return "root" + case snapAccess: + return "snap" + case interfaceOpenAccess: + return "interface-open" + case interfaceAuthenticatedAccess: + return "interface-authenticated" + case interfaceProviderRootAccess: + return "interface-provider-root" + case interfaceRootAccess: + return "interface-root" + case byActionAccess: + return "by-action" + default: + return "unknown" + } +} + +func snapdUser(user *auth.UserState) seclog.SnapdUser { + if user == nil { + return seclog.SnapdUser{} + } + return seclog.SnapdUser{ + ID: int64(user.ID), + StoreUserName: user.Username, + StoreUserEmail: user.Email, + } +} + +// peerFromUcred converts the daemon-internal ucrednet view of a peer +// into the seclog.Peer used by security audit events. A nil ucred is +// represented using seclog's "unknown" sentinels. +func peerFromUcred(ucred *ucrednet) seclog.Peer { + if ucred == nil { + return seclog.Peer{ + UID: ^uint32(0), + PID: 0, } - if !loggedWithAction { - logger.Trace("endpoint", "method", r.Method, "path", c.Path) + } + return seclog.Peer{ + Socket: ucred.Socket, + UID: ucred.Uid, + PID: ucred.Pid, + } +} + +const maxJSONActionBodySize = 4 * 1024 * 1024 // matches byActionAccess maxBodySize + +func jsonContentType(r *http.Request) bool { + ct := r.Header.Get("Content-Type") + return ct == "" || ct == "application/json" +} + +// readBodyAndParseAction reads up to limit bytes from r.Body, restores r.Body, +// and returns the top-level JSON "action" field. +func readBodyAndParseAction(r *http.Request, limit int64) (string, error) { + bodyBytes, err := io.ReadAll(io.LimitReader(r.Body, limit)) + r.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + if err != nil { + return "", err + } + + var data struct { + Action string `json:"action"` + } + if err := json.Unmarshal(bodyBytes, &data); err != nil { + return "", nil + } + return data.Action, nil +} + +// requestAction extracts the top-level JSON "action" field from POST/PUT bodies. +// The request body is restored so handlers can read it afterward. +func requestAction(r *http.Request) string { + if r.Method != "POST" && r.Method != "PUT" { + return "" + } + if !jsonContentType(r) || r.Body == nil { + return "" + } + + action, _ := readBodyAndParseAction(r, maxJSONActionBodySize) + return action +} + +func endpointFromRequest(c *Command, r *http.Request, ac accessChecker, level accessLevel, action string) seclog.Endpoint { + return seclog.Endpoint{ + Method: r.Method, + Path: c.Path, + Action: action, + AccessChecker: accessCheckerName(ac), + AccessLevel: string(level), + } +} + +func logAdminActivity(c *Command, r *http.Request, ucred *ucrednet, user *auth.UserState, ac accessChecker, level accessLevel, checks seclog.AuthzChecks) { + seclog.LogAdminActivity(snapdUser(user), peerFromUcred(ucred), endpointFromRequest(c, r, ac, level, requestAction(r)), checks) +} + +func logUnauthorizedAccess(c *Command, r *http.Request, ucred *ucrednet, user *auth.UserState, ac accessChecker, level accessLevel, rspe *apiError, checks seclog.AuthzChecks) { + seclog.LogUnauthorizedAccess(snapdUser(user), peerFromUcred(ucred), endpointFromRequest(c, r, ac, level, requestAction(r)), checks, rspe.reason()) +} + +func traceSnapdAPI(c *Command, w http.ResponseWriter, r *http.Request) { + if !osutil.GetenvBool("SNAPD_TRACE") { + return + } + + loggedWithAction := false + if r.Method == "POST" && jsonContentType(r) && r.Body != nil { + action, err := readBodyAndParseAction(r, maxJSONActionBodySize) + if err != nil { + logger.Trace("endpoint-error", "body-read", err) + } else if action != "" { + loggedWithAction = true + logger.Trace("endpoint", "method", r.Method, "path", c.Path, "action", action) } } + if !loggedWithAction { + logger.Trace("endpoint", "method", r.Method, "path", c.Path) + } } type wrappedWriter struct { diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 423952da707..4b4b4c14e7a 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -20,6 +20,7 @@ package daemon import ( + "bytes" "context" "encoding/json" "fmt" @@ -29,6 +30,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "sync" "syscall" "testing" @@ -51,6 +53,8 @@ import ( "github.com/snapcore/snapd/overlord/snapstate/snapstatetest" "github.com/snapcore/snapd/overlord/standby" "github.com/snapcore/snapd/overlord/state" + "github.com/snapcore/snapd/seclog" + "github.com/snapcore/snapd/seclog/seclogtest" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" "github.com/snapcore/snapd/store" @@ -370,19 +374,134 @@ func (s *daemonSuite) TestFillsWarnings(c *check.C) { c.Check(rst.WarningTimestamp, check.NotNil) } -type accessCheckFunc func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError +type accessCheckFunc func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) -func (f accessCheckFunc) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { +func (f accessCheckFunc) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { return f(d, r, ucred, user) } +func (s *daemonSuite) TestAdminActivitySecurityLogging(c *check.C) { + seclogBuf := bytes.NewBuffer(nil) + seclog.Setup(seclogtest.MockSecurityLogger(seclogBuf)) + s.AddCleanup(func() { seclog.Setup(seclog.NewNopLogger()) }) + + d := s.newTestDaemon(c) + + cmd := &Command{ + d: d, + Path: "/v2/system-info", + } + cmd.GET = func(*Command, *http.Request, *auth.UserState) Response { + return SyncResponse(nil) + } + cmd.ReadAccess = openAccess{} + + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = fmt.Sprintf("pid=100;uid=42;socket=%s;", dirs.SnapdSocket) + rec := httptest.NewRecorder() + cmd.ServeHTTP(rec, req) + c.Check(rec.Code, check.Equals, 200) + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_admin") + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_fail") + + seclogBuf.Reset() + + cmd = &Command{ + d: d, + Path: "/v2/snaps", + } + cmd.POST = func(*Command, *http.Request, *auth.UserState) Response { + return SyncResponse(nil) + } + cmd.WriteAccess = authenticatedAccess{} + + req = httptest.NewRequest("POST", "/", strings.NewReader(`{"action":"install","snaps":["test-snapd-sh"]}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = fmt.Sprintf("pid=100;uid=0;socket=%s;", dirs.SnapdSocket) + rec = httptest.NewRecorder() + cmd.ServeHTTP(rec, req) + c.Check(rec.Code, check.Equals, 200) + c.Check(seclogBuf.String(), testutil.Contains, "authz_admin") + c.Check(seclogBuf.String(), testutil.Contains, "POST:/v2/snaps:install") + c.Check(seclogBuf.String(), testutil.Contains, `Action:"install"`) + c.Check(seclogBuf.String(), testutil.Contains, `AccessChecker:"authenticated"`) + c.Check(seclogBuf.String(), testutil.Contains, `AccessLevel:"authenticated"`) + + seclogBuf.Reset() + + cmd.WriteAccess = authenticatedAccess{} + req = httptest.NewRequest("POST", "/", strings.NewReader(`{"action":"install","snaps":["test-snapd-sh"]}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = fmt.Sprintf("pid=100;uid=42;socket=%s;", dirs.SnapdSocket) + rec = httptest.NewRecorder() + cmd.ServeHTTP(rec, req) + c.Check(rec.Code, check.Equals, 401) + c.Check(seclogBuf.String(), testutil.Contains, "authz_fail") + c.Check(seclogBuf.String(), testutil.Contains, "POST:/v2/snaps:install") + c.Check(seclogBuf.String(), testutil.Contains, `Action:"install"`) + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_admin") + + seclogBuf.Reset() + + cmd = &Command{ + d: d, + Path: "/v2/interfaces/requests", + } + cmd.POST = func(*Command, *http.Request, *auth.UserState) Response { + return SyncResponse(nil) + } + cmd.WriteAccess = byActionAccess{ + ByAction: map[string]accessChecker{ + "ask": openAccess{}, + }, + Default: rootAccess{}, + } + + req = httptest.NewRequest("POST", "/", strings.NewReader(`{"action":"ask"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = fmt.Sprintf("pid=100;uid=42;socket=%s;", dirs.SnapdSocket) + rec = httptest.NewRecorder() + cmd.ServeHTTP(rec, req) + c.Check(rec.Code, check.Equals, 200) + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_admin") + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_fail") + + seclogBuf.Reset() + + req = httptest.NewRequest("POST", "/", strings.NewReader(`not json`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = fmt.Sprintf("pid=100;uid=42;socket=%s;", dirs.SnapdSocket) + rec = httptest.NewRecorder() + cmd.ServeHTTP(rec, req) + c.Check(rec.Code, check.Equals, 400) + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_fail") + c.Check(seclogBuf.String(), check.Not(testutil.Contains), "authz_admin") +} + +func (s *daemonSuite) TestRequestAction(c *check.C) { + req := httptest.NewRequest("POST", "/", strings.NewReader(`{"action":"install","snaps":["x"]}`)) + req.Header.Set("Content-Type", "application/json") + c.Check(requestAction(req), check.Equals, "install") + + req = httptest.NewRequest("PUT", "/", strings.NewReader(`{"action":"refresh"}`)) + req.Header.Set("Content-Type", "application/json") + c.Check(requestAction(req), check.Equals, "refresh") + + req = httptest.NewRequest("GET", "/", nil) + c.Check(requestAction(req), check.Equals, "") + + req = httptest.NewRequest("POST", "/", strings.NewReader("not json")) + req.Header.Set("Content-Type", "application/json") + c.Check(requestAction(req), check.Equals, "") +} + func (s *daemonSuite) TestReadAccess(c *check.C) { cmd := &Command{d: s.newTestDaemon(c)} cmd.GET = func(*Command, *http.Request, *auth.UserState) Response { return SyncResponse(nil) } var accessCalled bool - cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { accessCalled = true c.Check(d, check.Equals, cmd.d) c.Check(r, check.NotNil) @@ -391,11 +510,11 @@ func (s *daemonSuite) TestReadAccess(c *check.C) { c.Check(ucred.Pid, check.Equals, int32(100)) c.Check(ucred.Socket, check.Equals, "xyz") c.Check(user, check.IsNil) - return nil + return nil, seclog.NewAuthzChecks(), accessLevelOpen }) - cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { c.Fail() - return Forbidden("") + return Forbidden(""), seclog.NewAuthzChecks(), accessLevelOpen }) req := httptest.NewRequest("GET", "/", nil) @@ -414,12 +533,12 @@ func (s *daemonSuite) TestWriteAccess(c *check.C) { cmd.POST = func(*Command, *http.Request, *auth.UserState) Response { return SyncResponse(nil) } - cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { c.Fail() - return Forbidden("") + return Forbidden(""), seclog.NewAuthzChecks(), accessLevelOpen }) var accessCalled bool - cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { accessCalled = true c.Check(d, check.Equals, cmd.d) c.Check(r, check.NotNil) @@ -428,7 +547,7 @@ func (s *daemonSuite) TestWriteAccess(c *check.C) { c.Check(ucred.Pid, check.Equals, int32(100)) c.Check(ucred.Socket, check.Equals, "xyz") c.Check(user, check.IsNil) - return nil + return nil, seclog.NewAuthzChecks(), accessLevelOpen }) req := httptest.NewRequest("PUT", "/", nil) @@ -467,12 +586,12 @@ func (s *daemonSuite) TestWriteAccessWithUser(c *check.C) { cmd.POST = func(*Command, *http.Request, *auth.UserState) Response { return SyncResponse(nil) } - cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.ReadAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { c.Fail() - return Forbidden("") + return Forbidden(""), seclog.NewAuthzChecks(), accessLevelOpen }) var accessCalled bool - cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) *apiError { + cmd.WriteAccess = accessCheckFunc(func(d *Daemon, r *http.Request, ucred *ucrednet, user *auth.UserState) (*apiError, seclog.AuthzChecks, accessLevel) { accessCalled = true c.Check(d, check.Equals, cmd.d) c.Check(r, check.NotNil) @@ -481,7 +600,7 @@ func (s *daemonSuite) TestWriteAccessWithUser(c *check.C) { c.Check(ucred.Pid, check.Equals, int32(100)) c.Check(ucred.Socket, check.Equals, "xyz") c.Check(user, check.DeepEquals, authUser) - return nil + return nil, seclog.NewAuthzChecks(), accessLevelOpen }) req := httptest.NewRequest("PUT", "/", nil) diff --git a/daemon/errors.go b/daemon/errors.go index 77a68afb62f..472dd127202 100644 --- a/daemon/errors.go +++ b/daemon/errors.go @@ -31,6 +31,7 @@ import ( "github.com/snapcore/snapd/overlord/fdestate" "github.com/snapcore/snapd/overlord/servicestate" "github.com/snapcore/snapd/overlord/snapstate" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/store" ) @@ -46,6 +47,14 @@ type apiError struct { Value errorValue } +func (ae *apiError) reason() seclog.Reason { + return seclog.Reason{ + Code: ae.Status, + Kind: string(ae.Kind), + Message: ae.Message, + } +} + func (ae *apiError) Error() string { kindOrStatus := "api" if ae.Kind != "" { diff --git a/daemon/errors_test.go b/daemon/errors_test.go index cc767756a19..e93e41cd6ae 100644 --- a/daemon/errors_test.go +++ b/daemon/errors_test.go @@ -30,6 +30,7 @@ import ( "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/daemon" "github.com/snapcore/snapd/overlord/snapstate" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/store" ) @@ -271,3 +272,12 @@ func (s *errorsSuite) TestForbidden(c *C) { Kind: client.ErrorKindLoginRequired, }) } + +func (s *errorsSuite) TestAPIErrorReason(c *C) { + c.Check(daemon.APIErrorReason(daemon.Unauthorized("access denied")), DeepEquals, seclog.Reason{ + Code: 401, Kind: string(client.ErrorKindLoginRequired), Message: "access denied", + }) + c.Check(daemon.APIErrorReason(daemon.InternalError("broken")), DeepEquals, seclog.Reason{ + Code: 500, Message: "broken", + }) +} diff --git a/daemon/export_access_test.go b/daemon/export_access_test.go index d743bfcb692..9797f538658 100644 --- a/daemon/export_access_test.go +++ b/daemon/export_access_test.go @@ -28,6 +28,8 @@ import ( type ( AccessChecker = accessChecker + AccessLevel = accessLevel + AccessOptions = accessOptions OpenAccess = openAccess @@ -43,8 +45,19 @@ type ( InterfaceAccessReqs = interfaceAccessReqs ) +const ( + AccessLevelOpen = accessLevelOpen + AccessLevelAuthenticated = accessLevelAuthenticated + AccessLevelRoot = accessLevelRoot + AccessLevelNotEvaluated = accessLevelNotEvaluated +) + var ( CheckAccess = checkAccess + CheckPrerequisites = checkPrerequisites + CheckAccessLevelAuthorization = checkAccessLevelAuthorization + IsAdministrativeAccess = isAdministrativeAccess + AccessCheckerName = accessCheckerName CheckPolkitActionImpl = checkPolkitActionImpl RequireInterfaceApiAccessImpl = requireInterfaceApiAccessImpl ) diff --git a/daemon/export_test.go b/daemon/export_test.go index 38bb953113b..ad3c556cc7b 100644 --- a/daemon/export_test.go +++ b/daemon/export_test.go @@ -39,6 +39,7 @@ import ( "github.com/snapcore/snapd/overlord/restart" "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/overlord/state" + "github.com/snapcore/snapd/seclog" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" ) @@ -369,6 +370,10 @@ type ( SnapInstruction = snapInstruction ) +func APIErrorReason(e *APIError) seclog.Reason { + return e.reason() +} + func (inst *snapInstruction) Dispatch() snapActionFunc { return inst.dispatch() } diff --git a/seclog/eventdata.go b/seclog/eventdata.go index dfa41f8af63..f65ba37a6f5 100644 --- a/seclog/eventdata.go +++ b/seclog/eventdata.go @@ -45,7 +45,7 @@ import ( // unknown is the placeholder for empty fields in descriptions. const unknown = "" -// Reason codes are stable identifiers for security audit events. +// Reason kind values are stable identifiers for security audit events. const ( ReasonInvalidCredentials = "invalid-credentials" ReasonTwoFactorRequired = "two-factor-required" @@ -58,17 +58,18 @@ const ( // Reason describes why a security event happened. The JSON tags match // the security audit specification field names. type Reason struct { - Code string `json:"code"` + Code int `json:"code"` + Kind string `json:"kind,omitempty"` Message string `json:"message"` } // String returns a colon-separated representation in the form -// ":". Fields that are unset use "" as a +// ":". Fields that are unset use "" as a // placeholder. func (r Reason) String() string { - code := unknown - if r.Code != "" { - code = r.Code + kind := unknown + if r.Kind != "" { + kind = r.Kind } message := unknown @@ -76,7 +77,7 @@ func (r Reason) String() string { message = r.Message } - return code + ":" + message + return kind + ":" + message } // SnapdUser represents the identity of a user for security log events. @@ -87,6 +88,116 @@ type SnapdUser struct { Expiration time.Time `json:"expiration"` } +// Peer describes the Unix-domain peer of an API request (Socket, UID, PID). +// +// Callers may signal "unknown" by setting UID to ^uint32(0) (the daemon's +// "nobody" sentinel) and/or PID to 0 (the daemon's "no process" sentinel). +type Peer struct { + Socket string `json:"socket"` + UID uint32 `json:"uid"` + PID int32 `json:"pid"` +} + +// peerNobody and peerNoProcess mirror the daemon's ucrednetNobody and +// ucrednetNoProcess sentinels. They are duplicated here to keep seclog +// free of snapd package imports. +const ( + peerNobody = ^uint32(0) + peerNoProcess = int32(0) +) + +// String returns a colon-separated representation in the form +// "::". Fields that are unset, or set to a documented +// "unknown" sentinel, use "" as a placeholder. +func (p Peer) String() string { + socket := unknown + if p.Socket != "" { + socket = p.Socket + } + + uid := unknown + // 0 is a valid UID (root); only the "nobody" sentinel is unknown. + if p.UID != peerNobody { + uid = fmt.Sprintf("%d", p.UID) + } + + pid := unknown + if p.PID != peerNoProcess { + pid = fmt.Sprintf("%d", p.PID) + } + + return socket + ":" + uid + ":" + pid +} + +// Endpoint describes an API endpoint involved in an authorization event. +type Endpoint struct { + Method string `json:"method"` + Path string `json:"path"` + Action string `json:"action"` + AccessChecker string `json:"access-checker"` + AccessLevel string `json:"access-level"` +} + +// String returns a colon-separated representation in the form +// "::". +func (e Endpoint) String() string { + s := e.Method + ":" + e.Path + if e.Action != "" { + s += ":" + e.Action + } + return s +} + +// AuthzCheck represents the outcome of a single authorization check. +type AuthzCheck string + +const ( + AuthzNotApplicable AuthzCheck = "not-applicable" + AuthzNotReached AuthzCheck = "not-reached" + AuthzFail AuthzCheck = "fail" + AuthzPass AuthzCheck = "pass" +) + +// AuthzChecks captures the outcome of each authorization stage evaluated +// during an access check. Each field records whether that stage passed, +// failed, or was not applicable to the request. +type AuthzChecks struct { + AccessOptions AuthzCheck `json:"access-options"` + PeerCreds AuthzCheck `json:"peer-credentials"` + Socket AuthzCheck `json:"socket"` + Interface AuthzCheck `json:"interface-requirements"` + OpenAccess AuthzCheck `json:"open-access"` + UserAuth AuthzCheck `json:"user-authentication"` + Root AuthzCheck `json:"root"` + Polkit AuthzCheck `json:"polkit"` +} + +// AnyPerformed reports whether any authorization check was evaluated. +func (a AuthzChecks) AnyPerformed() bool { + return a.AccessOptions != AuthzNotApplicable || + a.PeerCreds != AuthzNotApplicable || + a.Socket != AuthzNotApplicable || + a.Interface != AuthzNotApplicable || + a.OpenAccess != AuthzNotApplicable || + a.UserAuth != AuthzNotApplicable || + a.Root != AuthzNotApplicable || + a.Polkit != AuthzNotApplicable +} + +// NewAuthzChecks returns an AuthzChecks with all fields set to [AuthzNotApplicable]. +func NewAuthzChecks() AuthzChecks { + return AuthzChecks{ + AccessOptions: AuthzNotApplicable, + PeerCreds: AuthzNotApplicable, + Socket: AuthzNotApplicable, + Interface: AuthzNotApplicable, + OpenAccess: AuthzNotApplicable, + UserAuth: AuthzNotApplicable, + Root: AuthzNotApplicable, + Polkit: AuthzNotApplicable, + } +} + // String returns a colon-separated description of the user in the form // "::". Fields that are unset use // "" as a placeholder; a zero ID is considered unset. diff --git a/seclog/eventdata_test.go b/seclog/eventdata_test.go index 903a74be4f7..45c10e4c4fc 100644 --- a/seclog/eventdata_test.go +++ b/seclog/eventdata_test.go @@ -26,16 +26,16 @@ import ( ) func (s *SecLogSuite) TestReasonString(c *C) { - // Both fields set. + // Kind and message set. c.Check(seclog.Reason{ - Code: seclog.ReasonInvalidCredentials, Message: "bad password", + Code: 401, Kind: seclog.ReasonInvalidCredentials, Message: "bad password", }.String(), Equals, "invalid-credentials:bad password") - // Both fields empty — all "". + // All fields empty — all "". c.Check(seclog.Reason{}.String(), Equals, ":") - // Only code set. - c.Check(seclog.Reason{Code: seclog.ReasonInternal}.String(), Equals, "internal:") + // Only kind set. + c.Check(seclog.Reason{Code: 500, Kind: seclog.ReasonInternal}.String(), Equals, "internal:") // Only message set. c.Check(seclog.Reason{Message: "something broke"}.String(), Equals, ":something broke") @@ -59,3 +59,24 @@ func (s *SecLogSuite) TestSnapdUserString(c *C) { // Only username set. c.Check(seclog.SnapdUser{StoreUserName: "root"}.String(), Equals, "::root") } + +func (s *SecLogSuite) TestPeerString(c *C) { + c.Check(seclog.Peer{ + Socket: "/run/snapd.socket", UID: 0, PID: 4242, + }.String(), Equals, "/run/snapd.socket:0:4242") + + // Zero UID is root; only the nobody sentinel is unknown. + c.Check(seclog.Peer{}.String(), Equals, ":0:") + + c.Check(seclog.Peer{Socket: "/run/snapd.socket"}.String(), Equals, "/run/snapd.socket:0:") + + c.Check(seclog.Peer{UID: ^uint32(0)}.String(), Equals, "::") +} + +func (s *SecLogSuite) TestAuthzChecksAnyPerformed(c *C) { + c.Check(seclog.NewAuthzChecks().AnyPerformed(), Equals, false) + + checks := seclog.NewAuthzChecks() + checks.PeerCreds = seclog.AuthzPass + c.Check(checks.AnyPerformed(), Equals, true) +} diff --git a/seclog/seclog.go b/seclog/seclog.go index eaabe73f0aa..6d98a4a6ee8 100644 --- a/seclog/seclog.go +++ b/seclog/seclog.go @@ -189,3 +189,38 @@ func LogUserRemoved(user SnapdUser) { Attr{Key: "user", Value: user}, ) } + +// LogAdminActivity logs an administrative API access event using the +// global security logger. It is emitted when authorization succeeds (the +// access gate passed), not when the API operation or handler succeeds. +func LogAdminActivity(user SnapdUser, peer Peer, endpoint Endpoint, checks AuthzChecks) { + lock.Lock() + defer lock.Unlock() + + globalLogger.LogEvent( + Event{Category: "AUTHZ", Name: "authz_admin", Level: LevelInfo}, + fmt.Sprintf("User %s (peer %s) accessed %s", user.String(), peer.String(), endpoint.String()), + Attr{Key: "user", Value: user}, + Attr{Key: "peer", Value: peer}, + Attr{Key: "endpoint", Value: endpoint}, + Attr{Key: "authz_checks", Value: checks}, + ) +} + +// LogUnauthorizedAccess logs an unauthorized access attempt using the +// global security logger. +func LogUnauthorizedAccess(user SnapdUser, peer Peer, endpoint Endpoint, checks AuthzChecks, reason Reason) { + lock.Lock() + defer lock.Unlock() + + globalLogger.LogEvent( + Event{Category: "AUTHZ", Name: "authz_fail", Level: LevelCritical}, + fmt.Sprintf("Peer %s: user %s attempted to access %s without entitlement: %s", + peer.String(), user.String(), endpoint.String(), reason.Message), + Attr{Key: "user", Value: user}, + Attr{Key: "peer", Value: peer}, + Attr{Key: "endpoint", Value: endpoint}, + Attr{Key: "authz_checks", Value: checks}, + Attr{Key: "error", Value: reason}, + ) +} diff --git a/seclog/seclog_test.go b/seclog/seclog_test.go index 3be818ee792..b4f71f04493 100644 --- a/seclog/seclog_test.go +++ b/seclog/seclog_test.go @@ -122,7 +122,7 @@ func (s *SecLogSuite) TestLogLoginFailure(c *C) { StoreUserEmail: "user@example.com", StoreUserName: "jdoe", } - seclog.LogLoginFailure(user, seclog.Reason{Code: seclog.ReasonInvalidCredentials, Message: "invalid credentials"}) + seclog.LogLoginFailure(user, seclog.Reason{Code: 401, Kind: seclog.ReasonInvalidCredentials, Message: "invalid credentials"}) c.Check(s.buf.String(), testutil.Contains, "authn_login_failure") c.Check(s.buf.String(), testutil.Contains, "user@example.com") @@ -218,3 +218,50 @@ func (s *SecLogSuite) TestLogUserRemoved(c *C) { c.Check(s.buf.String(), testutil.Contains, "jdoe") c.Check(s.buf.String(), testutil.Contains, "jdoe@test.com") } + +// TestLogAdminActivity verifies that LogAdminActivity emits the expected event and attributes. +func (s *SecLogSuite) TestLogAdminActivity(c *C) { + user := seclog.SnapdUser{ID: 1, StoreUserEmail: "admin@example.com", StoreUserName: "admin"} + peer := seclog.Peer{Socket: "/run/snapd.socket", UID: 0, PID: 4242} + endpoint := seclog.Endpoint{ + Method: "POST", + Path: "/v2/snaps", + Action: "install", + AccessChecker: "authenticated", + AccessLevel: "authenticated", + } + checks := seclog.NewAuthzChecks() + + seclog.LogAdminActivity(user, peer, endpoint, checks) + + c.Check(s.buf.String(), testutil.Contains, "authz_admin") + c.Check(s.buf.String(), testutil.Contains, "admin@example.com") + c.Check(s.buf.String(), testutil.Contains, "POST:/v2/snaps:install") + c.Check(s.buf.String(), testutil.Contains, "/run/snapd.socket") + c.Check(s.buf.String(), testutil.Contains, "4242") + c.Check(s.buf.String(), testutil.Contains, `AccessChecker:"authenticated"`) + c.Check(s.buf.String(), testutil.Contains, `AccessLevel:"authenticated"`) + c.Check(s.buf.String(), testutil.Contains, `[user=`) + c.Check(s.buf.String(), Not(testutil.Contains), `snapd_user`) + c.Check(s.buf.String(), Not(testutil.Contains), `Address:`) + c.Check(s.buf.String(), Not(testutil.Contains), `Port:`) +} + +// TestLogUnauthorizedAccess verifies that LogUnauthorizedAccess emits the expected event, peer, and reason. +func (s *SecLogSuite) TestLogUnauthorizedAccess(c *C) { + user := seclog.SnapdUser{ID: 1, StoreUserEmail: "hacker@example.com", StoreUserName: "hacker"} + peer := seclog.Peer{Socket: "/run/snapd.socket", UID: 1000, PID: 12345} + endpoint := seclog.Endpoint{Method: "DELETE", Path: "/v2/snaps/core"} + checks := seclog.NewAuthzChecks() + reason := seclog.Reason{Code: 401, Kind: seclog.ReasonInvalidCredentials, Message: "no permission"} + + seclog.LogUnauthorizedAccess(user, peer, endpoint, checks, reason) + + c.Check(s.buf.String(), testutil.Contains, "authz_fail") + c.Check(s.buf.String(), testutil.Contains, "hacker@example.com") + c.Check(s.buf.String(), testutil.Contains, "DELETE:/v2/snaps/core") + c.Check(s.buf.String(), testutil.Contains, "12345") + c.Check(s.buf.String(), testutil.Contains, `[user=`) + c.Check(s.buf.String(), Not(testutil.Contains), `snapd_user`) + c.Check(s.buf.String(), testutil.Contains, seclog.ReasonInvalidCredentials) +} diff --git a/seclog/slog.go b/seclog/slog.go index 5a191112dbe..a5282c5704e 100644 --- a/seclog/slog.go +++ b/seclog/slog.go @@ -166,10 +166,14 @@ func (h *errorAwareHandler) WithGroup(name string) slog.Handler { // LogValue implements [slog.LogValuer], allowing Reason to be // used directly as a structured log attribute value. func (r Reason) LogValue() slog.Value { - return slog.GroupValue( - slog.String("code", r.Code), - slog.String("message", r.Message), - ) + attrs := []slog.Attr{ + slog.Int("code", r.Code), + } + if r.Kind != "" { + attrs = append(attrs, slog.String("kind", r.Kind)) + } + attrs = append(attrs, slog.String("message", r.Message)) + return slog.GroupValue(attrs...) } // LogValue implements [slog.LogValuer], allowing SnapdUser to be @@ -186,3 +190,40 @@ func (u SnapdUser) LogValue() slog.Value { slog.String("expiration", expiration), ) } + +// LogValue implements [slog.LogValuer], allowing Endpoint to be +// used directly as a structured log attribute value. +func (e Endpoint) LogValue() slog.Value { + return slog.GroupValue( + slog.String("method", e.Method), + slog.String("path", e.Path), + slog.String("action", e.Action), + slog.String("access-checker", e.AccessChecker), + slog.String("access-level", e.AccessLevel), + ) +} + +// LogValue implements [slog.LogValuer], allowing Peer to be used +// directly as a structured log attribute value. +func (p Peer) LogValue() slog.Value { + return slog.GroupValue( + slog.String("socket", p.Socket), + slog.Int64("uid", int64(p.UID)), + slog.Int64("pid", int64(p.PID)), + ) +} + +// LogValue implements [slog.LogValuer], allowing AuthzChecks to be +// used directly as a structured log attribute value. +func (a AuthzChecks) LogValue() slog.Value { + return slog.GroupValue( + slog.String("access-options", string(a.AccessOptions)), + slog.String("peer-credentials", string(a.PeerCreds)), + slog.String("socket", string(a.Socket)), + slog.String("interface-requirements", string(a.Interface)), + slog.String("open-access", string(a.OpenAccess)), + slog.String("user-authentication", string(a.UserAuth)), + slog.String("root", string(a.Root)), + slog.String("polkit", string(a.Polkit)), + ) +} diff --git a/seclog/slog_test.go b/seclog/slog_test.go index 7562eccbacb..399fac6a882 100644 --- a/seclog/slog_test.go +++ b/seclog/slog_test.go @@ -73,6 +73,12 @@ type baseAttrs struct { Category string `json:"category"` } +// record is used for basic log event tests. +type record struct { + baseAttrs + Event string `json:"event"` +} + // orderedKeys extracts the top-level JSON object keys in order. func orderedKeys(data []byte) ([]string, error) { decoder := json.NewDecoder(bytes.NewReader(data)) @@ -106,12 +112,6 @@ func orderedKeys(data []byte) ([]string, error) { func (s *SlogSuite) TestLogEvent(c *C) { logger := seclog.NewSlogLogger(s.buf, s.appID, seclog.LevelInfo) - c.Assert(logger, NotNil) - - type record struct { - baseAttrs - Event string `json:"event"` - } logger.LogEvent( seclog.Event{Category: "TEST", Name: "test_event", Level: seclog.LevelInfo}, @@ -152,7 +152,8 @@ func (s *SlogSuite) TestLogEventWithAttrs(c *C) { Expiration string `json:"expiration"` } `json:"user"` Error struct { - Code string `json:"code"` + Code int `json:"code"` + Kind string `json:"kind"` Message string `json:"message"` } `json:"error"` } @@ -162,7 +163,7 @@ func (s *SlogSuite) TestLogEventWithAttrs(c *C) { StoreUserEmail: "user@gmail.com", StoreUserName: "jdoe", } - reason := seclog.Reason{Code: seclog.ReasonInvalidCredentials, Message: "invalid credentials"} + reason := seclog.Reason{Code: 401, Kind: seclog.ReasonInvalidCredentials, Message: "invalid credentials"} logger.LogEvent( seclog.Event{Category: "TEST", Name: "test_event", Level: seclog.LevelWarn}, fmt.Sprintf("User %s caused an issue: %s", user.String(), reason.String()), @@ -187,7 +188,8 @@ func (s *SlogSuite) TestLogEventWithAttrs(c *C) { c.Check(obtained.User.StoreUserName, Equals, "jdoe") c.Check(obtained.User.Expiration, Equals, "never") // Reason is a plain struct — verify JSON marshaling via slog.Any - c.Check(obtained.Error.Code, Equals, seclog.ReasonInvalidCredentials) + c.Check(obtained.Error.Code, Equals, 401) + c.Check(obtained.Error.Kind, Equals, seclog.ReasonInvalidCredentials) c.Check(obtained.Error.Message, Equals, "invalid credentials") // verify key order for human readability diff --git a/tests/main/security-logging/task.yaml b/tests/main/security-logging/task.yaml index 48fcb616146..ef1382aa465 100644 --- a/tests/main/security-logging/task.yaml +++ b/tests/main/security-logging/task.yaml @@ -8,7 +8,9 @@ details: | an "authn_login_success" event in the audit log. It also checks that user lifecycle events (user_created, user_updated, user_removed) are recorded when users are created via login, updated on re-login, and - removed via logout. + removed via logout. Finally it verifies that authorization events are + emitted for successful and rejected admin API calls (authz_admin and + authz_fail) when the endpoint requires access above open. # Amazon Linux 2: systemd-journald-audit.socket is not available systems: [ -amazon-linux-2-64 ] @@ -44,6 +46,7 @@ restore: | fi snap logout || true + snap remove --purge test-snapd-sh > /dev/null 2>&1 || true execute: | # Pin snapd journal search position to the current journal position @@ -73,7 +76,29 @@ execute: | echo '{"email":"someemail@testing.com","password":"wrong-password"}' | \ snap debug api -X POST -H 'Content-Type: application/json' /v2/login || true - journal_match '"level":"WARN","description":"User :someemail@testing.com: login failure: invalid-credentials:invalid credentials","app_id":"canonical.snapd.snapd","type":"security","category":"AUTHN","event":"authn_login_failure","user":{"snapd-user-id":0,"store-user-name":"","store-user-email":"someemail@testing.com","expiration":"never"},"error":{"code":"invalid-credentials","message":"invalid credentials"}' + journal_match '"level":"WARN","description":"User :someemail@testing.com: login failure: invalid-credentials:invalid credentials","app_id":"canonical.snapd.snapd","type":"security","category":"AUTHN","event":"authn_login_failure","user":{"snapd-user-id":0,"store-user-name":"","store-user-email":"someemail@testing.com","expiration":"never"},"error":{"code":401,"kind":"invalid-credentials","message":"invalid credentials"}' + + echo "Checking that an authorized admin API call produces an authz_admin event" + journal_pin + # POST /v2/snaps as root over snapd.socket exercises authenticatedAccess + # and is authorized via the root check. The body is well-formed JSON so + # the access check is reached; the change itself may fail asynchronously + # but that does not affect the audit event we want to test. + echo '{"action":"install","snaps":["test-snapd-sh"]}' | \ + snap debug api -X POST -H 'Content-Type: application/json' /v2/snaps > /dev/null 2>&1 || true + + journal_match '"level":"INFO","description":"User :: \(peer /run/snapd\.socket:0:[0-9]+\) accessed POST:/v2/snaps:install","app_id":"canonical.snapd.snapd","type":"security","category":"AUTHZ","event":"authz_admin","user":\{"snapd-user-id":0,"store-user-name":"","store-user-email":"","expiration":"never"\},"peer":\{"socket":"/run/snapd\.socket","uid":0,"pid":[0-9]+\},"endpoint":\{"method":"POST","path":"/v2/snaps","action":"install","access-checker":"authenticated","access-level":"authenticated"\},"authz_checks":\{"access-options":"pass","peer-credentials":"pass","socket":"pass","interface-requirements":"not-applicable","open-access":"not-applicable","user-authentication":"fail","root":"pass","polkit":"not-reached"\}' + + echo "Checking that an unauthorized admin API call produces an authz_fail event" + journal_pin + # POST /v2/snaps as an unprivileged user over snapd.socket exercises + # authenticatedAccess without macaroon, root, or polkit authorization. + runuser -u test -- curl -sS --unix-socket /run/snapd.socket \ + -X POST -H 'Content-Type: application/json' \ + -d '{"action":"install","snaps":["test-snapd-sh"]}' \ + http://localhost/v2/snaps > /dev/null || true + + journal_match '"level":"CRITICAL","description":"Peer /run/snapd\.socket:[1-9][0-9]*:[0-9]+: user :: attempted to access POST:/v2/snaps:install without entitlement: access denied","app_id":"canonical.snapd.snapd","type":"security","category":"AUTHZ","event":"authz_fail","user":\{"snapd-user-id":0,"store-user-name":"","store-user-email":"","expiration":"never"\},"peer":\{"socket":"/run/snapd\.socket","uid":[1-9][0-9]*,"pid":[0-9]+\},"endpoint":\{"method":"POST","path":"/v2/snaps","action":"install","access-checker":"authenticated","access-level":"authenticated"\},"authz_checks":\{"access-options":"pass","peer-credentials":"pass","socket":"pass","interface-requirements":"not-applicable","open-access":"not-applicable","user-authentication":"fail","root":"fail","polkit":"not-applicable"\},"error":\{"code":401,"kind":"login-required","message":"access denied"\}' # SPREAD_STORE_USER and SPREAD_STORE_PASSWORD are only available when running off master