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

Get the protocol professionally audited #1

Open
griffindoors opened this issue Mar 3, 2017 · 9 comments
Open

Get the protocol professionally audited #1

griffindoors opened this issue Mar 3, 2017 · 9 comments

Comments

@griffindoors
Copy link

Has your encryption code been audited?

@tasn tasn changed the title awwwww... dit Get the protocol professionally audited Mar 3, 2017
@tasn
Copy link
Member

tasn commented Mar 3, 2017

Hey,

Thanks for asking. The answer to that is yes and no.
No, the protocol (including the crypto mechanism) have not been professionally audited, and as always with software, and especially when crypto is involved, mistakes could be made.
Yes it has in a sense because I haven't implemented any crypto myself, and I've used well tested "standard ways" of doing things. More specifically, I use BouncyCastle for the crypto implementation, use it to generate IV. Have a unique IV per message, use AES in CBC mode with PKCS7 padding. Use HMAC-256 for integrity checks. Maybe I messed something up, would love to get the code audited, but I can't afford to fork off that money at this point.

I updated the title to be more descriptive.

I hope everything is clear. More info is available on the FAQ.

@suqdiq
Copy link

suqdiq commented May 12, 2017

would you be open to changing the crypto to something like ChaCha20/XSalsa20+Poly1305?

meaning, accepting a PR that does it while being retro-compatible with previous crypto usage

@tasn
Copy link
Member

tasn commented May 12, 2017

@suqdiq, hey.

As a rule, I'm open to all changes for which a compelling case can be made.

Though I'm honestly asking, is there such a compelling case? I chose AES-CBC + HMAC-256 because of their ubiquity and battle-tested implementations. This means that I can find a good implementation (or bindings) for those for every platform and every language. This is essentially why I avoided ChaCha20. Though I'm willing to admit I maybe erred on the side of caution here, because ChaCha20 may have reached the same level of ubiquity by now.

If you think there is a compelling case to switching, please open a new ticket (let's not go off-topic on this one) and include the pros/cons of switching to ChaCha20/XSalsa20+Poly1305 and how would our users benefit from such a change. "Speed" is probably not a good enough reason, unless a case could be made that it would actually affect our users. Please also include a link to this comment so people have some context.

P.S, the EteSync protocol is versioned, so such a change should be fairly simple. It's having to implement it on every platform (at the moment there are only two, Python and Java), and having more "special cases" (more versions) that are annoying.

Thanks.

@x11x
Copy link

x11x commented Feb 26, 2019

I would like to suggest using libnacl or libsodium "secret box" functions instead of directly using the low-level primitives. High level libraries such as these seem to be security best practise these days as they insulate the developer from "sharp edges" like having to think about what AES mode and padding to use and so on, and keeping up to date with best practises. I think using the crypto primitives directly as etesync is currently would be considered "doing your own crypto" (which you rightly say you want to avoid).

I may have missed some of etesync's requirements in my reading of the code and FAQ item about the design, but I think a secret box function would cover the current requirements. e.g., using crypto_secretbox_easy takes just a key, a message and a nonce and spits out cipher text. I think it also takes care of padding, but would need to do a bit more research on this. I noticed there are also padding APIs included in the library. You can use the included RNG functions to generate the nonce. There are functions to derive a key from a password (all "keys" in the API are proper keys, not passwords). Libsodium uses Xsalsa20 and Poly1305 internally.

As for which libraries/bindings to choose, I can't recommend any from personal experience, but this page has a list. Maybe this one - libsodium-jni - for Android. There is an official javascript build (just the C compiled to JS using emscripten) https://github.com/jedisct1/libsodium.js/ which could be used in the web client. There are several Python options there for the Python API.

I've also seen libhydrogen, also from Frank Denis (author of libsodium), implements a secret box using curve25519 and GIMLI (don't know anything about the latter). Seems to be a lightweight version of libsodium aimed at embedded systems. I don't know if there are any reasons to use this over libsodium unless you want things to be especially lightweight.

Some further research on these options and which bindings/port to use would be necessary, but once this is decided, the developer experience is very easy and would cut out pretty much all of the current crypto code and just replace with a few function calls. My understanding of these libraries is they try to be very stable and oriented towards developers who have enough other things to worry about ("easy to use and hard to misuse") and could be a perfect fit for this project.

I like the way this project keeps things simple with clever design and good reuse of other popular libraries, now that these crypto libraries are available, this seems like the logical thing to use.

I should add, these libraries have been professionally audited -- so getting etesync itself audited can become a much lower priority, maybe even unnecessary.

@tasn
Copy link
Member

tasn commented Mar 6, 2019

Thanks for your detailed suggestion, while I agree with the above, it's not as easy as it sounds.

The main concerns with doing that are:

  1. Platform support - making sure it's available on all the platforms we currently support (Python, React Native, browser and Android) and will support in the future (iOS) - seems to be fine.
    1. Apps / methods compatibility - are these flexible enough to support e.g. OpenKeychain on Android for when we do Integration with OpenKeyChain for sharing journals #12?
    2. Another issue is for example YubiKey support. We chose RSA because many smartkeys didn't support EC at the time, is the landscape much different? These force us to use curve.
  2. Backwards compatibility - I plan on making some changes to the encryption scheme very soon, so I can change it when I do that, but it's important to remember that we can't just switch without breaking compatibility with existing data.
  3. Ease of development - is it hard to get these libraries to work on all the platforms above? I don't want to be spending weeks with build issues and then later on related upgrading issues.

I'll properly evaluate it when I'm ready to change the encryption scheme, though point 1.2. seems to be a problem, which may have implications to point 1 as a whole.

Thanks a lot for the suggestion.

@tasn
Copy link
Member

tasn commented Mar 29, 2019

Just wanted to let you know that I gave it some more thought and have done some more research on the availability across platforms. Even though Android doesn't seem to have very good nacl support, it looks like we should make the switch. The benefits are just too significant to ignore, so thanks a lot of the suggestion!

It'll be done as part of the other planned overall improvements to the encryption scheme and protocol, which will hopefully happen in the next few months.

@YellowOnion
Copy link

Is there a document detailing the architecture somewhere? Even if it's just the API.

I would also recommend removing "zero knowledge" from descriptive material, as it might be misconstrued with zero knowledge proofs.

@tasn
Copy link
Member

tasn commented Aug 16, 2020

@YellowOnion, thanks for your comments.
There is some information in the FAQ though it's for EteSync 1.0, though EteSync 2.0 which is going to be released soon is already using libsodium (as suggested here) and has a different API. As you can see in the blog post we will are also committed to writing a proper spec, though unfortunately, at the moment it's "read the source code".
With that being said, we have very active in the community chat so if you have any questions just let us know and we will be happy to answer them.
Here are the docs for EteSync 2.0, though they are only for using the JavaScript API at the moment (soon we will add info about using the Java, Kotlin, Python and Rust (maybe) APIs too.

As for zero-knowledge: I hate this term too, though the reason why it's there is because everyone else in the privacy space use it and users were confused about us not having it! I think that people who know what ZKPs are, wouldn't get confused too often, and those who don't wouldn't get confused anyway. For whatever it's worth btw, EteSync 2.0 does use ZKP. :)

@tasn
Copy link
Member

tasn commented Sep 9, 2020

I forgot to highlight it in my previous comment, but EteSync 2.0 also uses libsodium, so this issue will be mostly addressed.

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

No branches or pull requests

5 participants