pyca / pynacl

Python binding to the Networking and Cryptography (NaCl) library
https://pynacl.readthedocs.io/
Apache License 2.0
1.08k stars 232 forks source link

Decryption failed. Ciphertext failed verification #431

Closed leobel96 closed 6 years ago

leobel96 commented 6 years ago

I'm pretty new with python and I'm trying to write a code to encrypt a file and one to decrypt it. I have installed the last version of pynacl and I'm using Windows 64 bit. I use a KDF function (argon2i) to derivate the key for the SecretBox from a custom password, then I encrypt the file using the key and I save all the parameters (except the password, obviously) at the beginning of the encrypted file. The encryption is very simple: I take all the parameters from the beginning of the file, I insert password to generate the Key and, then I decrypt it. The problem is that I keep obtaining this error:

Traceback (most recent call last):
  File "C:\Users\leobe\Desktop\pynacl.py", line 36, in <module>
    outfile.write(Box.decrypt(chunk,nonce))
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\nacl\secret.py", line 125, in decrypt
    nonce, self._key)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\nacl\bindings\crypto_secretbox.py", line 76, in crypto_secretbox_open
    raising=exc.CryptoError)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\nacl\exceptions.py", line 68, in ensure
    raise raising(*args)
nacl.exceptions.CryptoError: Decryption failed. Ciphertext failed verification 

It's very strange because I've verificated that alla parameters are the same in encryption and decryption code and also derivated key and SecretBox are identical. This is the encryption code:

from nacl import pwhash, secret, utils

password = b'ciao'
inFile = "file.avi"
outFile = "out.crypt"
chunksize = 64 * 1024

kdf = pwhash.argon2i.kdf
salt = utils.random(pwhash.argon2i.SALTBYTES)
ops = pwhash.argon2i.OPSLIMIT_SENSITIVE
mem = pwhash.argon2i.MEMLIMIT_SENSITIVE
nonce = utils.random(secret.SecretBox.NONCE_SIZE)
encodedOps = str(ops).encode(encoding='UTF-8')
encodedMem = str(mem).encode(encoding='UTF-8')

derivatedKey = kdf(secret.SecretBox.KEY_SIZE, password, salt, opslimit=ops, memlimit=mem)
Box = secret.SecretBox(derivatedKey)
print('Box = {}'.format(Box))

with open(inFile, "rb") as infile:
  with open(outFile, "wb") as outfile:
    outfile.write(salt)
    print('salt = {}'.format(salt))
    outfile.write(encodedOps)
    outfile.write(b'\n')
    print('ops = {}'.format(ops))
    print ('encodedOps = {}'.format(encodedOps))
    outfile.write(encodedMem)
    outfile.write(b'\n')
    print('mem = {}'.format(mem))
    print ('encodedMem = {}'.format(encodedMem))
    outfile.write(nonce)
    outfile.write(b'\n')
    print('nonce = {}'.format(nonce))
    while True:
      chunk = infile.read(chunksize)
      if len(chunk) == 0:
        break
      elif len(chunk) % 16 !=0:
        chunk += bytes(' ' *  (16 - (len(chunk) % 16)),'utf-8')
      outfile.write(Box.encrypt(chunk, nonce))

and this is the decryption one:

from nacl import pwhash, secret, utils

password = b'ciao'
outFile = "file1.avi"
inFile = "out.crypt"
chunksize = 64 * 1024

kdf = pwhash.argon2i.kdf

with open(inFile, "rb") as infile:
    with open(outFile, "wb") as outfile:
        salt = infile.read(pwhash.argon2i.SALTBYTES)
        encodedOps = infile.readline()
        encodedMem = infile.readline()
        nonce = infile.read(secret.SecretBox.NONCE_SIZE)
        print('salt = {}'.format(salt))
        print('encodedOps = {}'.format(encodedOps))
        ops = int(encodedOps.decode(encoding='UTF-8'))
        print('ops = {}'.format(ops))
        print('encodedMem = {}'.format(encodedMem))
        mem = int(encodedMem.decode(encoding='UTF-8'))
        print('mem = {}'.format(mem))
        print('nonce = {}'.format(nonce))
        derivatedKey = kdf(secret.SecretBox.KEY_SIZE, password, salt, opslimit=ops, memlimit=mem)
        print(derivatedKey)
        Box = secret.SecretBox(derivatedKey)
        print ('Box = {}'.format(Box))
        while True:
            chunk = infile.read(chunksize)
            if len(chunk) == 0:
                break
            elif len(chunk) % 16 !=0:
                chunk += bytes(' ' *  (16 - (len(chunk) % 16)),'utf-8')
            outfile.write(Box.decrypt(chunk,nonce))

I know the code is not well optimized but now it only needs to work. Thank you

lmctv commented 6 years ago

@leobel96, I'm appending the corrections I made to your examples as unified diffs and will discuss the changes at the end.

--- encrypt.py_leobel96 2018-04-30 15:01:08.981691228 +0200
+++ encrypt.py  2018-04-30 15:22:07.530359964 +0200
@@ -1,9 +1,11 @@
 from nacl import pwhash, secret, utils
+from nacl.bindings import sodium_increment

 password = b'ciao'
 inFile = "file.avi"
 outFile = "out.crypt"
 chunksize = 64 * 1024
+macsize = 16

 kdf = pwhash.argon2i.kdf
 salt = utils.random(pwhash.argon2i.SALTBYTES)
@@ -37,6 +39,10 @@
             chunk = infile.read(chunksize)
             if len(chunk) == 0:
                 break
-            elif len(chunk) % 16 !=0:
-                chunk += bytes(' ' *    (16 - (len(chunk) % 16)),'utf-8')
-            outfile.write(Box.encrypt(chunk, nonce))
+            outchunk = Box.encrypt(chunk, nonce).ciphertext
+            assert len(outchunk) == len(chunk) + macsize
+            outfile.write(outchunk)
+            nonce = sodium_increment(nonce)
--- decrypt.py_leobel96 2018-04-30 15:01:34.118105906 +0200
+++ decrypt.py  2018-04-30 15:05:46.010260582 +0200
@@ -1,9 +1,11 @@
 from nacl import pwhash, secret, utils
+from nacl.bindings import sodium_increment

 password = b'ciao'
 outFile = "file1.avi"
 inFile = "out.crypt"
 chunksize = 64 * 1024
+macsize = 16

 kdf = pwhash.argon2i.kdf

@@ -25,10 +27,12 @@
         print(derivatedKey)
         Box = secret.SecretBox(derivatedKey)
         print ('Box = {}'.format(Box))
+        _newline = infile.read(1)
         while True:
-            chunk = infile.read(chunksize)
-            if len(chunk) == 0:
+            rchunk = infile.read(chunksize + macsize)
+            if len(rchunk) == 0:
                 break
-            elif len(chunk) % 16 !=0:
-                chunk += bytes(' ' *  (16 - (len(chunk) % 16)),'utf-8')
-            outfile.write(Box.decrypt(chunk,nonce))
+            dchunk = Box.decrypt(rchunk, nonce)
+            assert len(dchunk) == len(rchunk) - macsize
+            outfile.write(dchunk)
+            nonce = sodium_increment(nonce)

First thing first: if you append a newline after writing the nonce, you must remember reading it, otherwise the decryptor won't be able to synchronize with what the encryptor wrote.

Next, if you choose to manually manage the nonce, you should not reuse the same nonce for different chunks, but instead increment it once for every Box.{en,de}crypt call; further, since the Box class can generate a random nonce on any encrypt() call, the returned EncryptedMessage prepends the used nonce to the actual ciphertext; if you are manually managing the nonce, you need to just get the ciphertext as done in the outchunk = Box.encrypt(…) line.

Last, since SecretBox leverages a stream construction, there is no need for explicit padding; if you need padding for other reasons, you should 1. use a reversible padding primitive and 2. pad and unpad only the plaintext stream.

I'll leave the issue open just as a documentation one.

leobel96 commented 6 years ago

Thank you!!!!

lmctv commented 6 years ago

I've added some needed documentation on usage of the nacl.utils.EncryptedMessage and the definition of nacl.secret.SecretBox.MACBYTES in #433. You can read the improved docs at http://lmctvpynacl.readthedocs.io/en/add-crypto-secretbox-macbytes-bindings/secret.html . Closing this issue, since it's superceded by #433.

leobel96 commented 6 years ago

@lmctv Sorry but I'm getting ImportError: cannot import name 'sodium_increment'

lmctv commented 6 years ago

Sorry, I forgot sodium_increment is only available in git master branch.

If you are not familiar with virtualenvs and pip installation from sources, I think it would be better if you just let SecretBox manage the nonce, writing the full nonce||ciphertext||Mac concatenated representation of EncryptedMessage on the encryption side, and reading chunksize+nonce_size+macsize byte-sized chuncks on decription.