crossbario / autobahn-cpp

WAMP for C++ in Boost/Asio
https://crossbar.io/autobahn
Boost Software License 1.0
251 stars 104 forks source link

Add basic cryptosign support. #231

Closed aarlt closed 2 years ago

aarlt commented 2 years ago

I added basic cryptosign support. I added a simple example examples/cryptosign.cpp that uses Botan to generate the ed25519 signature. This example is based on examples/wampcra.cpp and examples/websocket_callee.

I also prepared the usage of channel_id, but this is not used in the example. I also did not check the behaviour of the channel_id part, e.g. maybe it makes not that much sense to define/parse it as a string.

Also in wamp_session::join I added authentication_extra as a map<string, string>. I'm not sure whether this makes sense in all cases.

However, let me know what you think about this PR. I know that this is not perfect yet and that some changes might be needed. I just wanted to push the current state to get some feedback.

aarlt commented 2 years ago

I also added an example using OpenSSL examples/cryptosign-openssl.cpp. The Botan example examples/cryptosign-botan.cpp will only be build, if AUTOBAHN_BUILD_EXAMPLES_BOTAN is set. Maybe we could also just remove the Botan example.

oberstet commented 2 years ago

First of all, this is awesome! Thanks for contributing=)

Having WAMP-cryptosign in AutobahnC++ is definitely closing an important gap. WAMP-cryptosign is a secure, modern and relatively simple authentication method.

"Relatively", as when reusing a proper cryptographic library for primitives, and the choice of Botan for that is hence the crucial one. I had used Botan in the past, it is great! In particular, compared to OpenSSL;) And in the context of C++.

Basically the only question I have (rgd the choice) is only slightly related to this specific PR: would Botan also allow us to later (different PR / thing) add WAMP end-to-end encryption support?

The primitives we would need for that are:

  1. [x] NaCl cryptobox support https://nacl.cr.yp.to/box.html (WAMP-cryptosign uses NaCl primitives and scheme as well) - Salsa20 in Botan, poly1305, curve25519
  2. [x] Ethereum Keccak-256 signatures (in Botan)
  3. [ ] Ethereum EIP712 typed data (specifically, "eth_signTypedData_v4")

crypto_box is curve25519xsalsa20poly1305, a particular combination of Curve25519, Salsa20, and Poly1305 specified in "Cryptography in NaCl". This function is conjectured to meet the standard notions of privacy and third-party unforgeability.

Now, of these 3 I do know https://doc.libsodium.org/ supports 1 (https://doc.libsodium.org/public-key_cryptography/authenticated_encryption)., and libsodium is a very clean, light weight, focused reimplementation of what NaCl provides.

I haven't looked much yet for 2. and 3. in C++.

What I'm trying to get at: we might want to make the choice of an additional dependency for cryptography (besides what comes as part of the TLS support) to support as much as possible ...

I think it is worth discussing and make a conscious choice, so my comments are not regarding lines of code (which look good btw from quick look), but more about project governance, dependencies and such .. what do you think?

oberstet commented 2 years ago

Let me just underline: your PR is awesome, and it fills a gap! I am not trying to add more unrelated work (as WAMP end-to-end encryption) to that, I just want us to make a conscious decision - looking forward.

Finally, just skimmed through the existing issues, these two are also in the context of "cryptographic dependencies" - just for completeness:

https://github.com/crossbario/autobahn-cpp/issues/125 https://github.com/crossbario/autobahn-cpp/issues/126

oberstet commented 2 years ago

the one failing windows build/test in CI: "warning LNK4099: PDB 'ossl_static.pdb' was not found" .. sounds like the linker is missing sth for 64 bits? not sure

oberstet commented 2 years ago

alright, so as far as I see:

aarlt commented 2 years ago

Thanks for giving me a sense of the broader vision behind WAMP, autobahn & crossbar.io. I just recently discovered WAMP - so I was really not aware of its future steps and related ideas. This really looks awesome!

I was reading some of the historic issues regarding end-to-end encryption (https://github.com/wamp-proto/wamp-proto/issues/81) & payload transparency (https://github.com/wamp-proto/wamp-proto/issues/228). I also think that https://github.com/wamp-proto/wamp-proto/issues/298 is important to be able to support future algorithms.

From my point of view especially end-to-end encryption is very important.

However, for autobahn-cpp I would say we should definitely use Botan. It is not just supporting keccak256, secp256k1 & the needed NaCl primitives. I also just discovered it - Botan provides a libsodium compatibility shim https://github.com/randombit/botan/tree/master/src/lib/compat/sodium. That means, it is not just supporting the primitives salsa20, poly1305 and curve25519 - it seem to provide all needed cryptobox highlevel functions, e.g. crypto_secretbox_xsalsa20poly1305. It looks like that authenticated encryption is already available in Botan.

I was adding an OpenSSL example of cryptosign, because I saw that this was an already existing dependency. After realising that this is only used for HMAC in wamp_auth_utils.hpp, I guess it would make totally sense to replace OpenSSL with Botan. It looks like that Botan is perfectly suited for future versions of autobahn-cpp.

For the channel_id part in this PR I would need to read more about it. It would be nice to have somehow a testing environment available in autobahn-cpp. How do you normally test autobahn-cpp?

With regards to EIP712, with Botan we have the cryptographic primitives keccak256 & secp256k1 signature algorithms available. Maybe we just need a ABI encoder/decoder.

For what exactly are you using/plan to use EIP-712?

aarlt commented 2 years ago

the one failing windows build/test in CI: "warning LNK4099: PDB 'ossl_static.pdb' was not found" .. sounds like the linker is missing sth for 64 bits? not sure

I think this is related how OpenSSL was build/installed. From where is OpenSSL coming from? I did not see anything in the GitHub workflow.

But I don't understand why this problem just popped up in my PR.

oberstet commented 2 years ago

let me answer in 2 parts, this is everything "non-Botan":

... broader vision behind WAMP ...

great=) this is awesome! that you dug into the details, and also the issues you linked: very helpful!

actually you've found the core original itch: https://github.com/wamp-proto/wamp-proto/issues/81 for end-to-end encryption - wau, tbh, I didn't expect that;) personally, I think this is one of the few remaining areas in WAMP that are not completed yet. To complete the full original vision. fwiw, yes, the other issues you link are related to this, as they fill in the means to actually define the e2e stuff.

at this point of WAMP evolution, we have a complete list of the last areas we want to fill. It's been an incredible ride, and lots of work among WAMP implementors etc since WAMP v1 ... we are almost there;)

finally, if you want to know where WAMP is going in general, "the plan as it exists", this gives you the best and most current perspective:

both pretty hard core tech, and terse. I dare to just link those for you, as you know Ethereum and middleware .. just found your linkedin

Ethereum Foundation .... Design and Implementation of a Message Oriented Middleware

you are absolutely in the right place! we need you;)

But I don't understand why this problem just popped up in my PR.

yeah, me neither. I'd be fine with ignoring it for now (merge anyways).

on the one hand, it's a itch (what the heck is missing?), the other is the governance Q, as in "CI is not 100% green .. should we merge nevertheless?" and why we now are in a place where none of understands and has much motivation I guess to dig into windows: this was contributed by a developer, and I was like "awesome! lets merge", but now it lacks a proper maintainer:( I don't even have a windows box around. mmmh.

oberstet commented 2 years ago

Botan provides a libsodium compatibility shim ... It looks like that authenticated encryption is already available in Botan.

this is perfect!

However, for autobahn-cpp I would say we should definitely use Botan.

yes, agreed. let's use it then, depend on Botan.

which for this PR means: ready for merge, let me know (I'd be fine with win64 being red for now ..)

For what exactly are you using/plan to use EIP-712?

ok, pls let me explain. the pieces come together like this:

  1. WAMP-cryptobox provides real end-2-end encryption of WAMP application payloads (the args/kwargs in RPC and PubSub)
  2. The encryption is using NaCl authenticated encryption, and hence those primitives are needed
  3. To transport end-2-end encrypted app payload, instead of plain-text args/kwargs, we need WAMP payload transparency where instead of args/kwargs, the WAMP message has ciphertext/keyid (see from here https://github.com/crossbario/autobahn-python/blob/7becdaae12db7abee390dd86d4e247f5071b91ab/autobahn/wamp/cryptobox.py#L237)
  4. sidenote: WAMP payload transparency can also be used for other stuff, eg MQTT-WAMP bridging with transparent MQTT payload (https://github.com/crossbario/crossbar-examples/tree/master/mqtt/basic/passthrough)
  5. Now the next level is the actual key management of the app-payload encryption keys. A subscriber receiving an event with a keyid it doesn't have need to get key for keyid from the publisher - without exposing key to anyone else (https://github.com/crossbario/autobahn-python/blob/7becdaae12db7abee390dd86d4e247f5071b91ab/autobahn/xbr/_seller.py#L578)
  6. If I haven't lost you already, this is now the point where EIP712 comes into play. The subscriber calling into the publisher to get the key is a EIP712 signed transaction.
  7. Because of 5., the publisher/subscriber identities and authorities can be trust-rooted in Ethereum.
  8. And finally, that trust rooting extends to "consent": yes, me Alice has allowed my publishing app to publish my geo-location to this topic, and with that payload structure (app type in event data).
  9. So in this last step, Alice wants to give consent to her "WAMP delegates", app instances run by her, and for using or consuming specific WAMP APIs, eg see:

https://github.com/crossbario/xbr-protocol/blob/eae39f39f8522967752e042c17f5506dd9e9e4dd/contracts/XBRMarket.sol#L425 https://github.com/crossbario/autobahn-python/blob/7becdaae12db7abee390dd86d4e247f5071b91ab/autobahn/xbr/_eip712_consent.py#L137


Rereading my answer: I am sorry, this is a hard core ride:( I mean, it is true and honest, exactly where we are and what's the goal, but no worries if you wanted to skip all that.

This PR is great and concrete, ready for merge, I don't want to drag you into a black hole;)

oberstet commented 2 years ago

whatever, who needs windows;) merged. thanks for contributing!

aarlt commented 2 years ago

@oberstet ah nice! thanks for merging! Sorry, that I did not say anything yet to your additional input. But in general those ideas make a lot of sense. I could imagine to do some PRs from time to time - if I have time :)

As a next step I would like to add a test environment. For this test environment I have something in mind, where test-cases can be defined that will interact with a router (crossbar) that was specifically configured for those test-cases. I have a basic prototype ready, but I would like to add a testing step to the github workflow. I guess I will create a first PR for this maybe in a few hours (in the worst case during the next days).

om26er commented 2 years ago

I am making use of autobahn-cpp and was successfully able to get cryptosign working. One missing thing that I still haven't figured is, how can I get the private key from a hex string, an example for that would be helpful.

Currently private key seems to be generated on the fly https://github.com/crossbario/autobahn-cpp/blob/af6df8bb672c291e8c91b315e3759d1ef33a8d84/examples/cryptosign-openssl.cpp#L115

aarlt commented 2 years ago

@om26er nice! In general the private key is just a private key - a randomly generated 256 bit array that should kept private and should not be shared with anyone. In the example I just used a private key that is set to zero for all bytes.

You just need to be sure that you're generating the key with a save random generator algorithm, so that it is not possible to "guess" a generated private key. For OpenSSL this seem to be described here, e.g. you could use the RAND_bytes function.

If you want to give Botan a try, you just need to enable the Botan example by setting AUTOBAHN_BUILD_EXAMPLES_BOTAN, e.g. with cmake -DAUTOBAHN_BUILD_EXAMPLES_BOTAN=on ... Of course for this you need to have Botan installed in your system. Here you can initialise the private key by using secure_vector<uint8_t> Botan::RandomNumberGenerator::random_vec(size_t bytes), described here.

om26er commented 2 years ago

@aarlt in my test environment, I already have the private key in the form of a hex string and I need to be able to use that to authenticate. Our environment is a bit different and the private key is generated outside of the authenticating client. So I basically need to find a way to convert that hex string into a std::vector<uint8_t>

Disclaimer: my c++ skills are very limited, which might be the reason I haven't found a way already :-)

aarlt commented 2 years ago

@om26er if you use Botan, you could use Botan::hex_decode (see https://github.com/randombit/botan/blob/master/src/lib/codec/hex/hex.h#L118). I'm not sure that OpenSSL contains a helper function for doing this. If you can use Boost you could use boost::algorithm::unhex, e.g. with

std::vector<uint8_t> unhex(const std::string& hex)
{
    std::vector<uint8_t> bytes;
    boost::algorithm::unhex(hex, std::back_inserter(bytes));
    return bytes;
}

In general you want to convert a hex-string to a byte array (std::vector).

oberstet commented 2 years ago

yeah, the Botan based example https://github.com/crossbario/autobahn-cpp/blob/master/examples/cryptosign-botan.cpp already uses Botan::hex_decode, and I'd prefer Botan to OpenSSL in general - it's a great library with clean code ..

om26er commented 2 years ago

I was able to get it working with Botan. The issue was that it the current cryptosign implementation expects for Botan::hex_decode_locked to be called on the string that concatenate the private key hex and public key hex.