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

Des decrypt incorrent with lua-resty-nettle 0.95 #6

Closed hcaihao closed 8 years ago

hcaihao commented 8 years ago
function hexToBin(hex, spacer)
    if spacer == nil then
        spacer = ""
    end

    local h2b =
    {
        ["0"] = 0,
        ["1"] = 1,
        ["2"] = 2,
        ["3"] = 3,
        ["4"] = 4,
        ["5"] = 5,
        ["6"] = 6,
        ["7"] = 7,
        ["8"] = 8,
        ["9"] = 9,
        ["A"] = 10,
        ["B"] = 11,
        ["C"] = 12,
        ["D"] = 13,
        ["E"] = 14,
        ["F"] = 15,
        ["a"] = 10,
        ["b"] = 11,
        ["c"] = 12,
        ["d"] = 13,
        ["e"] = 14,
        ["f"] = 15,
    }

    return (string.gsub(hex, "(.)(.)"..spacer, function (h, l)
         return string.char(h2b[h]*16+h2b[l])
    end))
end

    local des = require "resty.nettle.des"
    local ds, wk = des.new("testtest")

    local plain = ds:decrypt(hexToBin("DB69A93477DDC028"))
    ngx.say(plain == "2.1.0.0")

Got false with lua-resty-nettle 0.95, but true with lua-resty-nettle 0.9.

bungle commented 8 years ago

thanks, 0.95 got some big changes, I'll look at it.

bungle commented 8 years ago

Yeah, this works:

function hexToBin(hex, spacer)
    if spacer == nil then
        spacer = ""
    end

    local h2b =
    {
        ["0"] = 0,
        ["1"] = 1,
        ["2"] = 2,
        ["3"] = 3,
        ["4"] = 4,
        ["5"] = 5,
        ["6"] = 6,
        ["7"] = 7,
        ["8"] = 8,
        ["9"] = 9,
        ["A"] = 10,
        ["B"] = 11,
        ["C"] = 12,
        ["D"] = 13,
        ["E"] = 14,
        ["F"] = 15,
        ["a"] = 10,
        ["b"] = 11,
        ["c"] = 12,
        ["d"] = 13,
        ["e"] = 14,
        ["f"] = 15,
    }

    return (string.gsub(hex, "(.)(.)"..spacer, function (h, l)
        return string.char(h2b[h]*16+h2b[l])
    end))
end

local des = require "resty.nettle.des"
local ds, wk = des.new("testtest")

local plain = ds:decrypt(hexToBin("DB69A93477DDC028"))
local unpad = require "resty.nettle.padding.zeropadding".unpad
local bytes = unpad(plain, 8)

print(#bytes, bytes)
print(bytes == "2.1.0.0")
bungle commented 8 years ago

This is not actually a bug, but a change in these bindings, see: #5.

So DES is a block cipher with 8 bytes block size. On 0.9, when decrypting we did return bytes until the first \0(null byte). This was plain wrong with binary data although it worked with text data just fine. Now, we don't at least yet do unpadding automatically as we don't really know how the input was padded in a first place. That's why I suggest you to look at these:

https://github.com/bungle/lua-resty-nettle/tree/master/lib/resty/nettle/padding

To get earlier (but "fixed") 0.9 behavior, look at this comment: https://github.com/bungle/lua-resty-nettle/issues/6#issuecomment-214989638

But what I would really do is to use this: https://github.com/bungle/lua-resty-nettle/blob/master/lib/resty/nettle/padding/pkcs7.lua

E.g. the above:

local pkcs7 = require "resty.nettle.padding.pkcs7"
local des = require "resty.nettle.des"
local ds, wk = des.new("testtest")

local encrypted = ds:encrypt(pkcs7.pad("2.1.0.0", 8))
local plain = pkcs7.unpad(ds:decrypt(encrypted), 8)

print(plain == "2.1.0.0")
bungle commented 8 years ago

In future I may add another parameter to new or maybe encrypt and decrypt where you can select padding algorithm, but right now, do it outside encryption/decryption like I showed above.

bungle commented 8 years ago

If you know the length of the decrypted string, then you can give length as an argument to decrypt:

local des = require "resty.nettle.des"
local ds, wk = des.new("testtest")

-- Here I added 7, the length of "2.1.0.0" 
local plain = ds:decrypt(hexToBin("DB69A93477DDC028"), 7)
ngx.say(plain == "2.1.0.0")
hcaihao commented 8 years ago
const char* EncodeDesString(const char* text, const char* key = "testtest")
{
    ECB_Mode<DES>::Encryption e;
    e.SetKey((const unsigned char *)key, DES::KEYLENGTH);
    std::string cipher;

    try
    {
        StringSource(text, true,
            new CryptoPP::StreamTransformationFilter(e, new HexEncoder(new CryptoPP::StringSink(cipher), true), BlockPaddingSchemeDef::ZEROS_PADDING, true)
        );
    }
    catch (const CryptoPP::Exception& e)
    {
        cipher.clear();
    }

    return cipher.c_str();
}
    local padding = require "resty.nettle.padding.zeropadding"
    local des = require "resty.nettle.des"
    local ds, wk = des.new("testtest")

    local plain = ds:decrypt(hexToBin("352d5980a4305508"))
    ngx.say(padding.unpad(plain, 8))
    --ngx.say(plain) --This works

I use cryptopp to encrypt text "chiper=a" with zero padding and get the chiper "352d5980a4305508", but when I decrypt it by nettle, I get error: "zeropadding.lua:24: Invalid padding found", How to make this two libiaries matched. I'm not very clearly, Help please, thank you!

bungle commented 8 years ago

@hcaihao,

Yes, it seems like invalid padding check is too strict when there was no padding added. I will fix this.

bungle commented 8 years ago

@hcaihao,

I changed unpadding to be less strict on validation. It has good and bad sides, but I think this is fair enough compromise. Please close this if the problem goes away.

The fix is in this commit: https://github.com/bungle/lua-resty-nettle/commit/d37d9c75d6d13d86840b1f9793a61aca48569a4d