From 860999951d5a5ea5877cfa33bdc343f6f227dcf1 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Thu, 6 Jan 2022 19:56:56 -0500 Subject: [PATCH] Enhance Authentication and Authorization system (#940) --- DOC/SECURITY.md | 19 +++++++++-- auth/credential_store.go | 34 +++++++++++++------ auth/credential_store_test.go | 58 +++++++++++++++++++++++++++++++++ cluster/join.go | 36 ++++++++++++++++++++ cluster/join_test.go | 46 ++++++++++++++++++++++++++ cmd/rqlited/main.go | 34 ++++++++++++++----- http/service.go | 37 +++++++++++---------- http/service_test.go | 2 +- system_test/full_system_test.py | 32 ++++++++++++++++++ 9 files changed, 258 insertions(+), 40 deletions(-) diff --git a/DOC/SECURITY.md b/DOC/SECURITY.md index 54c6100c..cbc11a70 100644 --- a/DOC/SECURITY.md +++ b/DOC/SECURITY.md @@ -59,13 +59,17 @@ An example configuration file is shown below. { "username": "mary", "password": "$2a$10$fKRHxrEuyDTP6tXIiDycr.nyC8Q7UMIfc31YMyXHDLgRDyhLK3VFS", - "perms": ["query", "status"] + "perms": ["query", "backup"] + }, + { + "username": "*", + "perms": ["status", "ready"] } ] ``` -This configuration file sets authentication for two usernames, _bob_ and _mary_, and it sets a password for each. No other users will be able to access the cluster. +This configuration file sets authentication for three usernames, _bob_, _mary_, and `*`. It sets a password for the first two. -This configuration also sets permissions for both users. _bob_ has permission to perform all operations, but _mary_ can only query the cluster, as well as check the cluster status. +This configuration also sets permissions for all usernames. _bob_ has permission to perform all operations, but _mary_ can only query the cluster, as well as backup the cluster. `*` is a special username, which indicates that all users -- even anonymous users (requests without any BasicAuth information) -- have permission to check the cluster and readiness. This can be useful if you wish to leave certain operations open to all accesses. ## Secure cluster example Starting a node with HTTPS enabled, node-to-node encryption, and with the above configuration file. It is assumed the HTTPS X.509 certificate and key are at the paths `server.crt` and `key.pem` respectively, and the node-to-node certificate and key are at `node.crt` and `node-key.pem` @@ -86,3 +90,12 @@ Querying the node, as user _mary_. curl -G 'https://mary:secret2@localhost:4001/db/query?pretty&timings' \ --data-urlencode 'q=SELECT * FROM foo' ``` + +### Avoiding passwords at the command line +The above example suffer from one shortcoming -- the password for user `bob` is entered at the command line. This is not ideal, as someone with access to the process table could learn the password. You can avoid this via the `-join-as` option, which will tell rqlite to retrieve the password from the configuration file. +```bash +rqlited -auth config.json -http-addr localhost:4003 -http-cert server.crt \ +-http-key key.pem -raft-addr :4004 -join https://localhost:4001 -join-as bob \ +-node-encrypt -node-cert node.crt -node-key node-key.pem -no-node-verify \ +~/node.2 +``` diff --git a/auth/credential_store.go b/auth/credential_store.go index f1e9d71c..30e2b97d 100644 --- a/auth/credential_store.go +++ b/auth/credential_store.go @@ -9,6 +9,10 @@ import ( "golang.org/x/crypto/bcrypt" ) +// AllUsers is the username that indicates all users, even anonymous users (requests without +// any BasicAuth information). +const AllUsers = "*" + // BasicAuther is the interface an object must support to return basic auth information. type BasicAuther interface { BasicAuth() (string, string, bool) @@ -76,6 +80,12 @@ func (c *CredentialsStore) Check(username, password string) bool { bcrypt.CompareHashAndPassword([]byte(pw), []byte(password)) == nil } +// Password returns the password for the given user. +func (c *CredentialsStore) Password(username string) (string, bool) { + pw, ok := c.store[username] + return pw, ok +} + // CheckRequest returns true if b contains a valid username and password. func (c *CredentialsStore) CheckRequest(b BasicAuther) bool { username, password, ok := b.BasicAuth() @@ -85,22 +95,26 @@ func (c *CredentialsStore) CheckRequest(b BasicAuther) bool { return true } -// HasPerm returns true if username has the given perm. It does not -// perform any password checking. +// HasPerm returns true if username has the given perm, either directly or +// via AllUsers. It does not perform any password checking. func (c *CredentialsStore) HasPerm(username string, perm string) bool { - m, ok := c.perms[username] - if !ok { - return false + if m, ok := c.perms[username]; ok { + if _, ok := m[perm]; ok { + return true + } } - if _, ok := m[perm]; !ok { - return false + if m, ok := c.perms[AllUsers]; ok { + if _, ok := m[perm]; ok { + return true + } } - return true + + return false } -// HasAnyPerm returns true if username has at least one of the given perms. -// It does not perform any password checking. +// HasAnyPerm returns true if username has at least one of the given perms, +// either directly, or via AllUsers. It does not perform any password checking. func (c *CredentialsStore) HasAnyPerm(username string, perm ...string) bool { return func(p []string) bool { for i := range p { diff --git a/auth/credential_store_test.go b/auth/credential_store_test.go index d07552fd..70f3014b 100644 --- a/auth/credential_store_test.go +++ b/auth/credential_store_test.go @@ -40,6 +40,17 @@ func Test_AuthLoadSingle(t *testing.T) { if check := store.Check("wrong", "wrong"); check { t.Fatalf("single credential not loaded correctly") } + + var pw string + var ok bool + pw, ok = store.Password("username1") + if pw != "password1" || !ok { + t.Fatalf("wrong password returned") + } + _, ok = store.Password("nonsense") + if ok { + t.Fatalf("password returned for nonexistent user") + } } func Test_AuthLoadMultiple(t *testing.T) { @@ -328,3 +339,50 @@ func Test_AuthPermsNilLoadSingle(t *testing.T) { t.Fatalf("wrong has foo perm") } } + +func Test_AuthPermsAllUsers(t *testing.T) { + const jsonStream = ` + [ + { + "username": "username1", + "password": "password1", + "perms": ["foo"] + }, + { + "username": "*", + "perms": ["bar", "abc"] + } + ] + ` + + store := NewCredentialsStore() + if err := store.Load(strings.NewReader(jsonStream)); err != nil { + t.Fatalf("failed to load single credential: %s", err.Error()) + } + + if check := store.Check("username1", "password1"); !check { + t.Fatalf("single credential not loaded correctly") + } + if check := store.Check("username1", "wrong"); check { + t.Fatalf("single credential not loaded correctly") + } + + if perm := store.HasPerm("username1", "qux"); perm { + t.Fatalf("username1 has qux perm") + } + if perm := store.HasPerm(AllUsers, "bar"); !perm { + t.Fatalf("* does not have bar perm") + } + if perm := store.HasPerm(AllUsers, "abc"); !perm { + t.Fatalf("* does not have abc perm") + } + if perm := store.HasPerm(AllUsers, "foo"); perm { + t.Fatalf("* has foo perm") + } + if perm := store.HasPerm("username1", "bar"); !perm { + t.Fatalf("username1 does not have bar perm via *") + } + if perm := store.HasPerm("username1", "abc"); !perm { + t.Fatalf("username1 does not have abc perm via *") + } +} diff --git a/cluster/join.go b/cluster/join.go index c224c157..f44cc371 100644 --- a/cluster/join.go +++ b/cluster/join.go @@ -10,6 +10,7 @@ import ( "log" "net" "net/http" + "net/url" "os" "strings" "time" @@ -18,10 +19,45 @@ import ( ) var ( + // ErrUserInfoExists is returned when a join address already contains + // a username and a password. + ErrUserInfoExists = errors.New("userinfo exists") + // ErrJoinFailed is returned when a node fails to join a cluster ErrJoinFailed = errors.New("failed to join cluster") ) +// AddUserInfo adds username and password to the join address. If username is empty +// joinAddr is returned unchanged. If joinAddr already contains a username, ErrUserInfoExists +// is returned. +func AddUserInfo(joinAddr, username, password string) (string, error) { + if username == "" { + return joinAddr, nil + } + + u, err := url.Parse(joinAddr) + if err != nil { + return "", err + } + + if u.User != nil && u.User.Username() != "" { + return "", ErrUserInfoExists + } + + u.User = url.UserPassword(username, password) + return u.String(), nil +} + +// RemoveUserInfo returns the joinAddr with any username and password removed. +func RemoveUserInfo(joinAddr string) string { + u, err := url.Parse(joinAddr) + if err != nil { + return joinAddr + } + u.User = nil + return u.String() +} + // Join attempts to join the cluster at one of the addresses given in joinAddr. // It walks through joinAddr in order, and sets the node ID and Raft address of // the joining node as id addr respectively. It returns the endpoint successfully diff --git a/cluster/join_test.go b/cluster/join_test.go index 12e113e9..b84e7eaa 100644 --- a/cluster/join_test.go +++ b/cluster/join_test.go @@ -13,6 +13,52 @@ import ( const numAttempts int = 3 const attemptInterval = 5 * time.Second +func Test_AddUserInfo(t *testing.T) { + var u string + var err error + + u, err = AddUserInfo("http://example.com", "user1", "pass1") + if err != nil { + t.Fatalf("failed to add user info: %s", err.Error()) + } + if exp, got := "http://user1:pass1@example.com", u; exp != got { + t.Fatalf("wrong URL created, exp %s, got %s", exp, got) + } + + u, err = AddUserInfo("http://example.com", "user1", "") + if err != nil { + t.Fatalf("failed to add user info: %s", err.Error()) + } + if exp, got := "http://user1:@example.com", u; exp != got { + t.Fatalf("wrong URL created, exp %s, got %s", exp, got) + } + + u, err = AddUserInfo("http://example.com", "", "pass1") + if err != nil { + t.Fatalf("failed to add user info: %s", err.Error()) + } + if exp, got := "http://example.com", u; exp != got { + t.Fatalf("wrong URL created, exp %s, got %s", exp, got) + } + + u, err = AddUserInfo("http://user1:pass1@example.com", "user2", "pass2") + if err == nil { + t.Fatalf("failed to get expected error when UserInfo exists") + } +} + +func Test_RemoveUserInfo(t *testing.T) { + if exp, got := "http://example.com", RemoveUserInfo("http://user1:pass1@example.com"); exp != got { + t.Fatalf("expected %s, got %s", exp, got) + } + if exp, got := "http://example.com", RemoveUserInfo("http://example.com"); exp != got { + t.Fatalf("expected %s, got %s", exp, got) + } + if exp, got := "nonsense", RemoveUserInfo("nonsense"); exp != got { + t.Fatalf("expected %s, got %s", exp, got) + } +} + func Test_SingleJoinOK(t *testing.T) { var body map[string]interface{} ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/rqlited/main.go b/cmd/rqlited/main.go index d065168b..571435f0 100644 --- a/cmd/rqlited/main.go +++ b/cmd/rqlited/main.go @@ -53,6 +53,7 @@ var nodeID string var raftAddr string var raftAdv string var joinAddr string +var joinAs string var joinAttempts int var joinInterval string var noVerify bool @@ -105,6 +106,7 @@ func init() { flag.StringVar(&raftAddr, "raft-addr", "localhost:4002", "Raft communication bind address") flag.StringVar(&raftAdv, "raft-adv-addr", "", "Advertised Raft communication address. If not set, same as Raft bind") flag.StringVar(&joinAddr, "join", "", "Comma-delimited list of nodes, through which a cluster can be joined (proto://host:port)") + flag.StringVar(&joinAs, "join-as", "", "Username in authentication file to join as. If not set, joins anonymously") flag.IntVar(&joinAttempts, "join-attempts", 5, "Number of join attempts to make") flag.StringVar(&joinInterval, "join-interval", "5s", "Period between join attempts") flag.StringVar(&discoURL, "disco-url", "http://discovery.rqlite.com", "Set Discovery Service URL") @@ -271,6 +273,12 @@ func main() { log.Fatalf("failed to open store: %s", err.Error()) } + // Get any credential store. + credStr, err := credentialStore() + if err != nil { + log.Fatalf("failed to get credential store: %s", err.Error()) + } + // Create cluster service now, so nodes will be able to learn information about each other. clstr, err := clusterService(mux.Listen(cluster.MuxClusterHeader), str) if err != nil { @@ -284,7 +292,7 @@ func main() { if err := clstrClient.SetLocal(raftAdv, clstr); err != nil { log.Fatalf("failed to set cluster client local parameters: %s", err.Error()) } - httpServ, err := startHTTPService(str, clstrClient) + httpServ, err := startHTTPService(str, clstrClient, credStr) if err != nil { log.Fatalf("failed to start HTTP server: %s", err.Error()) } @@ -314,6 +322,22 @@ func main() { } } + // Add credentials to any join addresses, if necessary. + if credStr != nil && joinAs != "" { + var err error + pw, ok := credStr.Password(joinAs) + if !ok { + log.Fatalf("user %s does not exist in credential store", joinAs) + } + for i := range joins { + joins[i], err = cluster.AddUserInfo(joins[i], joinAs, pw) + if err != nil { + log.Fatalf("failed to use credential store join_as: %s", err.Error()) + } + } + log.Println("added join_as identity from credential store") + } + if j, err := cluster.Join(joinSrcIP, joins, str.ID(), raftAdv, !raftNonVoter, joinAttempts, joinDur, &tlsConfig); err != nil { log.Fatalf("failed to join cluster at %s: %s", joins, err.Error()) @@ -414,13 +438,7 @@ func waitForConsensus(str *store.Store) error { return nil } -func startHTTPService(str *store.Store, cltr *cluster.Client) (*httpd.Service, error) { - // Get the credential store. - credStr, err := credentialStore() - if err != nil { - return nil, err - } - +func startHTTPService(str *store.Store, cltr *cluster.Client, credStr *auth.CredentialsStore) (*httpd.Service, error) { // Create HTTP server and load authentication information if required. var s *httpd.Service if credStr != nil { diff --git a/http/service.go b/http/service.go index f1e4def0..cd7f70d5 100644 --- a/http/service.go +++ b/http/service.go @@ -21,6 +21,7 @@ import ( "sync" "time" + "github.com/rqlite/rqlite/auth" "github.com/rqlite/rqlite/command" "github.com/rqlite/rqlite/command/encoding" "github.com/rqlite/rqlite/store" @@ -296,10 +297,6 @@ func (s *Service) HTTPS() bool { // ServeHTTP allows Service to serve HTTP requests. func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.addBuildVersion(w) - if !s.checkCredentials(r) { - w.WriteHeader(http.StatusUnauthorized) - return - } switch { case r.URL.Path == "/" || r.URL.Path == "": @@ -1028,8 +1025,8 @@ func (s *Service) FormRedirect(r *http.Request, url string) string { return fmt.Sprintf("%s%s%s", url, r.URL.Path, rq) } -// CheckRequestPerm returns true if authentication is enabled and the user contained -// in the BasicAuth request has either PermAll, or the given perm. +// CheckRequestPerm checks if the request is authenticated and authorized +// with the given Perm. func (s *Service) CheckRequestPerm(r *http.Request, perm string) (b bool) { defer func() { if b { @@ -1038,14 +1035,29 @@ func (s *Service) CheckRequestPerm(r *http.Request, perm string) (b bool) { stats.Add(numAuthFail, 1) } }() + + // No credential store? Auth is not even enabled. if s.credentialStore == nil { return true } - username, _, ok := r.BasicAuth() + // Is the required perm granted to all users, including anonymous users? + if s.credentialStore.HasAnyPerm(auth.AllUsers, perm, PermAll) { + return true + } + + // At this point there needs to be BasicAuth information in the request. + username, password, ok := r.BasicAuth() if !ok { return false } + + // Are the BasicAuth creds good? + if !s.credentialStore.Check(username, password) { + return false + } + + // Is the specified user authorized? return s.credentialStore.HasAnyPerm(username, perm, PermAll) } @@ -1116,17 +1128,6 @@ func (s *Service) addBuildVersion(w http.ResponseWriter) { w.Header().Add(VersionHTTPHeader, version) } -// checkCredentials returns if any authentication requirements -// have been successfully met. -func (s *Service) checkCredentials(r *http.Request) bool { - if s.credentialStore == nil { - return true - } - - username, password, ok := r.BasicAuth() - return ok && s.credentialStore.Check(username, password) -} - // writeResponse writes the given response to the given writer. func (s *Service) writeResponse(w http.ResponseWriter, r *http.Request, j *Response) { var b []byte diff --git a/http/service_test.go b/http/service_test.go index bfa54c1f..1a35bc7e 100644 --- a/http/service_test.go +++ b/http/service_test.go @@ -412,7 +412,7 @@ func Test_401Routes_NoBasicAuth(t *testing.T) { "/db/backup", "/db/load", "/join", - "/delete", + "/remove", "/status", "/nodes", "/readyz", diff --git a/system_test/full_system_test.py b/system_test/full_system_test.py index 873de7dd..536fff48 100644 --- a/system_test/full_system_test.py +++ b/system_test/full_system_test.py @@ -34,6 +34,7 @@ class Node(object): raft_addr=None, raft_adv=None, raft_voter=True, raft_snap_threshold=8192, raft_snap_int="1s", + auth=None, join_as=None, dir=None, on_disk=False): if api_addr is None: s, addr = random_addr() @@ -59,6 +60,8 @@ class Node(object): self.raft_voter = raft_voter self.raft_snap_threshold = raft_snap_threshold self.raft_snap_int = raft_snap_int + self.auth = auth + self.join_as = join_as self.on_disk = on_disk self.process = None self.stdout_file = os.path.join(dir, 'rqlited.log') @@ -112,8 +115,12 @@ class Node(object): command += ['-raft-adv-addr', self.raft_adv] if self.on_disk: command += ['-on-disk'] + if self.auth is not None: + command += ['-auth', self.auth] if join is not None: command += ['-join', 'http://' + join] + if self.join_as is not None: + command += ['-join-as', self.join_as] command.append(self.dir) self.process = subprocess.Popen(command, stdout=self.stdout_fd, stderr=self.stderr_fd) @@ -647,6 +654,31 @@ class TestEndToEndAdvAddr(TestEndToEnd): self.cluster = Cluster([n0, n1, n2]) +class TestAuthJoin(unittest.TestCase): + '''Test that joining works with authentication''' + + def test(self): + self.auth_file = tempfile.NamedTemporaryFile() + with open(self.auth_file.name, 'w') as f: + f.write('[{"username": "foo","password": "bar","perms": ["all"]}, {"username": "*", "perms": ["status", "ready"]}]') + + n0 = Node(RQLITED_PATH, '0', auth=self.auth_file.name) + n0.start() + n0.wait_for_leader() + + n1 = Node(RQLITED_PATH, '1', auth=self.auth_file.name) + n1.start(join=n0.APIAddr()) + self.assertRaises(Exception, n1.wait_for_leader) # Join should fail due to lack of auth. + + n2 = Node(RQLITED_PATH, '2', auth=self.auth_file.name, join_as="foo") + n2.start(join=n0.APIAddr()) + n2.wait_for_leader() + + self.cluster = Cluster([n0, n1, n2]) + + def tearDown(self): + self.auth_file.close() + self.cluster.deprovision() class TestClusterRecovery(unittest.TestCase): '''Test that a cluster can recover after all Raft network addresses change'''