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

Critical vulnerability in Monocypher (patched upstream) #2

Open
LoupVaillant opened this issue Mar 31, 2019 · 21 comments
Open

Critical vulnerability in Monocypher (patched upstream) #2

LoupVaillant opened this issue Mar 31, 2019 · 21 comments

Comments

@LoupVaillant
Copy link

Hi,

@fscoto just noticed you are using a vulnerable version of Monocypher. The vulnerability is about EdDSA signatures: this version of Monocypher accepts all-zero signatures as valid. This was disclosed several months ago, you might have missed it.

Please update to the latest version of Monocypher (currently 2.0.5). When it is done, I'll restore the link in my downloads page.

@HACKERALERT
Copy link

HACKERALERT commented Apr 18, 2021

@LoupVaillant I'm planning to use this Go binding of Monocypher for a file encryption tool. Assuming I fork this repo and update Monocypher to 3.1.2 and only use crypto_lock/crypto_unlock in this binding, would this cause any security issues? Since 3.1.2 is after the Cure53 audit, updating this binding's Monocypher version should eliminate any known security issues, right? Thank you.

@LoupVaillant
Copy link
Author

Yes, updating Monocypher will eliminate all known issues. I cannot vouch for the bindings, though: I haven't reviewed them (I don't know Go), and this repository seems to no longer be maintained. Do take a look, and use your judgement. Forking this repository sounds good.

@demonshreder, I understand that maintaining an open source repository takes time you may not have. Don't hesitate to pass the torch —perhaps to @HACKERALERT if they're willing? Personally, I'm still looking forward to go bindings I can link to from my downloads page.

@HACKERALERT
Copy link

Thanks for letting me know! I've taken a look at the bindings and so far everything looks good. Unfortunately, I don't have the time to maintain something like this, so the best I can do is fork this repo and update Monocypher. Thanks for creating Monocypher, though! It's one of the only crypto libraries I trust because it's small and audited.

@HACKERALERT
Copy link

I tried updating Monocypher to v3.1.2, but the bindings don't work for that version, and I don't have the time to rewrite the bindings. I guess I'll have to use another library.

@LoupVaillant
Copy link
Author

Crap, you're right, this was a major version change. There are two quick and easy solutions for this:

  • Update to version 2.0.7 instead. It's a fully compatible version with all know issues fixed.
  • Remove incompatible functions. Monocypher 3 is almost compatible with Monocypher 2, there were just a couple breaking changes in the low-level API (Chacha20 specifically).

That being said, a quick look over these bindings seem to indicate they didn't bind to the low-level interface, so I'm a little surprised to see version 3.1.2 break. Could you possibly tell me what exactly didn't work when tried to update, so I can maybe reproduce the issue? I generally try very hard to keep backward compatibility, if I failed I want to know about it.

Of course, if you're really timed constrained, maybe use Libsodium? It has Go bindings, though apparently they're lagging a bit behind (v1.0.12), and they don't support crypto_aead_xchacha20poly1305_ietf_encrypt_detached(), which is the one function in Libsodium that's compatible with Monocypher's crypto_lock() (they do have the same function without the x though, so making the change may be straightforward).

@HACKERALERT
Copy link

I found the issue :). If you scroll down to the bottom of monocypher.c in this binding and the latest monocypher.c, one has crypto_aead_unlock and the other has crypto_unlock_aead. A simple renaming in the binding should fix the issue. This makes sense because the issue I had yesterday was indeed with crypto_unlock_aead.

@HACKERALERT
Copy link

I'll fork and fix the bindings and I'll let you know how it goes.

@HACKERALERT
Copy link

Yup, everything works now: https://github.com/HACKERALERT/Monocypher-Go.

@LoupVaillant
Copy link
Author

If you scroll down to the bottom of monocypher.c in this binding and the latest monocypher.c, one has crypto_aead_unlock and the other has crypto_unlock_aead.

Oh my, that was an old change, that i committed over 3 years ago. The names of crypto_lock() and crypto_unlock() weren't perfectly consistent, so I fixed that. That was a breaking change, but lock and unlock functions were already undergoing a breaking change: IETF padding conformity (it's faster and more secure than what I had before, and as a bonus was also compatible with Libsodium).

Yup, everything works now: https://github.com/HACKERALERT/Monocypher-Go.

Nice! May I link this repository from my downloads page? Possibly with a warning that it is incomplete and mostly unmaintained if that makes you more comfortable?

@HACKERALERT
Copy link

HACKERALERT commented Apr 20, 2021

Yeah sure! You can leave a notice (if you would like) that there won't be any development (I won't add bindings for all Monocypher functions), but I'll do my best to maintain the existing bindings security-wise.

@LoupVaillant
Copy link
Author

Added, thanks a lot.

@HACKERALERT
Copy link

No problem. Thanks a lot to you for creating Monocypher!

@HACKERALERT
Copy link

HACKERALERT commented Apr 20, 2021

@LoupVaillant Can you remove the link to my repo for a short while? The original creator of this binding didn't check for a valid MAC, so I need a little time to fix it up this major security issue. I'll let you know when the issue is fixed.

@demonshreder
Copy link
Owner

@HACKERALERT @LoupVaillant I was inspired by Monocypher trying to validate current security standards from scratch. With no knowledge of cryptography and elementary knowledge of C, I took an attempt to write bindings for Go, trying to atleast pass objects between Go to Monocypher without distortion. Then due to change in my career and lack of time to read up on things, I had stopped working on it.

I have already handed over the Archlinux AUR package to another interested community member. I have invited @HACKERALERT to this repo, if this doesn't work (for whatsoever reason), please feel free to have another repo.

@HACKERALERT
Copy link

@demonshreder No worries! I can take on the responsibility of security for this library since I have a good amount of knowledge in cryptography. I've already created another repo, so you don't have to worry about anything. For any future users that come to this repo, it might be a good idea for you to link my fork of your repo to the homepage: github.com/HACKERALERT/Monocypher-Go.

@LoupVaillant
Copy link
Author

@HACKERALERT, this is done. I'll reinstate a link of your choosing when you're done. (Don't sweat it, though, it can always be changed.)

@demonshreder, thank you for responding. Your repository provided a jump start, we'll make sure it doesn't go to waste. (And thanks for the AUR package, I didn't remember it was you.)

@demonshreder
Copy link
Owner

@HACKERALERT I have updated the README pointing to your repo.

All the best to both of you, thanks for your efforts.

@HACKERALERT
Copy link

@LoupVaillant I've fixed the security issue and reviewed all the other functions. There shouldn't be any security issues now. You can put the same link back up, thanks!

@HACKERALERT
Copy link

@LoupVaillant I did some cleaning up, formatting, and renaming of my Go bindings. Can you update the repo link? It's the same, except lowercase (which is a Go convention): https://github.com/HACKERALERT/monocypher-go. Also, I just noticed that demonshreder's wonderful work is licensed under MIT. I'm not a licensing expert, but I would like to make sure that it is compatible with Monocypher's licensing. Thanks!

@LoupVaillant
Copy link
Author

OK, I’ll update the link this weekend.

Monocypher’s licensing is as permissive as possible, effectively public domain. It’s compatible with anything. I also believe the 2-clause BSD we use as a fallback is compatible with the MIT licence, though I’d rather have @fscoto confirm that.

@HACKERALERT
Copy link

I'm not using Monocypher anymore, and I'm also cleaning up some unnecessary repositories in my account. I would like to pass on the maintenance of monocypher-go to someone else. GitHub currently shows that no projects are dependent on my repository so there won't be any issues or breakage. I was thinking of pushing my current changes to this repository and you can point back to it, although I don't think I was invited to this repo. So instead, I've zipped the repo and attached it. Good luck to any future maintainers! Sorry for any inconvenience, but a man only has so much time in his hands.

monocypher-go-main.zip

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

No branches or pull requests

3 participants