[FAB-18339] CLI: update GetCerificate to only load client certificate as it is supposed to be

[wenjianqiao]

Signed-off-by: Wenjian Qiao wenjianq@gmail.com

Type of change

• Bug fix

Description

Both "peer lifecycle chaincode" and "peer chaincode" commands need to get
the client certificate (when peer.tls.clientAuthRequired=true) for deliver service.
GetCertificate() returns a client certificate to pass to DeliverService. Currently, it
creates a PeerClient that unnecessarily loads server tls root file based on env vars,
which caused a problem loading tls files from env even if --tlsRootCertFile
already overwrites the env var.

The fix is to update GetCertificate to only load client certificate and rename it
to GetClientCertificate to avoid confusion.

Related issues

https://jira.hyperledger.org/browse/FAB-18339

[ale-linux]

Thanks @wenjianqiao, one Q: if we don't load server certs, how can the client be sure that it contacts the intended OS node?

[wenjianqiao]

@ale-linux Good question. GetCertificate should be called GetClientCertificate because it only returns the client's TLS certificate (the client cert is passed to deliver service when mutual TLS is enabled). Therefore it is unnecessary to load server certs in this function. The CLI commands already load the server certs in GetEndorserClient, and GetPeerDeliverClient, etc. I will rename GetCertificate to GetClientCertificate to avoid confusion.

[ale-linux]

@ale-linux Good question. GetCertificate should be called GetClientCertificate because it only returns the client's TLS certificate (the client cert is passed to deliver service when mutual TLS is enabled). Therefore it is unnecessary to load server certs in this function. The CLI commands already load the server certs in GetEndorserClient, and GetPeerDeliverClient, etc. I will rename GetCertificate to GetClientCertificate to avoid confusion.

Thanks @wenjianqiao, so the server certs are already available, and yet GetCertificate was loading them anyway

[wlahti]

A few small nits that should be quick/easy to address.

One thing that you didn't introduce to these tests that I think can be split out into a follow-up JIRA is to stop using cert fixtures and instead use tlsgen (here) to generate the certs/keys and write them to a temp folder.

[wlahti]

Thanks, Wenjian! Looks good to me (I also manually verified the behavior before/after the fix just to be sure). I'll open a follow-up task to move away from the cert fixtures in these tests.