Add ability to configure and use tls with grpc traffic

[jmccann]

This PR is to add ability to encrypt GRPC traffic. I also split the grpc setup code to a grpc.go file for each the server and agent. If you prefer not to have that done I can work to undo it.

At a minimum the following is required to be configured:

Server

• DRONE_GRPC_CERT_FILE
• DRONE_GRPC_KEY_FILE

Agent

• DRONE_GRPC_TLS_ENABLE

This code can also be used to configure another layer of authentication using server/client cert/key verification. The following additional options are also provided to help facilitate that.

Server

• DRONE_GRPC_CA_FILE
• DRONE_GRPC_VERIFY_MODE

Agent

• DRONE_GRPC_CERT_FILE
• DRONE_GRPC_KEY_FILE
• DRONE_GRPC_CA_FILE

Finally, on the agent you can also specify DRONE_GRPC_SERVERNAME to override the hostname to use to match the certificate on the server. This is primarily for testing purposes.

I have done some manual testing of this with a private drone instance built from source.

[jmccann]

Some logs from my manual tests:

server

...
drone-server    | [GIN-debug] GET    /version                  --> github.com/drone/drone/server.Version (12 handlers)
drone-server    | [GIN-debug] GET    /healthz                  --> github.com/drone/drone/server.Health (12 handlers)
drone-server    | time="2018-02-28T16:20:06Z" level=debug msg="grpc: Configuring TLS" 
drone-server    | time="2018-02-28T16:20:06Z" level=debug msg="agent connected: 434cc4feecbe: polling" 
drone-server    | time="2018-02-28T16:20:06Z" level=debug msg="agent connected: 434cc4feecbe: polling" 
drone-server    | time="2018-02-28T16:20:07Z" level=debug msg="user feed: connection opened" 

agent

drone-agent     | {"time":"2018-02-28T16:20:06Z","level":"debug","message":"grpc: Configuring TLS"}
drone-agent     | {"time":"2018-02-28T16:20:06Z","level":"debug","message":"request next execution"}
drone-agent     | {"time":"2018-02-28T16:20:06Z","level":"debug","message":"request next execution"}

[jmccann]

Changed this from Error to Fatal as if invalid tls config was provided the server would start and hang and not return the error from TLS.

I think this is because of the errorgroup setup. The grpc server and http servers are started in g.Go calls then later g.Wait() is called. If the TLS config wants to return an error but g.Wait() is waiting for http server to finish which it never does. So this change forces Drone to fail and returns any TLS config errors encountered.

[moosh3]

\o/ this was exactly what I've been looking for. Drone is so goofy with the whole jsonrpc2 calls. Thanks for this.

[bradrydzewski]

Drone is so goofy with the whole jsonrpc2 calls. Thanks for this.

FYI, drone no longer uses jsonrpc2. It has used grpc as the transport protocol for a while now. This pull requests adds tls to the existing grpc implementation.

[strk]

@bradrydzewski without this PR we cannot upgrade Drone to avoid sending possibly sensitive information on the wire unencrypted, any reason not to accept it, beside the conflict ?

[tboerger]

It's not merged because 0.9 will get lots of improvements.

[strk]

Will these improvemets include encrypted build log streaming ?

[pyry]

@bradrydzewski Any updates when this might be introduced to drone? Securing communications between agents and server is a quite must for us for few use cases.

[bradrydzewski]

@pyry the next drone release removes support for GRPC in favor of plain old REST. This means agent-to-server communication will be encrypted as long as you have SSL enabled for your regular Drone http traffic. The next version should land in a few weeks.

[strk]

@pyry the next drone release removes support for grpc in favor of rest. So if ssl is enabled for your drone server the agent-to-server connection will be encrypted. The next version should land in a few weeks.

Great news! Will it be feasible/easy to upgrade from 0.5 to such a new release ?

[tboerger]

Great news! Will it be feasible/easy to upgrade
from 0.5 to such a new release ?

The pipeline schema will entirely change, but at least it will be versioned than.

[jmccann]

Closing this issue since GRPC goes away with 0.9