Incorrect parsing of SSL certificates and keys in ChaincodeBase in external launcher case

[emc2]

ChaincodeBase attempts to parse SSL certificates and keys provided by an external launcher in the following manner:

     final SslContext createSSLContext() throws IOException {
        final byte[] ckb = Files.readAllBytes(Paths.get(this.tlsClientKeyPath));
        final byte[] ccb = Files.readAllBytes(Paths.get(this.tlsClientCertPath));

         return GrpcSslContexts.forClient().trustManager(new File(this.tlsClientRootCertPath))
                .keyManager(new ByteArrayInputStream(Base64.getDecoder().decode(ccb)),
                        new ByteArrayInputStream(Base64.getDecoder().decode(ckb)))
                 .build();
     }

This fails, because the certs deployed by the external builder are in PEM format, but the code attempts to Base64 decode them.

The fix is obvious, and I have used it successfully in testing (however, I would have to jump through a bunch of hoops to contribute it, so someone else should probably do it).

[mbwhite]

Hello @emc2. you're correct that throughout Fabric there are two ways of handling certificates. And it is not always clear which is needed where.

Which external builder are you using?

Just on a 'admin' point - we're keen to get contributors and would like to limit the 'hoops' needed? Can you suggest what blockers we could look at removing? Thanks

[emc2]

We started out using the example Java external builder/launcher (I forget where exactly, but there's a boilerplate example). We later modified it so that we can just directly deploy JARs, but the issue is the same. It sets up certs in a temporary directory, and they are in PEM format.

As to the 'hoops', that's got to do with my org (research lab). We are in fact hoping to make contributions, but there are various checks, sponsor approval, etc. that we have to clear.

[mbwhite]

I understand the issues with 'hoops' :-)

Problem with the certificates is that when started by the peer the certificates get passed as base64, hence why there are a multitude of options. Not great I freely admit. For example

       LOGGER.info("CORE_PEER_TLS_ROOTCERT_FILE: " + this.tlsClientRootCertPath);
       LOGGER.info("CORE_TLS_CLIENT_KEY_PATH: " + this.tlsClientKeyPath);
       LOGGER.info("CORE_TLS_CLIENT_CERT_PATH: " + this.tlsClientCertPath);
       LOGGER.info("CORE_TLS_CLIENT_KEY_FILE: " + this.tlsClientKeyFile);
       LOGGER.info("CORE_TLS_CLIENT_CERT_FILE: " + this.tlsClientCertFile);

Think the best thing is to permit each form to be used, but have the PEM version have precedence if both specified.

On the subject of JAR files - were you aware that without using an external builder, you can supply a fully built JAR? It does get run in the fabric-javaenv docker image, so that might not match your requirements as to Java version.

Alternatively, chaincode-as-a-service gives you complete freedom in how the chaincode is run.

[emc2]

All of our work explicitly avoids the use of docker, for a number of reasons. We are using the external builder/launcher, installing with basic OS packages (which we have built), and doing all the setup and config with ansible.

At least the version of peer (2.2.4) that we are using drops regular PEM-encoded certs into a temp directory when it calls the external builder/launcher scripts. I verified this by modifying the scripts, and a patch that has trustManager and keyManager read in the files directly (with no base64 decoding) gets things working for us.

[mbwhite]

@emc2 and @jt-nti I've coded up PR https://github.com/hyperledger/fabric-chaincode-java/pull/222 to try and make this easier to use.

Thinking more about it however, I wonder if that PR is the wrong approach.

• We know the code works, when the certificates are in the correct format. (for example https://github.com/IBM-Blockchain/microfab/blob/e30f0ef517e24971399e04d606fd64aa8f270032/builders/java/bin/run#L22 is an example of how I changed Microfab's external builder. I need to push this back to the documentation I think)
• The PR still has the two 'file'/'path' options; it would be much better I think to have a single property that could be either, and the code checks what the file actually contains. Though this is a change of behaviour

Thoughts welcome!

[mbwhite]

FYI _ I've added the tag of help-wanted... I agree that the chaincodes should be consistent in the their approach to certificates... but personally I've not got the resources to make this update myself...