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.

Sign up for GitHub

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

AES get error message #65

Closed
wants to merge 1 commit into from
Closed

AES get error message #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions lib/resty/aes.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *outm, int *outl);
int EVP_BytesToKey(const EVP_CIPHER *type,const EVP_MD *md,
const unsigned char *salt, const unsigned char *data, int datal,
int count, unsigned char *key,unsigned char *iv);

unsigned long ERR_get_error(void);
void ERR_error_string_n(unsigned long e, char *buf, size_t len);
]]

local hash
Expand All @@ -104,6 +107,18 @@ cipher = function (size, _cipher)
end
_M.cipher = cipher

local function get_error()
local errno = C.ERR_get_error()
if errno == 0 then
return nil
end

local msg = ffi_new("char[?]", 256)
C.ERR_error_string_n(errno, msg, 256)
Comment on lines +116 to +117
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hope don’t usage magic number

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One OpenSSL call may cause several error messages. Therefore, to handle the error message properly, we need to draw the whole error messages out like:
https://github.com/spacewander/lua-resty-rsa/blob/6a6073f2dcfc319fdf760c65a26382f2a8cf67df/lib/resty/rsa.lua#L158

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One OpenSSL call may cause several error messages

I suspected it but couldn't find confirmation in the OpenSSL documentation. Does the documentation say it clearly or you know it from source code (or another source)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attention with https://github.com/spacewander/lua-resty-rsa/blob/6a6073f2dcfc319fdf760c65a26382f2a8cf67df/lib/resty/rsa.lua#L158.
C.ERR_reason_error_string function may return a null value when the code not match a human readable message.
so null value checking for err variable

local err = C.ERR_reason_error_string(code)
if err ~= ffi.null then
    err_queue[i] = ffi_str(err)
    ...
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toruneko
Yes, you are right. We need to check the returned err before using it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@un-def
Here is the doc:
https://en.wikibooks.org/wiki/OpenSSL/Error_handling

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, sometimes OpenSSL don't even return an error message when the call is failed. (Perhaps it is a bug): openssl/openssl#11962

So strictly speaking, we need to provide a default one if no error is found:
https://github.com/spacewander/lua-resty-rsa/blob/6a6073f2dcfc319fdf760c65a26382f2a8cf67df/lib/resty/rsa.lua#L180

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spacewander I've seen it, but this is not the official OpenSSL wiki and has not been updated for a long time. I meant the official docs hosted at https://www.openssl.org/ or official wiki https://wiki.openssl.org/ Anyway, I agree that it's worth popping all entries from the error stack, it's safe and future-compatible.


return ffi_str(msg)
end

function _M.new(self, key, salt, _cipher, _hash, hash_rounds)
local encrypt_ctx = C.EVP_CIPHER_CTX_new()
if encrypt_ctx == nil then
Expand Down Expand Up @@ -158,15 +173,15 @@ function _M.new(self, key, salt, _cipher, _hash, hash_rounds)
hash_rounds, gen_key, gen_iv)
~= _cipherLength
then
return nil
return nil, get_error()
end
end

if C.EVP_EncryptInit_ex(encrypt_ctx, _cipher.method, nil,
gen_key, gen_iv) == 0 or
C.EVP_DecryptInit_ex(decrypt_ctx, _cipher.method, nil,
gen_key, gen_iv) == 0 then
return nil
return nil, get_error()
end

return setmetatable({
Expand All @@ -185,15 +200,15 @@ function _M.encrypt(self, s)
local ctx = self._encrypt_ctx

if C.EVP_EncryptInit_ex(ctx, nil, nil, nil, nil) == 0 then
return nil
return nil, get_error()
end

if C.EVP_EncryptUpdate(ctx, buf, out_len, s, s_len) == 0 then
return nil
return nil, get_error()
end

if C.EVP_EncryptFinal_ex(ctx, buf + out_len[0], tmp_len) == 0 then
return nil
return nil, get_error()
end

return ffi_str(buf, out_len[0] + tmp_len[0])
Expand All @@ -208,15 +223,15 @@ function _M.decrypt(self, s)
local ctx = self._decrypt_ctx

if C.EVP_DecryptInit_ex(ctx, nil, nil, nil, nil) == 0 then
return nil
return nil, get_error()
end

if C.EVP_DecryptUpdate(ctx, buf, out_len, s, s_len) == 0 then
return nil
return nil, get_error()
end

if C.EVP_DecryptFinal_ex(ctx, buf + out_len[0], tmp_len) == 0 then
return nil
return nil, get_error()
end

return ffi_str(buf, out_len[0] + tmp_len[0])
Expand Down
31 changes: 31 additions & 0 deletions t/aes.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,34 @@ failed to new: bad iv
--- no_error_log
[error]



=== TEST 11: AES get error message
--- http_config eval: $::HttpConfig
--- config
location /t {
content_by_lua_block {
local aes = require "resty.aes"

local aes_default, err = aes:new("secret")

if not aes_default then
ngx.say("failed to new: ", err)
return
end

local encrypted, err = aes_default:decrypt("hello")
if not encrypted then
if err then
ngx.say(err)
end
end
}
}
--- request
GET /t
--- response_body
error:0606506D:digital envelope routines:EVP_DecryptFinal_ex:wrong final block length
--- no_error_log
[error]