fabric
fabric copied to clipboard
hyperledger
Add ed25519 support
Type of change
- New feature
Description
Due to some considering NIST curves insecure and Golang having ed25519 native support, there was not a reason for not implementing in Fabric.
Tests cases for ed25519 were also added. Since ed25519 key derivation is not called by any function, I left as a TODO.
As I am working on ed25519 support for node fabric-gateway, I needed to add ed25519 support for cryptogen also, aiming to pass tests with certificates containing ed25519 keys. Since the node fabric-gateway tests generate their crypto material with cryptogen, I adapted cryptogen to support ed25519 keys.
Additional details
Some tests concerning ed25519 key generation and certificate parsing were added.
Related issues
I generally support this idea, and am in favor of it, but unfortunately adding support for ed25519 in a safe manner is more complex than just adding the code for generating signatures and verifying them.
If you could atomically and instantly upgrade all nodes of a Fabric network to support ed25519 then there wouldn't have been any problem at all.
But Fabric nodes may be upgraded at different times, and some nodes might be offline or unreachable and then afterwards be reachable and online again and these nodes will receive the ed25519 signatures and they will reject them. On the other hand, the nodes that were upgraded, will not reject them, and you will have a fork in the blockchain.
The key problem is that we need a way for Fabric validation to understand that "until a certain point in the blockchain, all ed25519 signatures are invalid" and "starting from a certain point in the blockchain, all ed25519 signatures are valid".
In Fabric, this is done using the capabilities feature. Essentially, you "mark" in the channel config a certain configuration property and then the nodes know when to use that property and when not to use it.
Aside from my comment above, the PR is missing integration tests that generate ed25519 crypto material, and run a network and transact to ensure everything is good.
Incidentally, have you checked that the verification takes care of
https://datatracker.ietf.org/doc/html/rfc8032#section-8.4
We had to do this manually for ecdsa
Incidentally, have you checked that the verification takes care of
https://datatracker.ietf.org/doc/html/rfc8032#section-8.4
We had to do this manually for ecdsa
It seems that golang has already implemented it:
https://github.com/golang/go/blob/4e79f06dac712d35d67d72777dae6df6d8c97888/src/crypto/ed25519/ed25519.go#L220
https://github.com/golang/go/blob/4e79f06dac712d35d67d72777dae6df6d8c97888/src/crypto/ed25519/ed25519_test.go#L165
In previous versions they even had a comment about preventing signature malleability:
https://github.com/golang/go/blob/go1.15/src/crypto/ed25519/ed25519.go#L235
@johannww On ed25519 support for node fabric-gateway... I'm sure you already know the signing implementation is entirely pluggable. The client application can supply its own (ed25519) signing implementation and the API doesn't need to have an implementation built in. However, I would be quite happy to include this in the API. Just note that there need to be implementations in all three client languages (Go, Type/JavaScript and Java).
Ok i understand this is still work in progress. So please tag me once you feel this is ready. Take into account that it will take quite a while for this to be merged, and also even more time until this will be included into any release, because if it will go to any release it will be to Fabric v3.0
One important issue:
I am implementing Ed25519 support for fabric-gateway using the code in this pull request as my Hyperledger Fabric. I created the crypto-material by setting field PublicKeyAlgorithm to ed25519, instead of ecdsa as here:
https://github.com/johannww/fabric-gateway/blob/81f5ee44f37f7148500f1039e5b33bd4f17d8f4d/scenario/fixtures/crypto-material/crypto-config.yaml#L41-L45
This generated ed25519 TLS keys for the peers. When running the scenario-test, only the java-gateway failed in a scenario with ed25519 TLS keys, due to the library netty (see the issue). I see 2 possible ways to go from here, regarding changes to fabric and/or gateway:
- Disable ed25519 certificates for TLS explicitly in fabric and prevent cryptogen from generating ed25519 TLS certificates.
- Ignore Java limitation and issue a warning when errors related to the grpc and netty happen. Count on netty developers to implement ed25519 support soon.
- Any other suggestions?
When running the scenario-test, only the java-gateway failed in a scenario with ed25519 TLS keys, due to the library netty (see the issue).
I noticed this JDK issue, which says TLS support for EdDSA signatures was added in Java 16:
https://bugs.openjdk.org/browse/JDK-8254709
Any chance that it worked with Java 17?
I see 2 possible ways to go from here, regarding changes to fabric and/or gateway:
- Disable ed25519 certificates for TLS explicitly in fabric and prevent cryptogen from generating ed25519 TLS certificates.
- Ignore Java limitation and issue a warning when errors related to the grpc and netty happen. Count on netty developers to implement ed25519 support soon.
- Any other suggestions?
I think I would be happy to proceed with only Java clients not currently able to establish TLS connections to peers that use Ed25519 keys. There is precedent for certain crypto only being supported by certain client, such as Idemix. This is clearly a bug outside of our control and should get fixed at some point.
One important issue:
I am implementing Ed25519 support for fabric-gateway using the code in this pull request as my Hyperledger Fabric. I created the crypto-material by setting field PublicKeyAlgorithm to ed25519, instead of ecdsa as here:
https://github.com/johannww/fabric-gateway/blob/81f5ee44f37f7148500f1039e5b33bd4f17d8f4d/scenario/fixtures/crypto-material/crypto-config.yaml#L41-L45
This generated ed25519 TLS keys for the peers. When running the scenario-test, only the java-gateway failed in a scenario with ed25519 TLS keys, due to the library netty (see the issue). I see 2 possible ways to go from here, regarding changes to fabric and/or gateway:
1. Disable ed25519 certificates for TLS explicitly in fabric and prevent cryptogen from generating ed25519 TLS certificates. 2. Ignore Java limitation and issue a warning when errors related to the grpc and netty happen. Count on netty developers to implement ed25519 support soon. 3. Any other suggestions?
Fabric's TLS stack is generally up to the Go standard library, so if you use a Java client willingly, it is up to you to be able to connect correctly to the TLS stack of the nodes.
Generally, as a rule of thumb - Fabric core sets the standard, and the clients track the Fabric core, and not the other way around.
From my perspective it is ready.On 23 Dec 2022, at 08:16, yacovm @.***> wrote: @johannww what's the status of this? is this still work in progress?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
@johannww I'm not a maintainer of this repository so I don't know if there is any other reason this PR has not been reviewed, but it does show as having merge conflicts with the main branch that need to be resolved.
Thanks for resolving the conflicts.
@yacovm Could you take another look at this one?
Thanks for resolving the conflicts.
@yacovm Could you take another look at this one?
I want to merge this PR very much, as eddsa has additional benefits beyond having another signature algorithm - for example it has a much faster threshold signature algorithm than ECDSA.
I'm a bit overloaded this week and the next, but I will try to find some time.
For the fabric-gateway, I think it is better to wait for the release 3.0. It seems that a lot has changed (ubuntu docker images/fabric-tools does not have go binary anymore). The main ed25519 implementations are done, but I will wait for the gateway to catch up to Fabric 3.0 to offer the pull request.
For the fabric-gateway, I think it is better to wait for the release 3.0. It seems that a lot has changed (ubuntu docker images/fabric-tools does not have go binary anymore). The main ed25519 implementations are done, but I will wait for the gateway to catch up to Fabric 3.0 to offer the pull request.
I don't really understand the motivation for waiting for Fabric v3, and why there was no desire to include this in the Fabric v2.5 LTS release. What is the blocker to both reviewing and delivering this PR?
Also, note that Go binary has recently been re-added to fabric-tools image in main branch and release-2.5 branches - https://github.com/hyperledger/fabric/pull/4176.
I agree that core Fabric support can lead SDK support.
@johannww I tried to test your pull request with the recent release of fabric-gateway v1.3.0 (@bestbeforetoday ) which added support for Ed25519 signer implementations but I had no success. Here is what I did:
I used the asset-transfer-basic from fabric-samples repository to test. I mainly used the default settings. The only thing I changed was in the file fabric-samples/test-network/organizations/cryptogen/crypto-config-org1.yaml:
PeerOrgs:
- Name: Org1
Domain: org1.example.com
EnableNodeOUs: true
Template:
Count: 1
SANS:
- localhost
Users:
Count: 1
PublicKeyAlgorithm: ed25519
I used the following commands (inside the test-network folder) to create a channel and deploy it:
export FABRIC_CFG_PATH=$PWD/../config/
export CORE_PEER_TLS_ENABLED=true
export CORE_PEER_LOCALMSPID="Org1MSP"
export CORE_PEER_TLS_ROOTCERT_FILE=${PWD}/organizations/peerOrganizations/org1.example.com/peers/peer0.org1.example.com/tls/ca.crt
export CORE_PEER_MSPCONFIGPATH=${PWD}/organizations/peerOrganizations/org1.example.com/users/[email protected]/msp
export CORE_PEER_ADDRESS=localhost:7051
export CORE_VM_DOCKER_ATTACHSTDOUT=true
./network.sh down
./network.sh up createChannel -c mychannel
./network.sh deployCC -ccn basic -ccp ../asset-transfer-basic/chaincode-go -ccl go
After deploying the channel, I compiled and ran the assetTransfer application from fabric-samples/asset-transfer-basic/application-gateway-go/assetTransfer.go folder. In the initLedger step it fails with the following error:
failed to submit transaction: rpc error: code = Aborted desc = failed to endorse transaction, see attached details for more info.
check if creator is a valid certificate - access denied: channel [mychannel] creator org [Org1MSP]: could not validate identity's public key algorithm: Ed25519 is not supported.
Could you please help me out here if I am missing some configuration?
@mkhattat, I don't have much experience with the test network, but you should look here in the config.tx file. You should replace "V2_0" for "V3_0", because ed25519 is, for now, a fabric 3.0 feature. This might change, as discussed here in this pull request.
@mkhattat Did you have any luck getting the asset-transfer-basic sample to with with the Fabric configuration change suggested by @johannww above?
One other thing to note is the hash expectations of different signature algorithms, documented here.
Note that the Sign implementations have different expectations on the input data supplied to them.
The ECDSA signers operate on a pre-computed message digest, and should be combined with an appropriate hash algorithm. P-256 is typically used with a SHA-256 hash, and P-384 is typically used with a SHA-384 hash.
The Ed25519 signer operates on the full message content, and should be combined with a NONE (or no-op) hash implementation to ensure the complete message is passed to the signer.
By default the client API applies a SHA-256 hash to the message to create a message digest, and that message digest is passed to the signer. This is the desired behaviour for an ECDSA signer, which operates on a pre-computed digest. The Ed25519 signer computes the digest as part of the signing operation so needs to receive the complete message bytes.
To use an Ed25519 signer, it is not sufficient to only provide an Ed25519 signing implementation as an option to Connect. You must also provide the WithHash option with a NONE hash implementation. This causes the unaltered message bytes to be passed to the Ed25519 signing implementation instead of a pre-computed digest.
@hyperledger/fabric-core-maintainers Now that Fabric v3 is the next scheduled release from the main branch codebase, what is the blocker to this PR being reviewed and merged? This submission has been open for approaching 2 years so it would be good to make some kind of decision on what is to be done with it.
Complementing the @bestbeforetoday request, let me clarify the importance of this pull request for me and Hyperledger Fabric. I implemented the code in this pull request for a Brazilian Government project I work for. Brazil's public-key infrastructure does not allow NIST curves anymore. The first option was to adopt the Brainpool curves, but Go did not implement it. Thus, we opted to implement ed25519 support and offer this pull request.
Generally, I believe that supporting ed25519 can expand Hyperledger Fabric's adoption for similar cases. I guess that there are many other valid incentives. For instance, the Brazilian CBDC pilot is using Hyperledger Besu. Consider that, as I believe it is in your and the community's best interest to evaluate this pull request.
Thanks for not giving up on this PR. I did a superficial pass and gave some comments.
Please rebase on top of latest main and let's also have @ale-linux and @adecaro and @denyeart take a look at this.
By the way, what about TLS? Have you tried spinning up a node with TLS certificates containing ed25519 keys? I am sure it should be possible as we don't do anything special in our TLS stack regarding keys, but I would like to ask that regardless.
By the way, what about TLS? Have you tried spinning up a node with TLS certificates containing ed25519 keys? I am sure it should be possible as we don't do anything special in our TLS stack regarding keys, but I would like to ask that regardless.
Testing with Ed25519 TLS was mentioned in this comment above. The only issue mentioned was with Java client applications due to a bug / limitation in the Netty library used by gRPC-Java.
By the way, what about TLS? Have you tried spinning up a node with TLS certificates containing ed25519 keys? I am sure it should be possible as we don't do anything special in our TLS stack regarding keys, but I would like to ask that regardless.
Testing with Ed25519 TLS was mentioned in this comment above. The only issue mentioned was with Java client applications due to a bug / limitation in the Netty library used by gRPC-Java.
oh I totally forgot about it, I even replied to the comment :-)
I still concur with Yacov from the past:
Generally, as a rule of thumb - Fabric core sets the standard, and the clients track the Fabric core, and not the other way around.
@johannww likely because other changes have been merged in the meantime but this PR is showing as having merge conflicts that need to be resolved. You might need to rebase your change on the main branch and force-push an updated commit. Hopefully then @yacovm can review.
I can review but I think it's also good if @adecaro and @ale-linux will both take a look :-)
@bestbeforetoday I guess I will have to open a pull request in fabric-lib-go also. The deleted bccsp folder seems to be the only problem.
I rebased on main branch again. Notice that my bccsp modifications were moved to the fabric-lib-go vendor folder but the changes were not accepted at fabric-lib-go repo yet. A pull request was opened there.