google / certificate-transparency-go

Auditing for TLS certificates (Go code)
https://certificate.transparency.dev
Apache License 2.0
861 stars 230 forks source link

Add option to enable TLS for communication with Trillian #1524

Closed fghanmi closed 6 days ago

fghanmi commented 1 week ago

Description

Currently, the communication between CTlog and Trillian server does not support Transport Layer Security. To enhance the security and integrity of services communications, it is recommended to introduce an option to enable TLS - CTlog would ensure the verification of Trillian's certificates.

mhutchinson commented 1 week ago

@fghanmi can you provide more info on the motivation for this work? The expected way to deploy Trillian with a personality (such as CT FE) is to deploy them both to the same security domain. Given that the same user would be running both the personality and Trillian, it's not clear what the security hole is that adding TLS to the connection between these services would fix. Can you provide a concrete attack vector that adding TLS would fix?

fghanmi commented 1 week ago

@mhutchinson,

fghanmi commented 1 week ago

Hello @mhutchinson, Any updates on this issue, please :) ? (and also on this one https://github.com/google/certificate-transparency-go/issues/1522)

mhutchinson commented 1 week ago

Hi @fghanmi, we've been considering this as a team and we're still not convinced that this feature is needed. In order for us to take on this change and continue support and maintenance for this feature on an ongoing basis, it is important that the security guarantees and attack model are clear. Without clear understanding of why this path is having TLS applied to it, this feature becomes a strange thing we need to look after, prompting questions down the line such as:

Hopefully you can see why this seemingly small contained change is leading to some hesitation. What would be greatly beneficial is a write-up of what is motivating this work (covering the questions above, though there are doubtless more I haven't thought to list in this moment), and some other voices from the community giving their support.

  • Ctlog and Trillian could be deployed in a different security/cluster domain (if needed by the user), in this case, TLS would ensure a secure communication.

This is not a supported deployment model for personalities and Trillian. They are intended to be run in the same cluster by the same user. Think of the relationship really as a library call rather than a remote API.

  • Not all threats come from external attackers. If an insider has access to the network/cluster, they could potentially play with the communication between CTlog and Trillian (intercept signature, update action, change signature value,...). TLS would mitigate this risk by encrypting the data in transit (man-in-the-middle attack: this can be an example of an attack vector)

A user with network level access to two co-located machines is likely to have access to the machines or DB too. If not, they still have unauthenticated access to the Trillian log and could themselves log invalid (or illegal) information into the Trillian log via the gRPC API which could destroy the service anyway.

  • For compliance and standards: TLS is required by FIPS (Federal Information Processing Standards)

I'm not familiar with the details of this. It would be unfortunate if this requirement spread to all cooperating microservices in a cluster needing authentication. If this is the case, I would consider looking for a solution that scaled better than the solution you've proposed here of having an explicit certificate for every pair-wise connection.

If you'd like to discuss this more in real-time, please join our Slack channel: https://join.slack.com/t/transparency-dev/shared_invite/zt-27pkqo21d-okUFhur7YZ0rFoJVIOPznQ

mhutchinson commented 1 week ago

Some new information has come to light that changes the equation here. I wasn't aware that Trillian already had support for TLS on the log server (https://github.com/google/trillian/blob/master/cmd/trillian_log_server/main.go#L62). Adding support in the CT personality to use this seems pretty reasonable given that this feature has already been added to the server. The questions I asked above are still highly relevant, but this is more a question for that feature in Trillian.

I will get buy-in from the team and then look at the PRs related to this issue in the next day or two.