Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 44-byte long constraint for --privacy-public-key-file so Besu could support Tessera's EC encryptor #3819

Closed
freemanzMrojo opened this issue May 11, 2022 · 1 comment · Fixed by #4086
Assignees

Comments

@freemanzMrojo
Copy link
Contributor

freemanzMrojo commented May 11, 2022

Description

As a user, I want to remove the 44-byte long constraint for --privacy-public-key-file so that I can use Tessera's EC encryptor configuration.

Tessera currently supports the configuration of uncompressed SECP256r1 public keys in base64 (like MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE+NYbGgkOxL7LWJh/68eCsHMOvxydvNO4gck/0KFsoCtkyCS3O7TuYfWwk9tr/2WKAsd3/v1a+XGffwnahB1PEw==). This value is not currently supported by Besu for the attribute --privacy-public-key-file since it expects a value that is 44 chars long.

The purpose of this ticket is to get rid of that constraint since potentially Tessera could be configured with any JCA implementation and adapt the acceptance tests for privacy so we can use alternativa encryptors to NaCl (this time would be EC).

About the acceptance tests, we would need to adapt them somehow since web3j's Base64String has the same problem. Some of the possible options would be:

  1. Create another implementation for privacy of the org.web3j.tx.TransactionManager interface within the acceptance-tests package in Besu so we do not use Base64String. The problem with this option is that we will encounter the same issue when trying to submit a private transaction from a "blockchain service" to Besu. It would solve the problem within the acceptance tests though.

  2. Modify org.web3j.tx.PrivateTransactionManager so it uses something different to Base64String in the constructor. This is not an ideal approach IMO since I do not really know who is already using PrivateTransactionManager outside of Besu (within Besu though it looks like it is just used for the acceptance tests).

  3. (MY PREFERRED APPROACH) Modify Base64String so it does not check that length of 44, similarly to what is suggested in this issue for Besu (the logic is the same, sometimes the enclave public key in base64 is longer than 44). All the current methods will behave the same way regarding arguments and method outcomes. I have suggested this change already in Discord following web3j's contribution guidelines. This is my branch with the change applied. I have exported the package into my Besu branch to test it with good results overall in the acceptance tests locally (still need to check some errors).

Acceptance Criteria

  • It is possible to provide uncompressed SECP256r1 base64 public key as the content for the file passed as --privacy-public-key-file attribute.
  • The acceptance tests are adapted so they accept Tessera's EC encryptor configuration.
@NicolasMassart NicolasMassart added the doc-change-required Indicates an issue or PR that requires doc to be updated label May 11, 2022
@freemanzMrojo
Copy link
Contributor Author

Hi @NicolasMassart , I have adapted the acceptance tests but the ones related to this feature are failing.

On our side, it is ok if the EC encryptor does not work along with this feature, and I could accommodate the acceptance tests to run on the NaCl encryptor only (like nowadays). I have gone down the rabbit hole for the flexible privacy ones and it looks like we would need to adapt the management smart contracts to make them work since they use bytes32 for the enclave public keys.

Let me know what you guys think but IMO maybe that part should be included in a separate PR (or maybe do not implement it at all since the method signatures for the proxy contract include bytes32, so it would not be straightforward for running private Besu blockchains to migrate to the new contract version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants