-
Notifications
You must be signed in to change notification settings - Fork 53
Add support for getting/setting GCM authentication tag (fixes #115) #201
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.
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
Changes from 1 commit
6e70ca0
c77a7cf
82a9a53
e402818
4a8b770
0dad2b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11975,6 +11975,47 @@ static int cipher_final(lua_State *L) { | |||||
| } /* cipher_final() */ | ||||||
|
|
||||||
|
|
||||||
| static int cipher_get_tag(lua_State *L) { | ||||||
| EVP_CIPHER_CTX *ctx = checksimple(L, 1, CIPHER_CLASS); | ||||||
| luaL_Buffer tag; | ||||||
| size_t tag_size = luaL_checkinteger(L, 2); | ||||||
|
|
||||||
| luaL_buffinit(L, &tag); | ||||||
|
|
||||||
| if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, tag_size, (void*)luaL_prepbuffsize(&tag, tag_size))) { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When was
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it's been around a "long time". I don't know if it was in 0.9.8 (I can dig if really necessary), but it's certainly in 1.0.2. It's due to be retired post-3.0.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What is it going to be replaced with?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A new |
||||||
| goto sslerr; | ||||||
| } | ||||||
|
|
||||||
| luaL_pushresultsize(&tag, tag_size); | ||||||
|
|
||||||
| return 1; | ||||||
|
|
||||||
| sslerr: | ||||||
| lua_pushnil(L); | ||||||
| auxL_pusherror(L, auxL_EOPENSSL, NULL); | ||||||
|
|
||||||
| return 2; | ||||||
| } /* cipher_get_tag() */ | ||||||
|
|
||||||
daurnimator marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| static int cipher_set_tag(lua_State *L) { | ||||||
| EVP_CIPHER_CTX *ctx = checksimple(L, 1, CIPHER_CLASS); | ||||||
| size_t tag_size; | ||||||
| const char* tag = luaL_checklstring(L, 2, &tag_size); | ||||||
| if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag_size, (void*)tag)) { | ||||||
| goto sslerr; | ||||||
| } | ||||||
|
|
||||||
| lua_pushlstring(L, tag, tag_size); | ||||||
|
|
||||||
| return 1; | ||||||
|
|
||||||
| sslerr: | ||||||
| lua_pushnil(L); | ||||||
| auxL_pusherror(L, auxL_EOPENSSL, NULL); | ||||||
|
|
||||||
| return 2; | ||||||
| } /* cipher_get_tag() */ | ||||||
|
||||||
| } /* cipher_get_tag() */ | |
| } /* cipher_set_tag() */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0dad2b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
cipher_gcm_get_tagor similar to follow the naming of the option?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly. It's actually not unique to GCM, but to the family of AEAD modes - in OpenSSL this includes CCM. In 1.1.x there is now therefore an alias for
EVP_CTRL_AEAD_GET_TAGbut this constant is not defined in 1.0.x. The current code should work with both GCM and CCM.If we change the name, we could call it e.g.
:get_aead_tag(), but I don't know whether this level of disambiguation is necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be sensible to use
EVP_CIPHER_typeto get the type of the cipher and then use the relevant constant?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. As I mentioned, modern versions of OpenSSL have both CCM_GET_TAG and GCM_GET_TAG aliased to
AEAD_GET_TAG. The latter was added in 1.1, but before this the CCM variant was an alias of GCM_GET_TAG anyway.In summary: in old versions,
GCM_GET_TAGandCCM_GET_TAGare interchangeable identifiers for the same operation. In newer versions (1.1+)AEAD_GET_TAGwas added as a generic identifier for this operation.References:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment saying as much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 4a8b770