Gateway support for mutual TLS networks #3235
Conversation
internal/pkg/comm/config.go
Outdated
| @@ -164,6 +164,8 @@ type SecureOptions struct { | |||
| Certificate []byte | |||
| // PEM-encoded private key to be used for TLS communication | |||
| Key []byte | |||
| // The client certificate which, if specified, will be used instead of the PEM-encoded Certificate/Key pair | |||
There was a problem hiding this comment.
But why would you do this? Why have two different ways to construct the SecureOptions?
Until this PR, the entire code base used the SecureOptions as-is. Do we really have to expand this configuration structure and add another field that can be constructed from the existing fields?
| "github.com/tedsuo/ifrit" | ||
| ) | ||
|
|
||
| var _ = Describe("GatewayService with mutual TLS network", func() { |
There was a problem hiding this comment.
Do we really have to spin up an entire network, (with three Raft nodes....), to check that the gateway sends its TLS certificate to a peer or an orderer?
There was a problem hiding this comment.
It was probably overkill, but it gave me confidence I'd really fixed the problem. I've replaced it with a minimal unit test that spins up a grpc server with client auth enabled, and checks the gateway's endpointFactory can connect to it.
internal/pkg/gateway/gateway.go
Outdated
| @@ -76,13 +77,13 @@ func CreateServer(localEndorser peerproto.EndorserServer, discovery Discovery, p | |||
| return server | |||
| } | |||
|
|
|||
| func newServer(localEndorser peerproto.EndorserClient, discovery Discovery, finder CommitFinder, policy ACLChecker, ledgerProvider LedgerProvider, localPKIID common.PKIidType, localEndpoint, localMSPID string, options config.Options) *Server { | |||
| func newServer(localEndorser peerproto.EndorserClient, discovery Discovery, finder CommitFinder, policy ACLChecker, ledgerProvider LedgerProvider, localInfo gdiscovery.NetworkMember, localMSPID string, clientCert tls.Certificate, options config.Options) *Server { | |||
There was a problem hiding this comment.
Can we not test this with a unit test? We have plenty of TLS unit tests scattered around that test mutual TLS. They are much cheaper than to spin up an entire Fabric network.
To support client authentication (mTLS), the gateway needs to pass the host peer’s client certificate when making connections to the other nodes in the network. If client authentication is not enabled, then these certs will be ignored. Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
andrew-coleman
force-pushed
the
mutual_tls
branch
from
ba69562 to
3330620
Compare
| @@ -825,6 +825,7 @@ func serve(args []string) error { | |||
| serverEndorser, | |||
| discoveryService, | |||
| peerInstance, | |||
| &serverConfig.SecOpts, | |||
There was a problem hiding this comment.
Just one small thing - here you passed the server config, not the client config.
This would mean that you do not honor the tls.clientKey and tls.clientCert.
We probably need to check if the client TLS config is defined, and if so, then use it instead and only otherwise fallback on the server TLS config.
To support client authentication (mTLS), the gateway needs to pass the host peer’s client certificate when making connections to the other nodes in the network. If client authentication is not enabled, then these certs will be ignored. Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
|
@Mergifyio backport release-2.4 |
✅ Backports have been created
|
To support client authentication (mTLS), the gateway needs to pass the host peer’s client certificate when making connections to the other nodes in the network. If client authentication is not enabled, then these certs will be ignored. Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com> (cherry picked from commit 59835e6)
To support client authentication (mTLS), the gateway needs to pass the host peer’s client certificate when making connections to the other nodes in the network. If client authentication is not enabled, then these certs will be ignored. Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com> (cherry picked from commit 59835e6)
To support client authentication (mTLS), the gateway needs to pass the host peer’s client certificate when making connections to the other nodes in the network.
If client authentication is not enabled, then these certs will be ignored.
Resolves #3234
Signed-off-by: andrew-coleman andrew_coleman@uk.ibm.com