ed25519 signing and verification using a Curve25519 key-pair

[rafalsk]

Implemented functionallity which allows to perform ed25519 signing and verification using a Curve25519 key-pair.

Usage:
`
//sign
Botan::PK_Signer signer(prevPrivKey25519, rng, "Pure");
signer.update(msgv);
std::vector<uint8_t> signature2 = signer.signature(rng);

//verify
Botan::PK_Verifier verifier(pubKey25519, "Pure");
verifier.update(msgv);
bool valid = verifier.check_signature(signature2);`

[randombit]

Thanks! It will take a while for me to review this. In the meantime can you check the CI failures, and also reformat? It appears some functions (like ge_scalarmult_base) got reindented. So it is hard to see what actually changed. There is a configuration for the astyle formatter (http://astyle.sourceforge.net/) in src/configs

[rafalsk]

yes; I'll be taking a look into these matters.

[codecov-io]

Codecov Report

Merging #1239 into master will decrease coverage by 0.31%.
The diff coverage is 32.51%.

@@            Coverage Diff             @@
##           master    #1239      +/-   ##
==========================================
- Coverage   92.29%   91.97%   -0.32%     
==========================================
  Files         547      752     +205     
  Lines       59470    45131   -14339     
  Branches     6262     5217    -1045     
==========================================
- Hits        54888    41510   -13378     
+ Misses       4582     3587     -995     
- Partials        0       34      +34

Impacted Files	Coverage Δ
src/lib/pubkey/curve25519/curve25519.h	100% <ø> (ø)
src/lib/pubkey/curve25519/curve25519.cpp	31.67% <0%> (-66.66%)	⬇️
src/lib/pubkey/ed25519/ed25519_fe.h	88.37% <0%> (ø)
src/lib/pubkey/ed25519/ge.cpp	99.69% <100%> (-0.31%)	⬇️
src/lib/pubkey/ed25519/ed25519_internal.h	100% <100%> (ø)
src/lib/filters/basefilt.cpp	55.55% <0%> (-44.45%)	⬇️
src/lib/tls/msg_cert_status.cpp	36% <0%> (-42.58%)	⬇️
src/cli/cli.h	59.71% <0%> (-40.29%)	⬇️
src/lib/utils/cpuid/cpuid.cpp	57.77% <0%> (-31.88%)	⬇️
src/lib/utils/version.cpp	70% <0%> (-30%)	⬇️
... and 844 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f789d82...b40de80. Read the comment docs.

[randombit]

@rafalsk Sorry about taking so long to review this.

Big concern, do we even want this in the library? This is a documented approach for signing with x25519 (https://moderncrypto.org/mail-archive/curves/2014/000205.html) but is it implemented by any other library? If we're going to offer such conversions, going in the ed25519 to x25519 direction seems simpler and is implemented by libsodium (https://download.libsodium.org/doc/advanced/ed25519-curve25519.html) and its various wrappers. That's especially a concern as (AIUI) this signing approach is not compatible with standard Ed25519 signatures (please correct me If I'm wrong about this.)

Assuming we do want it in the library:

• There are no tests, these need to be added, preferably with vectors created by an independent implementation.
• This creates a new internal dependency where x25519 uses Ed25519. This should be optional, with the signing operations disabled if Ed25519 is not included in the build.
• This adds functions ed25519_verify_modified and ed25519_sign_modified - how are they modified? Why can't these just do the conversion and call the versions in Ed25519? At the very least there should be a comment clearly explaining this, but overall I'd be a lot happier with the EdDSA algorithm not being duplicated in two places with slightly different implementations.

There are also minor style things (used of C APIs like malloc+free, not using stdint typedefs) but no point addressing those until the larger issues are cleared up.

[rafalsk]

Yes, yes we do want this into the library; It is a hot feature. None has it - we got it. The proliferation of x25519 keys seems to be much higher; I know I could go the easier way doing ed25519 to x25519. I hear you in regard to all 3 points you've made. I'll document everything in detail and implement all the tests etc. - at the beginning of January; I'm too busy right now. Hope to make it during the first week.

[deemru]

These kind of signatures are used for example at @wavesplatform and it is true that it is hard to find a comprehensive solution to implement a full stack of crypto things by a single library.

[rafalsk]

I'm still on the subject. I'll try to find time to document and rationalize everything which has been implemented by me here in weeks to follow. I already do use the proposed functionality.

[deemru]

Just tried your rafalsk:ed25519sigWithCurve25519, an example of Waves transaction signing did worked for me on verify their etalon signature. That is nice.

Secondly, ed25519_verify works the same but much better by design against ed25519_verify_modified, is there a reason for _modified versions?

[rafalsk]

@deemru there was a reason; I promise to dive into this in days to follow. We'll need to bring this into production on hardware devices soon; so I'll need to verify this once again.

[codecov-commenter]

Codecov Report

Merging #1239 into master will increase coverage by 0.17%.
The diff coverage is 32.51%.

@@            Coverage Diff             @@
##           master    #1239      +/-   ##
==========================================
+ Coverage   91.79%   91.97%   +0.17%     
==========================================
  Files         752      752              
  Lines       45021    45131     +110     
  Branches     5204     5217      +13     
==========================================
+ Hits        41329    41510     +181     
+ Misses       3658     3587      -71     
  Partials       34       34              

Impacted Files	Coverage Δ
src/lib/pubkey/curve25519/curve25519.cpp	31.67% <0.00%> (-54.77%)	⬇️
src/lib/pubkey/curve25519/curve25519.h	100.00% <ø> (ø)
src/lib/pubkey/ed25519/ed25519_fe.h	88.37% <0.00%> (-9.07%)	⬇️
src/lib/pubkey/ed25519/ed25519_internal.h	100.00% <100.00%> (ø)
src/lib/pubkey/ed25519/ge.cpp	99.69% <100.00%> (-0.01%)	⬇️
src/lib/tls/msg_certificate.cpp	97.05% <0.00%> (-2.95%)	⬇️
src/tests/unit_tls.cpp	90.88% <0.00%> (-0.64%)	⬇️
src/lib/tls/tls_handshake_state.cpp	83.18% <0.00%> (-0.46%)	⬇️
src/lib/tls/tls_channel.cpp	79.10% <0.00%> (-0.35%)	⬇️
src/lib/tls/tls_client.cpp	85.40% <0.00%> (ø)
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9518529...b40de80. Read the comment docs.

[rafalsk]

The thing is at the heart of our project https://twitter.com/gridnetproject, it's been all validated, we've got a version upgraded to the recent version of Botan running locally, as soon as I find some time I'll update this. never failed on us, passing all tests.

[randombit]

Supplanted by #2774