Authorization not enforced for inter-node comms

[otoolep]

rqlite security is controlled as described here.

Security can be configured such that a node requires credentials to join a cluster (since joining uses the HTTP API). However all further requests by the node which joined to any other node will use the inter-node comms link, which does not enforce authorization. This means changes could be made to the cluster via node that has joined, but which doesn't have a local Security config file.

The fix, in general terms, is to enforce authorization, even for the inter-node comms channel.

[otoolep]

This should be fairly straightforward to do:

• Add a new Credentials protobuf object to https://github.com/rqlite/rqlite/blob/master/cluster/message.proto. The credentials will be "username" and "password", both strings.
• Add the Credentials object to Command (see https://github.com/rqlite/rqlite/blob/master/cluster/message.proto#L12)
• Then copy the credentials from a HTTP request to the proto object, when a request is forwarded to another node in the cluster. Presumably this will require changes to https://github.com/rqlite/rqlite/blob/master/cluster/client.go
• Adding permission checking in the Cluster Service code (at https://github.com/rqlite/rqlite/blob/master/cluster/service.go#L170) similar to how it's done currently in the HTTP layer (see https://github.com/rqlite/rqlite/blob/master/http/service.go#L1247). This means a CredentialStore object will need to be passed into the Cluster Service object, like what happens with HTTP today.
• Come up with a way to signal the error back, so the HTTP layer in the first node can respond with a 403 if the other node returns "no authorization".

New unit tests need to be added at each layer too.

[ngharrington]

Thanks @otoolep happy to take a look a this -- these notes are great and align with what I had in mind. Hopefully will spend some time this evening (EST), but certainly will look this week.

[otoolep]

There is one tricky part to this -- queued writes. Because queued writes happen asynchronously, the context of which user sent the original HTTP request is not passed to the queue. So that part is going to need some thinking.

You can see the queued write implementation in the HTTP layer.

[otoolep]

Queued writes are called "queued executes" in the code.

[otoolep]

The queuedExecute() is complicated because it can contain multiple requests, each with different credentials -- so the question becomes which credentials to use when all requests are merged in the queue. I'll need to think about it.

I suggest we just work on the synchronous path for now (execute()) and see what shape that takes. That may help us understand the best way to deal with queued writes.

[otoolep]

Thinking about this some more, it's not that bad. The main issue here is that credential-checking is not enforced in the cluster service. That is what needs to be addressed. Once that is fixed the security issue is addressed. So adding credential checking in https://github.com/rqlite/rqlite/blob/master/cluster/service.go#L170 is step one, that would patch the hole.

[otoolep]

@ngharrington - check out #1056

It's just to give you an idea of what needs to be done, I don't plan to merge it if you make the full changes. But it should give you the idea. What remains includes:

• Fixing unit tests
• Actually enforcing credential checking in the Cluster service code
• Repeating this logic for the query path too.

Note that the pb.go code was generated automatically, as per the directions in CONTRIBUTING.md

[ngharrington]

Thanks! I will take a look at this

[otoolep]

Will be fixed in 7.6.1, currently fixed on HEAD.