diff --git a/Documentation/rfc/api_security.md b/Documentation/security_api.md similarity index 64% rename from Documentation/rfc/api_security.md rename to Documentation/security_api.md index 82d04de4631..7b22b2057d0 100644 --- a/Documentation/rfc/api_security.md +++ b/Documentation/security_api.md @@ -10,37 +10,32 @@ There are three types of resources in etcd ### Permission Resources #### Users -A user is an identity to be authenticated. Each user can have multiple roles. The user has a capability on the resource if one of the roles has that capability. +A user is an identity to be authenticated. Each user can have multiple roles. The user has a capability (such as reading or writing) on the resource if one of the roles has that capability. -The special static `root` user has a ROOT role. (Caps for visual aid throughout) +A user named `root` is required before security can be enabled, and it always has the ROOT role. The ROOT role can be granted to multiple users, but `root` is required for recovery purposes. -#### Role -Each role has exact one associated Permission List. An permission list exists for each permission on key-value resources. A role with `manage` permission of a key-value resource can grant/revoke capability of that key-value to other roles. +#### Roles +Each role has exact one associated Permission List. An permission list exists for each permission on key-value resources. + +The special static ROOT (named `root`) role has a full permissions on all key-value resources, the permission to manage user resources and settings resources. Only the ROOT role has the permission to manage user resources and modify settings resources. The ROOT role is built-in and does not need to be created. -The special static ROOT role has a full permissions on all key-value resources, the permission to manage user resources and settings resources. Only the ROOT role has the permission to manage user resources and modify settings resources. +There is also a special GUEST role, named 'guest'. These are the permissions given to unauthenticated requests to etcd. This role will be created when security is enabled, unless it already exists, and by default allows access to the full keyspace due to backward compatability. (etcd did not previously authenticate any actions.). This role can be modified by a ROOT role holder at any time. #### Permissions -There are two types of permissions, `read` and `write`. All management stems from the ROOT user. +There are two types of permissions, `read` and `write`. All management and settings require the ROOT role. -A Permission List is a list of allowed patterns for that particular permission (read or write). Only ALLOW prefixes (incidentally, this is what Amazon S3 does). DENY becomes more complicated and is TBD. +A Permission List is a list of allowed patterns for that particular permission (read or write). Only ALLOW prefixes are supported. DENY becomes more complicated and is TBD. ### Key-Value Resources A key-value resource is a key-value pairs in the store. Given a list of matching patterns, permission for any given key in a request is granted if any of the patterns in the list match. -The glob match rules are as follows: - -* `*` and `\` are special characters, representing "greedy match" and "escape" respectively. - * As a corrolary, `\*` and `\\` are the corresponding literal matches. -* All other bytes match exactly their bytes, starting always from the *first byte*. (For regex fans, `re.match` in Python) -* Examples: - * `/foo` matches only the single key/directory of `/foo` - * `/foo*` matches the prefix `/foo`, and all subdirectories/keys - * `/foo/*/bar` matches the keys bar in any (recursive) subdirectory of `/foo`. +Only prefixes or exact keys are supported. A prefix permission string ends in `*`. +A permission on `/foo` is for that exact key or directory, not its children or recursively. `/foo*` is a prefix that matches `/foo` recursively, and all keys thereunder, and keys with that prefix (eg. `/foobar`. Contrast to the prefix `/foo/*`). `*` alone is permission on the full keyspace. ### Settings Resources -Specific settings for the cluster as a whole. This can include adding and removing cluster members, enabling or disabling security, replacing certificates, and any other dynamic configuration by the administrator. +Specific settings for the cluster as a whole. This can include adding and removing cluster members, enabling or disabling security, replacing certificates, and any other dynamic configuration by the administrator (holder of the ROOT role). ## v2 Auth @@ -123,7 +118,7 @@ GET/HEAD /v2/security/users/alice "roles" : ["fleet", "etcd"] } -**Create A User** +**Create Or Update A User** A user can be created with initial roles, if filled in. However, no roles are required; only the username and password fields @@ -132,7 +127,9 @@ PUT /v2/security/users/charlie Sent Headers: Authorization: Basic Put Body: - JSON struct, above, matching the appropriate name and with starting roles. + JSON struct, above, matching the appropriate name + * Starting password and roles when creating. + * Grant/Revoke/Password filled in when updating (to grant roles, revoke roles, or change the password). Possible Status Codes: 200 OK 403 Forbidden @@ -152,68 +149,21 @@ DELETE /v2/security/users/charlie 200 Headers: 200 Body: (empty) -**Grant a Role(s) to a User** - -PUT /v2/security/users/charlie/grant - - Sent Headers: - Authorization: Basic - Put Body: - { "grantRoles" : ["fleet", "etcd"], (extra JSON data for checking OK) } - Possible Status Codes: - 200 OK - 403 Forbidden - 404 Not Found - 409 Conflict - 200 Body: - JSON user struct, updated. "roles" now contains the grants, and "grantRoles" is empty. If there is an error in the set of roles to be added, for example, a non-existent role, then 409 is returned, with an error JSON stating why. - -**Revoke a Role(s) from a User** - -PUT /v2/security/users/charlie/revoke - - Sent Headers: - Authorization: Basic - Put Body: - { "revokeRoles" : ["fleet"], (extra JSON data for checking OK) } - Possible Status Codes: - 200 OK - 403 Forbidden - 404 Not Found - 409 Conflict - 200 Body: - JSON user struct, updated. "roles" now doesn't contain the roles, and "revokeRoles" is empty. If there is an error in the set of roles to be removed, for example, a non-existent role, then 409 is returned, with an error JSON stating why. - -**Change password** - -PUT /v2/security/users/charlie/password - - Sent Headers: - Authorization: Basic - Put Body: - {"user": "charlie", "password": "newCharliePassword"} - Possible Status Codes: - 200 OK - 403 Forbidden - 404 Not Found - 200 Body: - JSON user struct, updated - #### Roles A full role structure may look like this. A Permission List structure is used for the "permissions", "grant", and "revoke" keys. ``` { "role" : "fleet", - "permissions" : { - "kv" { - "read" : [ "/fleet/" ], - "write": [ "/fleet/" ], - } + "permissions" : { + "kv" { + "read" : [ "/fleet/" ], + "write": [ "/fleet/" ], } - "grant" : {"kv": {...}}, - "revoke": {"kv": {...}}, - "members" : ["alice", "bob"], + } + "grant" : {"kv": {...}}, + "revoke": {"kv": {...}}, + "members" : ["alice", "bob"] } ``` @@ -258,14 +208,16 @@ GET/HEAD /v2/security/roles/fleet }, } -**Create A Role** +**Create Or Update A Role** PUT /v2/security/roles/rocket Sent Headers: Authorization: Basic Put Body: - Initial desired JSON state, complete with prefixes and + Initial desired JSON state, including the role name for verification and: + * Starting permission set if creating + * Granted/Revoked permission set if updating Possible Status Codes: 201 Created 403 Forbidden @@ -287,35 +239,6 @@ DELETE /v2/security/roles/rocket 200 Headers: 200 Body: (empty) -**Update a Role’s Permission List for {read,write}ing** - -PUT /v2/security/roles/rocket/update - - Sent Headers: - Authorization: Basic - Put Body: - { - "role" : "rocket", - "grant": { - "kv": { - "read" : [ "/rocket/"] - } - }, - "revoke": { - "kv": { - "read" : [ "/fleet/"] - } - } - } - Possible Status Codes: - 200 OK - 403 Forbidden - 404 Not Found - 200 Headers: - ETag: "roles/rocket:" - 200 Body: - JSON state of the role, with change containing empty lists and the deltas applied appropriately. - #### Enable and Disable Security @@ -334,16 +257,10 @@ GET /v2/security/enable **Enable security** -Enabling security means setting an explicit `root` user and password. ROOTs roles are irrelevant, as this user has full permissions. - PUT /v2/security/enable Sent Headers: - Put Body: - { - "user" : "root", - "password": "toor" - } + Put Body: (empty) Possible Status Codes: 200 OK 400 Bad Request (if not a root user) @@ -378,15 +295,13 @@ PUT /v2/security/enable ### Change root's password ``` -PUT /v2/security/users/root/password +PUT /v2/security/users/root Headers: Authorization: Basic Put Body: {"user" : "root", "password": "betterRootPW!"} ``` -//TODO(barakmich): How do you recover the root password? *This* may require a flag and a restart. `--disable-permissions` - ### Create Roles for the Applications Create the rocket role fully specified: @@ -401,10 +316,10 @@ PUT /v2/security/roles/rocket "permissions" : { "kv": { "read": [ - "/rocket/" + "/rocket/*" ], "write": [ - "/rocket/" + "/rocket/*" ] } } @@ -430,7 +345,7 @@ Well, we finally figured out where we want fleet to live. Let's fix it. ``` -PUT /v2/security/roles/fleet/update +PUT /v2/security/roles/fleet Headers: Authorization: Basic Put Body: @@ -439,7 +354,8 @@ PUT /v2/security/roles/fleet/update "grant" : { "kv" : { "read": [ - "/fleet/" + "/rocket/fleet", + "/fleet/*" ] } } @@ -471,7 +387,7 @@ PUT /v2/security/users/fleetuser Likewise, let's explicitly grant fleetuser access. ``` -PUT /v2/security/users/fleetuser/grant +PUT /v2/security/users/fleetuser Headers: Authorization: Basic Body: diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 3a1fe87c9c0..71570fbc75e 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -129,7 +129,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } // The path must be valid at this point (we've parsed the request successfully). - if !hasKeyPrefixAccess(h.sec, r, r.URL.Path[len(keysPrefix):]) { + if !hasKeyPrefixAccess(h.sec, r, r.URL.Path[len(keysPrefix):], rr.Recursive) { writeNoAuth(w) return } diff --git a/etcdserver/etcdhttp/client_security.go b/etcdserver/etcdhttp/client_security.go index 72dbfb24cae..f1e3aaf6e62 100644 --- a/etcdserver/etcdhttp/client_security.go +++ b/etcdserver/etcdhttp/client_security.go @@ -41,7 +41,7 @@ func hasWriteRootAccess(sec *security.Store, r *http.Request) bool { func hasRootAccess(sec *security.Store, r *http.Request) bool { if sec == nil { - // No store means no security avaliable, eg, tests. + // No store means no security available, eg, tests. return true } if !sec.SecurityEnabled() { @@ -51,24 +51,27 @@ func hasRootAccess(sec *security.Store, r *http.Request) bool { if !ok { return false } - if username != "root" { - log.Printf("security: Attempting to use user %s for resource that requires root.", username) - return false - } - root, err := sec.GetUser("root") + rootUser, err := sec.GetUser(username) if err != nil { return false } - ok = root.CheckPassword(password) + ok = rootUser.CheckPassword(password) if !ok { log.Printf("security: Wrong password for user %s", username) + return false + } + for _, role := range rootUser.Roles { + if role == security.RootRoleName { + return true + } } - return ok + log.Printf("security: User %s does not have the %s role for resource %s.", username, security.RootRoleName, r.URL.Path) + return false } -func hasKeyPrefixAccess(sec *security.Store, r *http.Request, key string) bool { +func hasKeyPrefixAccess(sec *security.Store, r *http.Request, key string, recursive bool) bool { if sec == nil { - // No store means no security avaliable, eg, tests. + // No store means no security available, eg, tests. return true } if !sec.SecurityEnabled() { @@ -76,7 +79,7 @@ func hasKeyPrefixAccess(sec *security.Store, r *http.Request, key string) bool { } username, password, ok := netutil.BasicAuth(r) if !ok { - return false + return hasGuestAccess(sec, r, key) } user, err := sec.GetUser(username) if err != nil { @@ -88,23 +91,34 @@ func hasKeyPrefixAccess(sec *security.Store, r *http.Request, key string) bool { log.Printf("security: Incorrect password for user: %s.", username) return false } - if user.User == "root" { - return true - } writeAccess := r.Method != "GET" && r.Method != "HEAD" for _, roleName := range user.Roles { role, err := sec.GetRole(roleName) if err != nil { continue } - if role.HasKeyAccess(key, writeAccess) { - return true + if recursive { + return role.HasRecursiveAccess(key, writeAccess) } + return role.HasKeyAccess(key, writeAccess) } log.Printf("security: Invalid access for user %s on key %s.", username, key) return false } +func hasGuestAccess(sec *security.Store, r *http.Request, key string) bool { + writeAccess := r.Method != "GET" && r.Method != "HEAD" + role, err := sec.GetRole(security.GuestRoleName) + if err != nil { + return false + } + if role.HasKeyAccess(key, writeAccess) { + return true + } + log.Printf("security: Invalid access for unauthenticated user on resource %s.", key) + return false +} + func writeNoAuth(w http.ResponseWriter) { herr := httptypes.NewHTTPError(http.StatusUnauthorized, "Insufficient credentials") herr.WriteTo(w) @@ -136,6 +150,9 @@ func (sh *securityHandler) baseRoles(w http.ResponseWriter, r *http.Request) { writeError(w, err) return } + if roles == nil { + roles = make([]string, 0) + } rolesCollections.Roles = roles err = json.NewEncoder(w).Encode(rolesCollections) @@ -149,7 +166,13 @@ func (sh *securityHandler) handleRoles(w http.ResponseWriter, r *http.Request) { // Split "/roles/rolename/command". // First item is an empty string, second is "roles" pieces := strings.Split(subpath, "/") + if len(pieces) == 2 { + sh.baseRoles(w, r) + return + } if len(pieces) != 3 { + writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Invalid path")) + return } sh.forRole(w, r, pieces[2]) } @@ -186,15 +209,19 @@ func (sh *securityHandler) forRole(w http.ResponseWriter, r *http.Request, role return } if in.Role != role { - writeError(w, httptypes.NewHTTPError(400, "Role JSON name does not match the name in the URL")) + writeError(w, httptypes.NewHTTPError(401, "Role JSON name does not match the name in the URL")) return } - newrole, err := sh.sec.CreateOrUpdateRole(in) + newrole, created, err := sh.sec.CreateOrUpdateRole(in) if err != nil { writeError(w, err) return } - w.WriteHeader(http.StatusCreated) + if created { + w.WriteHeader(http.StatusCreated) + } else { + w.WriteHeader(http.StatusOK) + } err = json.NewEncoder(w).Encode(newrole) if err != nil { log.Println("etcdhttp: forRole error encoding on", r.URL) @@ -228,6 +255,9 @@ func (sh *securityHandler) baseUsers(w http.ResponseWriter, r *http.Request) { writeError(w, err) return } + if users == nil { + users = make([]string, 0) + } usersCollections.Users = users err = json.NewEncoder(w).Encode(usersCollections) @@ -238,9 +268,13 @@ func (sh *securityHandler) baseUsers(w http.ResponseWriter, r *http.Request) { func (sh *securityHandler) handleUsers(w http.ResponseWriter, r *http.Request) { subpath := path.Clean(r.URL.Path[len(securityPrefix):]) - // Split "/users/username/command". + // Split "/users/username". // First item is an empty string, second is "users" pieces := strings.Split(subpath, "/") + if len(pieces) == 2 { + sh.baseUsers(w, r) + return + } if len(pieces) != 3 { writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Invalid path")) return @@ -285,14 +319,20 @@ func (sh *securityHandler) forUser(w http.ResponseWriter, r *http.Request, user writeError(w, httptypes.NewHTTPError(400, "User JSON name does not match the name in the URL")) return } - newuser, err := sh.sec.CreateOrUpdateUser(u) + newuser, created, err := sh.sec.CreateOrUpdateUser(u) if err != nil { writeError(w, err) return } - newuser.Password = "" + if u.Password == "" { + newuser.Password = "" + } - w.WriteHeader(http.StatusCreated) + if created { + w.WriteHeader(http.StatusCreated) + } else { + w.WriteHeader(http.StatusOK) + } err = json.NewEncoder(w).Encode(newuser) if err != nil { log.Println("etcdhttp: forUser error encoding on", r.URL) @@ -331,17 +371,7 @@ func (sh *securityHandler) enableDisable(w http.ResponseWriter, r *http.Request) log.Println("etcdhttp: error encoding security state on", r.URL) } case "PUT": - var in security.User - err := json.NewDecoder(r.Body).Decode(&in) - if err != nil { - writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Invalid JSON in request body.")) - return - } - if in.User != "root" { - writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Need to create root user")) - return - } - err = sh.sec.EnableSecurity(in) + err := sh.sec.EnableSecurity() if err != nil { writeError(w, err) return diff --git a/etcdserver/etcdhttp/http.go b/etcdserver/etcdhttp/http.go index 85eed71df46..d376763a873 100644 --- a/etcdserver/etcdhttp/http.go +++ b/etcdserver/etcdhttp/http.go @@ -52,7 +52,7 @@ func writeError(w http.ResponseWriter, err error) { e.WriteTo(w) case *httptypes.HTTPError: e.WriteTo(w) - case security.MergeError: + case security.Error: herr := httptypes.NewHTTPError(http.StatusBadRequest, e.Error()) herr.WriteTo(w) default: diff --git a/etcdserver/security/security.go b/etcdserver/security/security.go index a89e9e59ed6..c0877a06b00 100644 --- a/etcdserver/security/security.go +++ b/etcdserver/security/security.go @@ -17,12 +17,16 @@ package security import ( "encoding/json" "fmt" + "log" "path" + "reflect" "sort" + "strings" "time" "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/crypto/bcrypt" "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" + etcderr "github.com/coreos/etcd/error" "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdserverpb" "github.com/coreos/etcd/pkg/types" @@ -31,8 +35,34 @@ import ( const ( // StorePermsPrefix is the internal prefix of the storage layer dedicated to storing user data. StorePermsPrefix = "/2" + + // RootRoleName is the name of the ROOT role, with privileges to manage the cluster. + RootRoleName = "root" + + // GuestRoleName is the name of the role that defines the privileges of an unauthenticated user. + GuestRoleName = "guest" ) +var rootRole = Role{ + Role: RootRoleName, + Permissions: Permissions{ + KV: rwPermission{ + Read: []string{"*"}, + Write: []string{"*"}, + }, + }, +} + +var guestRole = Role{ + Role: GuestRoleName, + Permissions: Permissions{ + KV: rwPermission{ + Read: []string{"*"}, + Write: []string{"*"}, + }, + }, +} + type doer interface { Do(context.Context, etcdserverpb.Request) (etcdserver.Response, error) } @@ -67,14 +97,18 @@ type rwPermission struct { Write []string `json:"write"` } -type MergeError struct { +type Error struct { errmsg string } -func (m MergeError) Error() string { return m.errmsg } +func (se Error) Error() string { return se.errmsg } + +func mergeErr(s string, v ...interface{}) Error { + return Error{fmt.Sprintf("security-merging: "+s, v...)} +} -func mergeErr(s string, v ...interface{}) MergeError { - return MergeError{fmt.Sprintf(s, v...)} +func securityErr(s string, v ...interface{}) Error { + return Error{fmt.Sprintf("security: "+s, v...)} } func NewStore(server doer, timeout time.Duration) *Store { @@ -82,6 +116,7 @@ func NewStore(server doer, timeout time.Duration) *Store { server: server, timeout: timeout, } + s.ensureSecurityDirectories() s.enabled = s.detectSecurity() return s } @@ -103,6 +138,11 @@ func (s *Store) AllUsers() ([]string, error) { func (s *Store) GetUser(name string) (User, error) { resp, err := s.requestResource("/users/"+name, false) if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeKeyNotFound { + return User{}, securityErr("User %s does not exist.", name) + } + } return User{}, err } var u User @@ -110,61 +150,92 @@ func (s *Store) GetUser(name string) (User, error) { if err != nil { return u, err } + // Require that root always has a root role. + if u.User == "root" { + u.Roles = append(u.Roles, RootRoleName) + } return u, nil } -func (s *Store) CreateOrUpdateUser(user User) (User, error) { - _, err := s.GetUser(user.User) +func (s *Store) CreateOrUpdateUser(user User) (out User, created bool, err error) { + _, err = s.GetUser(user.User) if err == nil { // Remove the update-user roles from updating downstream. // Roles are granted or revoked, not changed directly. user.Roles = nil - return s.UpdateUser(user) + out, err := s.UpdateUser(user) + return out, false, err } - return user, s.CreateUser(user) + u, err := s.CreateUser(user) + return u, true, err } -func (s *Store) CreateUser(user User) error { - if user.User == "root" { - return mergeErr("Cannot create root user; enable security to set root.") +func (s *Store) CreateUser(user User) (User, error) { + u, err := s.createUserInternal(user) + if err == nil { + log.Printf("security: created user %s", user.User) } - return s.createUserInternal(user) + return u, err } -func (s *Store) createUserInternal(user User) error { +func (s *Store) createUserInternal(user User) (User, error) { + if user.Password == "" { + return user, securityErr("Cannot create user %s with an empty password", user.User) + } hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), bcrypt.DefaultCost) if err != nil { - return err + return user, err } user.Password = string(hash) _, err = s.createResource("/users/"+user.User, user) - return err + if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeNodeExist { + return user, securityErr("User %s already exists.", user.User) + } + } + } + return user, err } func (s *Store) DeleteUser(name string) error { - if name == "root" { - return mergeErr("Can't delete root user; disable security instead.") + if s.SecurityEnabled() && name == "root" { + return securityErr("Cannot delete root user while security is enabled.") } _, err := s.deleteResource("/users/" + name) + if err == nil { + log.Printf("security: deleted user %s", name) + } return err } func (s *Store) UpdateUser(user User) (User, error) { old, err := s.GetUser(user.User) if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeKeyNotFound { + return user, securityErr("User %s doesn't exist.", user.User) + } + } return old, err } newUser, err := old.Merge(user) if err != nil { return old, err } + if reflect.DeepEqual(old, newUser) { + if user.Revoke != nil || user.Grant != nil { + return old, securityErr("User not updated. Grant/Revoke lists didn't match any current roles.") + } + return old, securityErr("User not updated. Use Grant/Revoke/Password to update the user.") + } _, err = s.updateResource("/users/"+user.User, newUser) - if err != nil { - return newUser, err + if err == nil { + log.Printf("security: updated user %s", user.User) } - return newUser, nil + return newUser, err } func (s *Store) AllRoles() ([]string, error) { @@ -177,14 +248,22 @@ func (s *Store) AllRoles() ([]string, error) { _, role := path.Split(n.Key) nodes = append(nodes, role) } + nodes = append(nodes, RootRoleName) sort.Strings(nodes) return nodes, nil } func (s *Store) GetRole(name string) (Role, error) { - // TODO(barakmich): Possibly add a cache + if name == RootRoleName { + return rootRole, nil + } resp, err := s.requestResource("/roles/"+name, false) if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeKeyNotFound { + return Role{}, securityErr("Role %s does not exist.", name) + } + } return Role{}, err } var r Role @@ -196,63 +275,124 @@ func (s *Store) GetRole(name string) (Role, error) { return r, nil } -func (s *Store) CreateOrUpdateRole(role Role) (Role, error) { - _, err := s.GetRole(role.Role) +func (s *Store) CreateOrUpdateRole(r Role) (role Role, created bool, err error) { + _, err = s.GetRole(r.Role) if err == nil { - return s.UpdateRole(role) + role, err = s.UpdateRole(r) + created = false + return } - return role, s.CreateRole(role) + return r, true, s.CreateRole(r) } func (s *Store) CreateRole(role Role) error { + if role.Role == RootRoleName { + return securityErr("Cannot modify role %s: is root role.", role.Role) + } _, err := s.createResource("/roles/"+role.Role, role) + if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeNodeExist { + return securityErr("Role %s already exists.", role.Role) + } + } + } + if err == nil { + log.Printf("security: created new role %s", role.Role) + } return err } func (s *Store) DeleteRole(name string) error { + if name == RootRoleName { + return securityErr("Cannot modify role %s: is superuser role.", name) + } _, err := s.deleteResource("/roles/" + name) + if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeKeyNotFound { + return securityErr("Role %s doesn't exist.", name) + } + } + } + if err == nil { + log.Printf("security: deleted role %s", name) + } return err } func (s *Store) UpdateRole(role Role) (Role, error) { old, err := s.GetRole(role.Role) if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeKeyNotFound { + return role, securityErr("Role %s doesn't exist.", role.Role) + } + } return old, err } newRole, err := old.Merge(role) if err != nil { return old, err } + if reflect.DeepEqual(old, newRole) { + if role.Revoke != nil || role.Grant != nil { + return old, securityErr("Role not updated. Grant/Revoke lists didn't match any current permissions.") + } + return old, securityErr("Role not updated. Use Grant/Revoke to update the role.") + } _, err = s.updateResource("/roles/"+role.Role, newRole) - if err != nil { - return newRole, err + if err == nil { + log.Printf("security: updated role %s", role.Role) } - return newRole, nil + return newRole, err } func (s *Store) SecurityEnabled() bool { return s.enabled } -func (s *Store) EnableSecurity(rootUser User) error { +func (s *Store) EnableSecurity() error { + if s.SecurityEnabled() { + return securityErr("already enabled") + } err := s.ensureSecurityDirectories() if err != nil { return err } - if rootUser.User != "root" { - mergeErr("Trying to create root user not named root") + _, err = s.GetUser("root") + if err != nil { + return securityErr("No root user available, please create one") + } + _, err = s.GetRole(GuestRoleName) + if err != nil { + log.Printf("security: no guest role access found, creating default") + err := s.CreateRole(guestRole) + if err != nil { + log.Printf("security: error creating guest role. aborting security enable.") + return err + } } - err = s.createUserInternal(rootUser) + err = s.enableSecurity() if err == nil { s.enabled = true + log.Printf("security: enabled security") + } else { + log.Printf("error enabling security: %v", err) } return err } func (s *Store) DisableSecurity() error { - err := s.DeleteUser("root") + if !s.SecurityEnabled() { + return securityErr("already disabled") + } + err := s.disableSecurity() if err == nil { s.enabled = false + log.Printf("security: disabled security") + } else { + log.Printf("error disabling security: %v", err) } return err } @@ -279,13 +419,15 @@ func (u User) Merge(n User) (User, error) { currentRoles := types.NewUnsafeSet(u.Roles...) for _, g := range n.Grant { if currentRoles.Contains(g) { - return out, mergeErr("Granting duplicate role %s for user %s", g, n.User) + log.Printf("Granting duplicate role %s for user %s", g, n.User) + continue } currentRoles.Add(g) } for _, r := range n.Revoke { if !currentRoles.Contains(r) { - return out, mergeErr("Revoking ungranted role %s for user %s", r, n.User) + log.Printf("Revoking ungranted role %s for user %s", r, n.User) + continue } currentRoles.Remove(r) } @@ -319,9 +461,19 @@ func (r Role) Merge(n Role) (Role, error) { } func (r Role) HasKeyAccess(key string, write bool) bool { + if r.Role == RootRoleName { + return true + } return r.Permissions.KV.HasAccess(key, write) } +func (r Role) HasRecursiveAccess(key string, write bool) bool { + if r.Role == RootRoleName { + return true + } + return r.Permissions.KV.HasRecursiveAccess(key, write) +} + // Grant adds a set of permissions to the permission object on which it is called, // returning a new permission object. func (p Permissions) Grant(n *Permissions) (Permissions, error) { @@ -376,14 +528,16 @@ func (rw rwPermission) Revoke(n rwPermission) (rwPermission, error) { currentRead := types.NewUnsafeSet(rw.Read...) for _, r := range n.Read { if !currentRead.Contains(r) { - return out, mergeErr("Revoking ungranted read permission %s", r) + log.Printf("Revoking ungranted read permission %s", r) + continue } currentRead.Remove(r) } currentWrite := types.NewUnsafeSet(rw.Write...) for _, w := range n.Write { if !currentWrite.Contains(w) { - return out, mergeErr("Revoking ungranted write permission %s", w) + log.Printf("Revoking ungranted write permission %s", w) + continue } currentWrite.Remove(w) } @@ -400,10 +554,38 @@ func (rw rwPermission) HasAccess(key string, write bool) bool { list = rw.Read } for _, pat := range list { - match, err := path.Match(pat, key) + match, err := simpleMatch(pat, key) + if err == nil && match { + return true + } + } + return false +} + +func (rw rwPermission) HasRecursiveAccess(key string, write bool) bool { + list := rw.Read + if write { + list = rw.Write + } + for _, pat := range list { + match, err := prefixMatch(pat, key) if err == nil && match { return true } } return false } + +func simpleMatch(pattern string, key string) (match bool, err error) { + if pattern[len(pattern)-1] == '*' { + return strings.HasPrefix(key, pattern[:len(pattern)-1]), nil + } + return key == pattern, nil +} + +func prefixMatch(pattern string, key string) (match bool, err error) { + if pattern[len(pattern)-1] != '*' { + return false, nil + } + return strings.HasPrefix(key, pattern[:len(pattern)-1]), nil +} diff --git a/etcdserver/security/security_requests.go b/etcdserver/security/security_requests.go index 28e7f92c662..7e8da4ddd0b 100644 --- a/etcdserver/security/security_requests.go +++ b/etcdserver/security/security_requests.go @@ -47,18 +47,49 @@ func (s *Store) ensureSecurityDirectories() error { return err } } + _, err := s.createResource("/enabled", false) + if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeNodeExist { + return nil + } + } + return err + } return nil } +func (s *Store) enableSecurity() error { + _, err := s.updateResource("/enabled", true) + return err +} +func (s *Store) disableSecurity() error { + _, err := s.updateResource("/enabled", false) + return err +} + func (s *Store) detectSecurity() bool { if s.server == nil { return false } - _, err := s.requestResource("/users/root", false) - if err == nil { - return true + value, err := s.requestResource("/enabled", false) + if err != nil { + if e, ok := err.(*etcderr.Error); ok { + if e.ErrorCode == etcderr.EcodeNodeExist { + return false + } + } + log.Println("security: Trying to detect security settings failed:", err) + return false + } + + var u bool + err = json.Unmarshal([]byte(*value.Event.Node.Value), &u) + if err != nil { + log.Println("security: internal bookkeeping value for enabled isn't valid JSON") + return false } - return false + return u } func (s *Store) requestResource(res string, dir bool) (etcdserver.Response, error) { diff --git a/etcdserver/security/security_test.go b/etcdserver/security/security_test.go index 688337f5328..1c7fc55d9f8 100644 --- a/etcdserver/security/security_test.go +++ b/etcdserver/security/security_test.go @@ -109,19 +109,19 @@ func TestMergeRole(t *testing.T) { }, { Role{Role: "foo"}, - Role{Role: "foo", Grant: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Grant: &Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, false, }, { Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, - Role{Role: "foo", Revoke: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Revoke: &Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{}, Write: []string{}}}}, false, }, { Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/bardir"}}}}, - Role{Role: "foo", Revoke: Permissions{KV: rwPermission{Read: []string{"/foodir"}}}}, + Role{Role: "foo", Revoke: &Permissions{KV: rwPermission{Read: []string{"/foodir"}}}}, Role{}, true, }, @@ -140,28 +140,33 @@ func TestMergeRole(t *testing.T) { } type testDoer struct { - get etcdserver.Response + get []etcdserver.Response + index int } func (td *testDoer) Do(_ context.Context, req etcdserverpb.Request) (etcdserver.Response, error) { if req.Method == "GET" { - return td.get, nil + res := td.get[td.index] + td.index++ + return res, nil } return etcdserver.Response{}, nil } func TestAllUsers(t *testing.T) { d := &testDoer{ - etcdserver.Response{ - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Nodes: store.NodeExterns{ - &store.NodeExtern{ - Key: StorePermsPrefix + "/users/cat", - }, - &store.NodeExtern{ - Key: StorePermsPrefix + "/users/dog", + get: []etcdserver.Response{ + { + Event: &store.Event{ + Action: store.Get, + Node: &store.NodeExtern{ + Nodes: store.NodeExterns{ + &store.NodeExtern{ + Key: StorePermsPrefix + "/users/cat", + }, + &store.NodeExtern{ + Key: StorePermsPrefix + "/users/dog", + }, }, }, }, @@ -170,7 +175,7 @@ func TestAllUsers(t *testing.T) { } expected := []string{"cat", "dog"} - s := NewStore(d, time.Second) + s := Store{d, time.Second, false} users, err := s.AllUsers() if err != nil { t.Error("Unexpected error", err) @@ -183,19 +188,21 @@ func TestAllUsers(t *testing.T) { func TestGetAndDeleteUser(t *testing.T) { data := `{"user": "cat", "roles" : ["animal"]}` d := &testDoer{ - etcdserver.Response{ - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Key: StorePermsPrefix + "/users/cat", - Value: &data, + get: []etcdserver.Response{ + { + Event: &store.Event{ + Action: store.Get, + Node: &store.NodeExtern{ + Key: StorePermsPrefix + "/users/cat", + Value: &data, + }, }, }, }, } expected := User{User: "cat", Roles: []string{"animal"}} - s := NewStore(d, time.Second) + s := Store{d, time.Second, false} out, err := s.GetUser("cat") if err != nil { t.Error("Unexpected error", err) @@ -211,50 +218,54 @@ func TestGetAndDeleteUser(t *testing.T) { func TestAllRoles(t *testing.T) { d := &testDoer{ - etcdserver.Response{ - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Nodes: store.NodeExterns{ - &store.NodeExtern{ - Key: StorePermsPrefix + "/roles/animal", - }, - &store.NodeExtern{ - Key: StorePermsPrefix + "/roles/human", + get: []etcdserver.Response{ + { + Event: &store.Event{ + Action: store.Get, + Node: &store.NodeExtern{ + Nodes: store.NodeExterns{ + &store.NodeExtern{ + Key: StorePermsPrefix + "/roles/animal", + }, + &store.NodeExtern{ + Key: StorePermsPrefix + "/roles/human", + }, }, }, }, }, }, } - expected := []string{"animal", "human"} + expected := []string{"animal", "human", "root"} - s := NewStore(d, time.Second) + s := Store{d, time.Second, false} out, err := s.AllRoles() if err != nil { t.Error("Unexpected error", err) } if !reflect.DeepEqual(out, expected) { - t.Error("AllUsers doesn't match given store. Got", out, "expected", expected) + t.Error("AllRoles doesn't match given store. Got", out, "expected", expected) } } func TestGetAndDeleteRole(t *testing.T) { data := `{"role": "animal"}` d := &testDoer{ - etcdserver.Response{ - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Key: StorePermsPrefix + "/roles/animal", - Value: &data, + get: []etcdserver.Response{ + { + Event: &store.Event{ + Action: store.Get, + Node: &store.NodeExtern{ + Key: StorePermsPrefix + "/roles/animal", + Value: &data, + }, }, }, }, } expected := Role{Role: "animal"} - s := NewStore(d, time.Second) + s := Store{d, time.Second, false} out, err := s.GetRole("animal") if err != nil { t.Error("Unexpected error", err) @@ -267,3 +278,71 @@ func TestGetAndDeleteRole(t *testing.T) { t.Error("Unexpected error", err) } } + +func TestEnsure(t *testing.T) { + d := &testDoer{ + get: []etcdserver.Response{ + { + Event: &store.Event{ + Action: store.Set, + Node: &store.NodeExtern{ + Key: StorePermsPrefix, + Dir: true, + }, + }, + }, + { + Event: &store.Event{ + Action: store.Set, + Node: &store.NodeExtern{ + Key: StorePermsPrefix + "/users/", + Dir: true, + }, + }, + }, + { + Event: &store.Event{ + Action: store.Set, + Node: &store.NodeExtern{ + Key: StorePermsPrefix + "/roles/", + Dir: true, + }, + }, + }, + }, + } + + s := Store{d, time.Second, false} + err := s.ensureSecurityDirectories() + if err != nil { + t.Error("Unexpected error", err) + } +} + +func TestSimpleMatch(t *testing.T) { + role := Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir/*", "/fookey"}, Write: []string{"/bardir/*", "/barkey"}}}} + if !role.HasKeyAccess("/foodir/foo/bar", false) { + t.Fatal("role lacks expected access") + } + if !role.HasKeyAccess("/fookey", false) { + t.Fatal("role lacks expected access") + } + if role.HasKeyAccess("/bardir/bar/foo", false) { + t.Fatal("role has unexpected access") + } + if role.HasKeyAccess("/barkey", false) { + t.Fatal("role has unexpected access") + } + if role.HasKeyAccess("/foodir/foo/bar", true) { + t.Fatal("role has unexpected access") + } + if role.HasKeyAccess("/fookey", true) { + t.Fatal("role has unexpected access") + } + if !role.HasKeyAccess("/bardir/bar/foo", true) { + t.Fatal("role lacks expected access") + } + if !role.HasKeyAccess("/barkey", true) { + t.Fatal("role lacks expected access") + } +} diff --git a/integration/v2_http_kv_test.go b/integration/v2_http_kv_test.go index b6439dbd361..8b9525ed1fa 100644 --- a/integration/v2_http_kv_test.go +++ b/integration/v2_http_kv_test.go @@ -53,19 +53,19 @@ func TestV2Set(t *testing.T) { "/v2/keys/foo/bar", v, http.StatusCreated, - `{"action":"set","node":{"key":"/foo/bar","value":"bar","modifiedIndex":3,"createdIndex":3}}`, + `{"action":"set","node":{"key":"/foo/bar","value":"bar","modifiedIndex":7,"createdIndex":7}}`, }, { "/v2/keys/foodir?dir=true", url.Values{}, http.StatusCreated, - `{"action":"set","node":{"key":"/foodir","dir":true,"modifiedIndex":4,"createdIndex":4}}`, + `{"action":"set","node":{"key":"/foodir","dir":true,"modifiedIndex":8,"createdIndex":8}}`, }, { "/v2/keys/fooempty", url.Values(map[string][]string{"value": {""}}), http.StatusCreated, - `{"action":"set","node":{"key":"/fooempty","value":"","modifiedIndex":5,"createdIndex":5}}`, + `{"action":"set","node":{"key":"/fooempty","value":"","modifiedIndex":9,"createdIndex":9}}`, }, } @@ -214,12 +214,12 @@ func TestV2CAS(t *testing.T) { }, { "/v2/keys/cas/foo", - url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"3"}}), + url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"7"}}), http.StatusOK, map[string]interface{}{ "node": map[string]interface{}{ "value": "YYY", - "modifiedIndex": float64(4), + "modifiedIndex": float64(8), }, "action": "compareAndSwap", }, @@ -231,8 +231,8 @@ func TestV2CAS(t *testing.T) { map[string]interface{}{ "errorCode": float64(101), "message": "Compare failed", - "cause": "[10 != 4]", - "index": float64(4), + "cause": "[10 != 8]", + "index": float64(8), }, }, { @@ -281,7 +281,7 @@ func TestV2CAS(t *testing.T) { map[string]interface{}{ "errorCode": float64(101), "message": "Compare failed", - "cause": "[bad_value != ZZZ] [100 != 5]", + "cause": "[bad_value != ZZZ] [100 != 9]", }, }, { @@ -291,12 +291,12 @@ func TestV2CAS(t *testing.T) { map[string]interface{}{ "errorCode": float64(101), "message": "Compare failed", - "cause": "[100 != 5]", + "cause": "[100 != 9]", }, }, { "/v2/keys/cas/foo", - url.Values(map[string][]string{"value": {"XXX"}, "prevValue": {"bad_value"}, "prevIndex": {"5"}}), + url.Values(map[string][]string{"value": {"XXX"}, "prevValue": {"bad_value"}, "prevIndex": {"9"}}), http.StatusPreconditionFailed, map[string]interface{}{ "errorCode": float64(101), @@ -446,7 +446,7 @@ func TestV2CAD(t *testing.T) { map[string]interface{}{ "errorCode": float64(101), "message": "Compare failed", - "cause": "[100 != 3]", + "cause": "[100 != 7]", }, }, { @@ -458,12 +458,12 @@ func TestV2CAD(t *testing.T) { }, }, { - "/v2/keys/foo?prevIndex=3", + "/v2/keys/foo?prevIndex=7", http.StatusOK, map[string]interface{}{ "node": map[string]interface{}{ "key": "/foo", - "modifiedIndex": float64(5), + "modifiedIndex": float64(9), }, "action": "compareAndDelete", }, @@ -491,7 +491,7 @@ func TestV2CAD(t *testing.T) { map[string]interface{}{ "node": map[string]interface{}{ "key": "/foovalue", - "modifiedIndex": float64(6), + "modifiedIndex": float64(10), }, "action": "compareAndDelete", }, @@ -529,7 +529,7 @@ func TestV2Unique(t *testing.T) { http.StatusCreated, map[string]interface{}{ "node": map[string]interface{}{ - "key": "/foo/3", + "key": "/foo/7", "value": "XXX", }, "action": "create", @@ -541,7 +541,7 @@ func TestV2Unique(t *testing.T) { http.StatusCreated, map[string]interface{}{ "node": map[string]interface{}{ - "key": "/foo/4", + "key": "/foo/8", "value": "XXX", }, "action": "create", @@ -553,7 +553,7 @@ func TestV2Unique(t *testing.T) { http.StatusCreated, map[string]interface{}{ "node": map[string]interface{}{ - "key": "/bar/5", + "key": "/bar/9", "value": "XXX", }, "action": "create", @@ -615,8 +615,8 @@ func TestV2Get(t *testing.T) { map[string]interface{}{ "key": "/foo/bar", "dir": true, - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), }, }, }, @@ -634,14 +634,14 @@ func TestV2Get(t *testing.T) { map[string]interface{}{ "key": "/foo/bar", "dir": true, - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), "nodes": []interface{}{ map[string]interface{}{ "key": "/foo/bar/zar", "value": "XXX", - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), }, }, }, @@ -709,8 +709,8 @@ func TestV2QuorumGet(t *testing.T) { map[string]interface{}{ "key": "/foo/bar", "dir": true, - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), }, }, }, @@ -728,14 +728,14 @@ func TestV2QuorumGet(t *testing.T) { map[string]interface{}{ "key": "/foo/bar", "dir": true, - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), "nodes": []interface{}{ map[string]interface{}{ "key": "/foo/bar/zar", "value": "XXX", - "createdIndex": float64(3), - "modifiedIndex": float64(3), + "createdIndex": float64(7), + "modifiedIndex": float64(7), }, }, }, @@ -781,7 +781,7 @@ func TestV2Watch(t *testing.T) { "node": map[string]interface{}{ "key": "/foo/bar", "value": "XXX", - "modifiedIndex": float64(3), + "modifiedIndex": float64(7), }, "action": "set", } @@ -802,7 +802,7 @@ func TestV2WatchWithIndex(t *testing.T) { var body map[string]interface{} c := make(chan bool, 1) go func() { - resp, _ := tc.Get(fmt.Sprintf("%s%s", u, "/v2/keys/foo/bar?wait=true&waitIndex=4")) + resp, _ := tc.Get(fmt.Sprintf("%s%s", u, "/v2/keys/foo/bar?wait=true&waitIndex=8")) body = tc.ReadBodyJSON(resp) c <- true }() @@ -839,7 +839,7 @@ func TestV2WatchWithIndex(t *testing.T) { "node": map[string]interface{}{ "key": "/foo/bar", "value": "XXX", - "modifiedIndex": float64(4), + "modifiedIndex": float64(8), }, "action": "set", }