hyperledger/fabric-sdk-java

release-2.2 mutual tls config key and peer property key not matching

irririki opened this issue · 5 comments

irririki commented

Here is the extraction from my connection profile:

 "peers":{
    "peer0": {
      "tlsCACerts": {
        "path": "./tls/tlscacerts/tlsca.pem",
        "client": {
          "certfile": "./tls/signcerts/sign.pem",
          "keyfile": "./tls/keystore/key.pem"
        }
      }, 
      "url": "grpcs://xx.xx.xx.xx:7051"
    }
  },

and here is the code:

Path walletDirectory = Paths.get("wallet");
Wallet wallet = Wallets.newFileSystemWallet(walletDirectory);

// Path to a common connection profile describing the network.
Path networkConfigFile = Paths.get("connection_profile.json");

// Configure the gateway connection used to access the network.
Gateway.Builder builder = Gateway.createBuilder()
        .identity(wallet, "admin")
        .networkConfig(networkConfigFile);

// Create a gateway connection
try (Gateway gateway = builder.connect()) {
 

    // Obtain a smart contract deployed on the network.
    Network network = gateway.getNetwork("dev");
    
    Contract contract = network.getContract("fabcar");

} catch (ContractException e) {
    e.printStackTrace();
}

The problem happens at 2 places. The first one is:

 .networkConfig(networkConfigFile);

where in NetworkConfig.java, it will put in props the key 'tlsClientKeyFile'.

if (keyfile != null) {
    props.put("tlsClientKeyFile", keyfile);
}

if (certfile != null) {
    props.put("tlsClientCertFile", certfile);
}

See

fabric-sdk-java/src/main/java/org/hyperledger/fabric/sdk/NetworkConfig.java

Lines 845 to 862 in 3dfc858

if (keyfile != null) {
props.put("tlsClientKeyFile", keyfile);
}
if (certfile != null) {
props.put("tlsClientCertFile", certfile);
}
String keyBytes = getJsonValueAsString(jsonTlsClientCerts.get("keyPem"));
String certBytes = getJsonValueAsString(jsonTlsClientCerts.get("certPem"));
if (keyBytes != null) {
props.put("tlsClientKeyBytes", keyBytes.getBytes());
}
if (certBytes != null) {
props.put("tlsClientCertBytes", certBytes.getBytes());
}

This change was introduced since v2.2.4.

Then it goes on to the line:

Network network = gateway.getNetwork("dev");

where, in Endpoint.java, it tries to find 'clientKeyFile' in the properties

if (properties.containsKey("clientKeyFile") || properties.containsKey("clientCertFile")) {

See

fabric-sdk-java/src/main/java/org/hyperledger/fabric/sdk/Endpoint.java

Lines 175 to 198 in 3dfc858

if (properties.containsKey("clientKeyFile") && properties.containsKey("clientKeyBytes")) {
throw new RuntimeException("Properties \"clientKeyFile\" and \"clientKeyBytes\" must cannot both be set");
} else if (properties.containsKey("clientCertFile") && properties.containsKey("clientCertBytes")) {
throw new RuntimeException("Properties \"clientCertFile\" and \"clientCertBytes\" must cannot both be set");
} else if (properties.containsKey("clientKeyFile") || properties.containsKey("clientCertFile")) {
if ((properties.getProperty("clientKeyFile") != null) && (properties.getProperty("clientCertFile") != null)) {
try {
logger.trace(format("Endpoint %s reading clientKeyFile: %s", url, properties.getProperty("clientKeyFile")));
ckb = Files.readAllBytes(Paths.get(properties.getProperty("clientKeyFile")));
logger.trace(format("Endpoint %s reading clientCertFile: %s", url, properties.getProperty("clientCertFile")));
ccb = Files.readAllBytes(Paths.get(properties.getProperty("clientCertFile")));
} catch (IOException e) {
throw new RuntimeException("Failed to parse TLS client key and/or cert", e);
}
} else {
throw new RuntimeException("Properties \"clientKeyFile\" and \"clientCertFile\" must both be set or both be null");
}
} else if (properties.containsKey("clientKeyBytes") || properties.containsKey("clientCertBytes")) {
ckb = (byte[]) properties.get("clientKeyBytes");
ccb = (byte[]) properties.get("clientCertBytes");
if ((ckb == null) || (ccb == null)) {
throw new RuntimeException("Properties \"clientKeyBytes\" and \"clientCertBytes\" must both be set or both be null");
}
}

Since the property has a different name, the method return null, which results on the peer side returning an error saying the client didn't send the certificate.

irririki commented

By changing the code in NetworkConfig.java to match the certFile and keyFile key names, it works. I have tested with my local fix.

if (keyfile != null) {
    props.put("clientKeyFile", keyfile);
}

if (certfile != null) {
    props.put("clientCertFile", certfile);
}
bestbeforetoday commented

This looks like something that has never worked, and perhaps wasn't intended to. The change you refer to just introduced some new functionality and didn't change any existing property naming set by NetworkConfig. It seems by happy coincidence that it would provide the capability you are looking for too though.

The Endpoint code is picking up properties set either explicitly if nodes are programmatically added, or from configuration and/or environment variables when nodes are added by service discovery, here:

  • Peer:

    fabric-sdk-java/src/main/java/org/hyperledger/fabric/sdk/Channel.java

    Lines 1696 to 1713 in 4d3cfc5

    String clientCertFile = (String) findClientProp(config, "clientCertFile", mspid, endpoint, null);
    byte[] clientCertBytes = (byte[]) findClientProp(config, "clientCertBytes", mspid, endpoint, null);
    if (null != clientCertBytes) {
    properties.put("clientCertBytes", clientCertBytes);
    } else if (null != clientCertFile) {
    properties.put("clientCertFile", clientCertFile);
    }
    properties.put(Peer.PEER_ORGANIZATION_MSPID_PROPERTY, sdPeerAddition.getMspId());
    byte[] clientKeyBytes = (byte[]) findClientProp(config, "clientKeyBytes", mspid, endpoint, null);
    String clientKeyFile = (String) findClientProp(config, "clientKeyFile", mspid, endpoint, null);
    if (null != clientKeyBytes) {
    properties.put("clientKeyBytes", clientKeyBytes);
    } else if (null != clientKeyFile) {
    properties.put("clientKeyFile", clientKeyFile);
    }
  • Orderer:

    fabric-sdk-java/src/main/java/org/hyperledger/fabric/sdk/Channel.java

    Lines 1630 to 1649 in 4d3cfc5

    String clientCertFile = (String) findClientProp(config, "clientCertFile", mspid, endpoint, null);
    if (null != clientCertFile) {
    properties.put("clientCertFile", clientCertFile);
    }
    String clientKeyFile = (String) findClientProp(config, "clientKeyFile", mspid, endpoint, null);
    if (null != clientKeyFile) {
    properties.put("clientKeyFile", clientKeyFile);
    }
    byte[] clientCertBytes = (byte[]) findClientProp(config, "clientCertBytes", mspid, endpoint, null);
    if (null != clientCertBytes) {
    properties.put("clientCertBytes", clientCertBytes);
    }
    byte[] clientKeyBytes = (byte[]) findClientProp(config, "clientKeyBytes", mspid, endpoint, null);
    if (null != clientKeyBytes) {
    properties.put("clientKeyBytes", clientKeyBytes);
    }

Your suggested enhancement seems reasonable, although I think "tlsClientKeyFile", "tlsClientCertFile", "tlsClientKeyBytes" and "tlsClientCertBytes" would all need to change to the forms without the leading "tls" to match the Endpoint code. Note that these properties are also used in HFClient, here:

fabric-sdk-java/src/main/java/org/hyperledger/fabric_ca/sdk/HFCAClient.java

Lines 1620 to 1630 in 4d3cfc5

String tlsClientKeyFile = properties.getProperty("tlsClientKeyFile");
String tlsClientCertFile = properties.getProperty("tlsClientCertFile");
byte[] tlsClientKeyAsBytes = (byte[]) properties.get("tlsClientKeyBytes");
if (tlsClientKeyFile != null && tlsClientKeyAsBytes != null) {
logger.warn("SSL CA client key is specified as bytes and as a file path. Using client key specified as bytes.");
}
if (tlsClientKeyFile != null && tlsClientKeyAsBytes == null) {
tlsClientKeyAsBytes = Files.readAllBytes(Paths.get(tlsClientKeyFile));
}
byte[] tlsClientCertAsBytes = (byte[]) properties.get("tlsClientCertBytes");

I think the changes required would be:

  • Change "tlsClient..." properties set in NetworkConfig to "client...".
  • Change the corresponding property names in HFCAClient.
  • Some unit testing (at least in NetworkConfigTest).
  • Extract the strings hard-coded in all these locations to internal constants, probably in Endpoint

Would you like to implement these changes?

irririki commented

Happy to do so.

  • ❤️1
irririki commented

@bestbeforetoday I have made the changes, but it seems that I can't create a new pull request. Could you have a look?

https://github.com/irririki/fabric-sdk-java-260

  • Change "tlsClient..." properties set in NetworkConfig to "client...".
  • Change the corresponding property names in HFCAClient.
  • Some unit testing (at least in NetworkConfigTest).
    NetworkConfigTest and HFCAClientTest.java
  • Extract the strings hard-coded in all these locations to internal constants, probably in Endpoint
    except in NetworkConfig and HFCAClient. I feel that it's better to pass the config around and define constants there, but that'd require some big changes.
[WARNING] Tests run: 444, Failures: 0, Errors: 0, Skipped: 5

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  35.443 s
[INFO] Finished at: 2023-02-07T12:14:03+09:00
[INFO] ------------------------------------------------------------------------
bestbeforetoday commented

I think the problem might be that your repository is not a fork if this repository. The repository with your changes needs to be a fork of the repository to which you want to propose changes by creating a pull request. See the GitHub documentation on how to fork a repo. Then the documentation on creating a pull request from a fork.