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.

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

Added support for verify callback #207

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bigben93
Copy link

I need some additional certificate checking during verification. Vanilla OpenSSL library provides support for custom verify callback as described here https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_verify.html

Unfortunately luaossl doesn't support this feature at the moment. This pull request contains my proposal of this feature. I extended the openssl.context:setVerify. Now you can pass an optional third argument with a custom verify callback written in LUA. I also added X509_STCTX_CLASS with methods getCurrentCert (X509_STORE_CTX_get_current_cert) and getCert (X509_STORE_CTX_get0_cert). Both methods return openssl.x509 object.

Example:

...
local function verifyCallback(preverify, x509Ctx)
    local cert = x509Ctx:getCert()
    print(cert:getSubject())
    return preverify
end
...
ctx:setVerify(yourFlags, nil, verifyCallback)
...

Copy link
Collaborator

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

I haven't reviewed thoroughly, just made some cursory notes.

I vaguely recall that such a function wasn't required, as you could always do the verify step manually? or during some other callback.

@@ -8974,7 +8983,6 @@ static const auxL_Reg stx_metatable[] = {
};

static const auxL_Reg stx_globals[] = {
{ "new", &stx_new },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Author

Choose a reason for hiding this comment

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

As was mentioned in comment in src/openssl.c: X509_STORE_CTX is an temporary object used internally in OpenSSL library. On line 8930 you can find "#if 0" which disables this structure from Luaossl. At the moment I see no point in allowing an end user to create this type of object.

return 0;
} /* stx__gc() */


static const auxL_Reg stx_methods[] = {
{ "add", &stx_add },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Author

Choose a reason for hiding this comment

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

The same situation as "stx_new".

src/openssl.c Outdated
x509_ctx_lua = prepsimple(L, X509_STCTX_CLASS);
*x509_ctx_lua = x509_ctx;

/* passed LUA callback, preferify_ok, x509_ctx */
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo preferify_ok

Copy link
Author

Choose a reason for hiding this comment

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

It will be fixed.

src/openssl.c Outdated

ctx = SSL_get_SSL_CTX(ssl);

/* expect one value: LUA callback */
Copy link
Collaborator

Choose a reason for hiding this comment

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

weird indentation

Copy link
Author

Choose a reason for hiding this comment

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

It will be fixed.

src/openssl.c Outdated

static int sx_setVerify(lua_State *L) {
SSL_CTX *ctx = checksimple(L, 1, SSL_CTX_CLASS);
int mode = luaL_optinteger(L, 2, -1);
int depth = luaL_optinteger(L, 3, -1);
int error = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

indentation is off

Copy link
Author

Choose a reason for hiding this comment

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

It will be fixed.

@bigben93
Copy link
Author

I've fixed typos and added wrapper for X509_STORE_CTX_get_error_depth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants