pynacl icon indicating copy to clipboard operation
pynacl copied to clipboard

Creation of box from zero public key raises RuntimeError: Unexpected library error

Open maqp opened this issue 7 years ago • 4 comments

A simple way to reproduce this bug is

import nacl.public
sk_bob   = nacl.public.PrivateKey.generate()
pk_alice = nacl.public.PublicKey(bytes(32))
box      = nacl.public.Box(sk_bob, pk_alice)

IIRC in X25519 zero public key leads to zero shared secret, so it's understandable libsodium raises exception in this case. I think pynacl should handle the detection and raise ValueError with understandable description along the lines of Zero public keys are not allowed.

maqp avatar Aug 02 '17 18:08 maqp

libsodium expectation is the caller will send the public part of a correctly generated key-pair, I think the behaviour is correct both on libsodium's side and on our one. I'm inclined to close this issue.

lmctv avatar Sep 02 '17 10:09 lmctv

I understand this but it's now the case that any delivered zero public key from frenemy / third party can lead to unhandled crash of recipient's client (availability is part of the CIA triad after all); There's a fair chance developers who use the library won't have exception handling for nacl.exceptions.RuntimeError as it happens only with this specific public key value.

maqp avatar Sep 06 '17 13:09 maqp

Since all of the exceptions PyNaCl raises are subclasses of nacl.exceptions.CryptoError (https://pynacl.readthedocs.io/en/latest/exceptions/), downstream users can (and IMHO should) wrap any call into nacl code inside a try:..except nacl.exceptions.CryptoError: ... control structure; this way they are in a position to fail-fast for any cryptographic problem.

Until some kind of consensus is found and standardized on possible recommended reactions to the findings exposed in https://eprint.iacr.org/2017/806 , I think libsodium will continue to avoid exposing a method to check beforehand if the received public key in the "unsafe in case ``contributory'' behavior is required" set described in https://cr.yp.to/ecdh.html#validate , and I'd rather avoid introducing such a majior API difference with our upstream.

Thank you very much

lmctv avatar Sep 06 '17 16:09 lmctv

@lmctv You're right. The recommendations on how to use exceptions are well documented. I think it would however be a good idea to include exception handling in examples. We could either recommend checking the article, place everything inside try-except, or just the ones with external inputs: Box(), box.decrypt(), SealedBox(), unseal_box.decrypt(), secret_box.decrypt(), VerifyKey() and verify_key.verify().

maqp avatar Sep 11 '17 15:09 maqp