bungle / lua-resty-nettle

LuaJIT FFI bindings for Nettle (a low-level cryptographic library)
BSD 2-Clause "Simplified" License
182 stars 45 forks source link

sometime aes:decrypt return data has been cuted #5

Closed angelandy closed 8 years ago

angelandy commented 8 years ago

sometime aes:decrypt return data has been cuted,

i fixed it like that:

https://github.com/bungle/lua-resty-nettle/blob/master/lib/resty/nettle/aes.lua line:385 return ffi_str(dst) => return ffi_str(dst,len)

bungle commented 8 years ago

thanks for the report, i need to check this more thoroughly. Can you give an example where this did happen? Is it some binary data with \0 in them?

angelandy commented 8 years ago

yes some binary data with \0 in them,

when your data have been zlib compress, it would happen.

bungle commented 8 years ago

Just to let you know, I already implemented a few padding algos that you may try: https://github.com/bungle/lua-resty-nettle/tree/master/lib/resty/nettle/padding

Before feeding data to encrypt, pad it using one of these (excluding the nopadding.lua). for example using PKCS7:

local pad = require "resty.nettle.padding.pkcs7"
data = pad(data)
-- ... encrypt here

And after decryption, unpad the data

local pad = require "resty.nettle.padding.pkcs7"
-- local data = ... decrypt here
data = pad.unpad(data)

I will continue fixing the other places where this could go wrong, but at least the above is a good work-a-round. Maybe I need to add padding algorihm as a parameter for AES etc. funcs.

angelandy commented 8 years ago

got it,thank you

bungle commented 8 years ago

@angelandy, I have now finished my first try to fix this.

I have added len argument to :encrypt and :decrypt functions (defaulting to input size). What changes here is now the :decrypt behaves correctly even when there are \0 in input. But what also changes is, that we do return the padded zeros on decrypt. We don't really know how long the decrypted string will really be if we don't know the padding. On block ciphers like AES ECB / CBC the output bytes of :encrypt will always be multiples of 16 (the block size). We now do zero-padding by default (but we don't zero pad if the string is exactly a multiple of 16 – that differs from the behavior of resty.nettle.padding.zeropadding that will always pad zeros, maximum being 16 zeros).

So on say AES ECB / CBC :decrypt we get x times 16 bytes in and the output will also be same size (if you don't pass in the different, aka known size). That means that padded zeroes will not be automatically removed. You can remove them with resty.nettle.padding.zeropadding's .unpad function.

But preferably you should pad your strings (using your preferred algorithm, pkcs7 being a reasonable choice) before feeding the data to :encrypt and unpad with same algorithm after doing decrypt.

bungle commented 8 years ago

I think this is fixed now in 0.95. Please reopen if you feel there are still issues with it.

hcaihao commented 8 years ago

function desEncrypt(plain, key)
    local padding = require "resty.nettle.padding.zeropadding"
    local des = require "resty.nettle.des"
    local ds, wk = des.new(key)

    local cipher = ds:encrypt(padding.pad(plain, 8))
    return UtilityService:binToHex(cipher)
end

function desDecrypt(chiper, key)
    local padding = require "resty.nettle.padding.zeropadding"
    local des = require "resty.nettle.des"
    local ds, wk = des.new(key)

    local plain = ds:decrypt(UtilityService:hexToBin(chiper))
    return padding.unpad(plain, 8)
end

I use this 2 funcs to encrypt&decrypt, is it right?

bungle commented 8 years ago

@hcaihao,

Looks just right. One thing though about the padding funcs. They will always pad even if the data is multiple of the block size, here 8 bytes, 16 bytes, etc. Meaning that 8 bytes plain text will result 16 bytes ciphertext, and 16 bytes plain text will result 24 bytes cipher text. Zeropadding is kinda default in all encrypt functions. If the data is not multiple of block size we will automatically allocate a string that is multiple of block size, and LuaJIT initializes the rest with NULLs aka zeropadding. That means, that in above example you don't need to pad on desEncrypt, it will also mean that 8 bytes of plain text will result 8 bytes of ciphertext.

There is optional third argument in padding functions optional:

-- s will be 8 bytes (this is what block ciphers will do automatically on 0.95)
local s = padding.pad("12345678", 8, true)
-- s will be 16 bytes
local s = padding.pad("12345678", 8, false)
-- s will be 16 bytes
local s = padding.pad("12345678", 8, nil)
-- s will be 16 bytes
local s = padding.pad("12345678", 8)

So by default padding always pads, and maximum padding is the blocksize of bytes and minimum is 1. If the third parameter is true then maximum is blocksize -1, and minimum is 0.

Most libs do automatic zeropadding and zerounpadding by default, but there is a risk that it cuts out the leading null bytes that may or may not be relevant. That's the only reason I have not enabled zero unpadding by default. On encrypt we do zero pad automatically, because otherwise you cannot supply arbitrary length strings to it and you would always need to pad by yourself. There are better padding algorithms than zeropadding that may be used, common choice is pkcs7.

Here are how different paddings works (plain, hex, hex with padding), examples use 16 byte block size here:

zeropadding (aka null bytes padded):

a
61
61000000000000000000000000000000

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616100000000000000000000000000000000

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
6161616161616161616161616161616161000000000000000000000000000000

pkcs7 (aka padding with number of how many padding chars repeated):

a
61
610F0F0F0F0F0F0F0F0F0F0F0F0F0F0F

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616110101010101010101010101010101010

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
61616161616161616161616161616161610F0F0F0F0F0F0F0F0F0F0F0F0F0F0F

ansix923 (aka null bytes + last byte tells how many padding chars added):

a
61
6100000000000000000000000000000F

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616100000000000000000000000000000010

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
616161616161616161616161616161616100000000000000000000000000000F

iso7816-4 (aka 80 (hex) + rest null bytes):

a
61
61800000000000000000000000000000

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616180000000000000000000000000000000

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
6161616161616161616161616161616161800000000000000000000000000000

iso10126 (aka random bytes + last byte tells how many padding bytes added):

a
61
614E158D4333434454A7FD745254A50F

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616130909724E4DAD6C76D06383EF4CECC10

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
61616161616161616161616161616161613AA5DC845C89725B5630B6EDA2D00F

spacepadding (same as zeropadding but with spaces instead of null bytes):

a
61
61202020202020202020202020202020

aaaaaaaaaaaaaaaa
61616161616161616161616161616161
6161616161616161616161616161616120202020202020202020202020202020

aaaaaaaaaaaaaaaaa
6161616161616161616161616161616161
6161616161616161616161616161616161202020202020202020202020202020
hcaihao commented 8 years ago
local padding = require "resty.nettle.padding.zeropadding"
-- s will be 8 bytes (this is what block ciphers will do automatically on 0.95)
local s = padding.pad("12345678", 8, true)
ngx.say(s.." "..#s)
-- s will be 16 bytes
local s = padding.pad("12345678", 8, false)
ngx.say(s.." "..#s)
-- s will be 16 bytes
local s = padding.pad("12345678", 8, nil)
ngx.say(s.." "..#s)
-- s will be 16 bytes
local s = padding.pad("12345678", 8)
ngx.say(s.." "..#s)

Output: 12345678 16 12345678 16 12345678 16 12345678 16

I found "s" is always 16 bytes.

I use the lastest zeropadding.lua.

local assert = assert
local string = string
local type = type
local gsub = string.gsub
local rep = string.rep
local padding = {}
function padding.pad(data, blocksize, optional)
    blocksize = blocksize or 16
    assert(type(blocksize) == "number" and blocksize > 0 and blocksize < 257, "Invalid block size")
    local ps = blocksize - #data % blocksize
    if ps == 0 then
        if optional then return data end
        ps = blocksize
    end
    return data .. rep("\0", ps)
end
function padding.unpad(data, blocksize)
    blocksize = blocksize or 16
    assert(type(blocksize) == "number" and blocksize > 0 and blocksize < 257, "Invalid block size")
    local len = #data
    assert(len % blocksize == 0, "Data's length is not a multiple of the block size")
    data = gsub(data, "%z+$", "")
    local rem = len - #data
    assert(rem > 0 and rem <= blocksize, "Invalid padding found")
    return data
end
return padding
bungle commented 8 years ago

@hcaihao,

Great catch! Thank you! I will fix this.

bungle commented 8 years ago

@hcaihao, this in now fixed on this commit: https://github.com/bungle/lua-resty-nettle/commit/1b740408e8eb983fccd70c0fbbbc264fe0265ab7

Please close if you find it fixed as well.