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

Jump to bottom

(feat) Add additional methods for crl ad csr modules #8

Closed

nasrullo wants to merge 39 commits into fffonion:master from nasrullo:csr

Contributor

nasrullo commented Aug 6, 2020

Adds get_extensions and get_extension to csr module
Adds revoked module for adding revoked items
Adds add_revoked method to crl module

Owner

fffonion commented Aug 6, 2020

@nasrullo Thanks for the PR! The failure in CI is unrelated, I didn't pin the lua-nginx-module and it's crying. I'll go ahead with a review now and fix the CI later.

fffonion reviewed

View reviewed changes

lib/resty/openssl/include/asn1.lua Outdated Show resolved Hide resolved

lib/resty/openssl/include/x509/crl.lua Outdated Show resolved Hide resolved

lib/resty/openssl/include/x509/csr.lua Outdated Show resolved Hide resolved

lib/resty/openssl/revoked.lua Outdated Show resolved Hide resolved

lib/resty/openssl/revoked.lua Outdated Show resolved Hide resolved

lib/resty/openssl/x509/csr.lua Show resolved Hide resolved

lib/resty/openssl/include/x509/crl.lua Outdated Show resolved Hide resolved

lib/resty/openssl/x509/crl.lua Outdated Show resolved Hide resolved

lib/resty/openssl/x509/csr.lua Outdated Show resolved Hide resolved

lib/resty/openssl/x509/csr.lua Outdated Show resolved Hide resolved

fffonion added the enhancement label

Owner

fffonion commented Aug 6, 2020

I've fixed the CI, please rebase to master when appropriate.

nasrullo added 19 commits

August 6, 2020 08:41


          removed varargs


          Removed varargs and cached methods

95aa829


          Added support for revoked list and getting csr extensions

eb6bfa0


          Added support for revoked list and getting csr extensions

5e07ec0


          Added support for revoked list and getting csr extensions

b4a573c


          removed varargs

9d69620


          Removed varargs and cached methods

7118bc9


          Added support for revoked list and getting csr extensions

9d1accb


          Added support for revoked list and getting csr extensions

7576fb2


          Minor commit to trigger build

c33254b


          removed varargs

aeba9a9


          Removed varargs and cached methods

ebe4bb2


          Added support for revoked list and getting csr extensions

655c21f


          Added support for revoked list and getting csr extensions

a7812d4


          removed varargs

3df0d47


          Removed varargs and cached methods

21d44df


          Added support for revoked list and getting csr extensions

3442ba4


          Added support for revoked list and getting csr extensions

2a1feea


          Removed dupe code

c624769

nasrullo force-pushed the csr branch from 7d9409d to c624769 Compare

August 6, 2020 12:42

nasrullo added 3 commits

August 6, 2020 09:53


          Addressed PR review changes


          Adds sn check and passing bn instance support

5280e8c


          Added include revoked to rockspec

23f6c5b

fffonion reviewed

View reviewed changes

Owner

fffonion left a comment

This is becoming in a good shape! I'd also humbly ask for some test on those new functions. For now each function with a happy path case will be good.

lib/resty/openssl/include/x509/name.lua Outdated Show resolved Hide resolved

lib/resty/openssl/x509/csr.lua Show resolved Hide resolved

lib/resty/openssl/revoked.lua Outdated Show resolved Hide resolved

lib/resty/openssl/revoked.lua Outdated Show resolved Hide resolved

nasrullo added 2 commits

August 6, 2020 12:51


          Addressed PR review change requests

a5ca2ba


          Adds tests and fixes crl sign method

b7760ee

nasrullo added 2 commits

August 7, 2020 00:21


          fixed missing declaration for symbol 'OPENSSL_sk_value

7da15b6


          Added csr sign test

bd92a2c

fffonion reviewed

View reviewed changes

t/openssl/x509/crl.t Outdated Show resolved Hide resolved

lib/resty/openssl/x509/revoked.lua Show resolved Hide resolved

lib/resty/openssl/x509/revoked.lua Show resolved Hide resolved

lib/resty/openssl/x509/csr.lua Outdated Show resolved Hide resolved

lua-resty-openssl-0.6.3-1.rockspec Outdated Show resolved Hide resolved

t/openssl/x509/csr.lua Outdated Show resolved Hide resolved

nasrullo added 10 commits

August 7, 2020 06:57


          Addressed PR review changes

7bd597c


          Removed checks


          Fixed test

ca7bea3


          Fixed typo

5f236b6


          Fixed typo

2e40cb9


          Fixed typo

9c3041a


          Changed get_extension method

fbcb80e


          Changed get_extension method

8a8fa13


          Removed concat

73694e4


          Removed concat

fffonion force-pushed the master branch from eabb7a2 to f4cf725 Compare

August 9, 2020 09:03

fffonion approved these changes

View reviewed changes

lib/resty/openssl/x509/csr.lua Outdated

+                          push(ret, obj)
+                      end
+                  end
+                  return ret

Owner

fffonion Aug 9, 2020

we should return the stack itself rather than a table here. i just finished the extensions module today, will merge as is and update to use that later.

Contributor Author

nasrullo Aug 9, 2020

Agree, Changed. I only need get_extension. BTW, Did you review new logic in get extension method? What do you think? I did not find any method to directly get single extension without getting all

Owner

fffonion Aug 9, 2020

Yeah i do have some investigation a while ago, the API for X509_REQ is bit different for others. But it should be fine, from I can see X509_REQ_get_extension returns a internal pointer and not duplicating the stack.

fffonion mentioned this pull request

feat(x509.*) add revoked module and get_extension/get_extensions for x509.csr #9

Merged

nasrullo added 2 commits

August 9, 2020 11:03


          Implemented PR review changes

45bc63f


          Changed get_extensions test

605e6fa

Contributor Author

nasrullo commented Aug 9, 2020 via email

thank you

On Sun, Aug 9, 2020 at 11:34 AM Wangchong Zhou ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/resty/openssl/x509/csr.lua <#8 (comment)> : > +function _M.get_extensions(self) + local extensions = C.X509_REQ_get_extensions(self.ctx) + ffi_gc(extensions, stack_macro.OPENSSL_sk_free) + local n = stack_macro.OPENSSL_sk_num(extensions) + local ret = {} + for i = 0, n - 1 do + local ext = stack_macro.OPENSSL_sk_value(extensions, i) + ffi_gc(ext, C.X509_EXTENSION_free) + local dup, err = extension_lib.dup(ffi_cast(ext_typ_ptr, ext)) + if not err then + local obj = dup:get_object() + obj.blob = dup:tostring() + push(ret, obj) + end + end + return ret Yeah i do have some investigation a while ago, the API for X509_REQ is bit different for others. But it should be fine, from I can see X509_REQ_get_extension returns a internal pointer and not duplicating the stack. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCWUDNUW3Q5HTG4VQBIN33R726YZANCNFSM4PWDQ6DQ> .

Owner

fffonion commented Aug 9, 2020

I'm going to do a manual rebase and merge with #9.

Contributor Author

nasrullo commented Aug 9, 2020 via email

Ok, I sent you an invite to chat

On Sun, Aug 9, 2020 at 11:37 AM Wangchong Zhou ***@***.***> wrote: I'm going to do a manual rebase and merge with #9 <#9>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCWUDIYO5BY2YD7TLIQHRDR727DRANCNFSM4PWDQ6DQ> .

Owner

fffonion commented Aug 9, 2020

Manually merged in with #9

fffonion closed this

Owner

fffonion commented Aug 9, 2020

@nasrullo I'll add you to credits tomorrow.

Contributor Author

nasrullo commented Aug 9, 2020 via email

Can you bump up rockspec and publish it now

On Sun, Aug 9, 2020 at 1:25 PM Wangchong Zhou ***@***.***> wrote: @nasrullo <https://github.com/nasrullo> I'll add you to credits tomorrow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCWUDMUMUDYA3I4ZBLAB6LR73LWXANCNFSM4PWDQ6DQ> .

Owner

fffonion commented Aug 9, 2020

@nasrullo Sure, just published.

Contributor Author

nasrullo commented Aug 9, 2020 via email

We need to remove varars from :update methods, it is very slow. I am using digest methods per request and it makes it slow, what do you think?

On Sun, Aug 9, 2020 at 1:36 PM Wangchong Zhou ***@***.***> wrote: @nasrullo <https://github.com/nasrullo> Sure, just published. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCWUDOWJTI5WY664L6FZ4LR73NCVANCNFSM4PWDQ6DQ> .

Owner

fffonion commented Aug 10, 2020

@nasrullo please open a new issue and don't reply in closed issue. This will help other people find answer easier.

zhuizhuhaomeng added a commit to zhuizhuhaomeng/lua-resty-openssl that referenced this pull request


          fixed use-after-free in bn.lua.

c0ee094

READ of size 4 at 0x60300004fba8 thread T0
    #0 0x7ffff6d96fb4 in BN_get_word crypto/bn/bn_lib.c:411
    fffonion#1 0x555555ca9d98  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x755d98)
    fffonion#2 0x555555d7149f in lj_ccall_func /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lj_ccall.c:1402
    fffonion#3 0x555555ca35b7 in lj_cf_ffi_meta___call /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lib_ffi.c:230
    fffonion#4 0x555555ca7773  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x753773)
    fffonion#5 0x55555599e140 in ngx_http_lua_run_thread ../ngx_lua-0.10.26.8/src/ngx_http_lua_util.c:1190
    fffonion#6 0x5555559a9d21 in ngx_http_lua_content_by_chunk ../ngx_lua-0.10.26.8/src/ngx_http_lua_contentby.c:124
    fffonion#7 0x55555575d41d in ngx_http_core_content_phase src/http/ngx_http_core_module.c:1269
    fffonion#8 0x555555748024 in ngx_http_core_run_phases src/http/ngx_http_core_module.c:885
    fffonion#9 0x55555577348d in ngx_http_process_request src/http/ngx_http_request.c:2130
    fffonion#10 0x5555557749a6 in ngx_http_process_request_headers src/http/ngx_http_request.c:1529
    fffonion#11 0x5555557758c4 in ngx_http_process_request_line src/http/ngx_http_request.c:1196
    fffonion#12 0x55555570fb1c in ngx_epoll_process_events src/event/modules/ngx_epoll_module.c:968
    fffonion#13 0x5555556e5706 in ngx_process_events_and_timers src/event/ngx_event.c:262
    fffonion#14 0x55555570b323 in ngx_single_process_cycle src/os/unix/ngx_process_cycle.c:338
    fffonion#15 0x555555660ef4 in main src/core/nginx.c:403
    fffonion#16 0x7ffff683feaf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    fffonion#17 0x7ffff683ff5f in __libc_start_main_impl ../csu/libc-start.c:389
    fffonion#18 0x5555556648f4 in _start (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x1108f4)

0x60300004fba8 is located 8 bytes inside of 24-byte region [0x60300004fba0,0x60300004fbb8)
freed by thread T0 here:
    #0 0x7ffff74b46b7 in free (/lib64/libasan.so.6+0xb46b7)
    fffonion#1 0x7ffff6ea66e7 in CRYPTO_free crypto/mem.c:312
    fffonion#2 0x7ffff6d9810e in BN_free crypto/bn/bn_lib.c:231
    fffonion#3 0x555555ca9d98  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x755d98)
    fffonion#4 0x555555d7149f in lj_ccall_func /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lj_ccall.c:1402
    fffonion#5 0x555555ca35b7 in lj_cf_ffi_meta___call /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lib_ffi.c:230
    fffonion#6 0x555555ca7773  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x753773)

previously allocated by thread T0 here:
    #0 0x7ffff74b4a07 in __interceptor_malloc (/lib64/libasan.so.6+0xb4a07)
    fffonion#1 0x7ffff6ea66bc in CRYPTO_malloc crypto/mem.c:222
    fffonion#2 0x7ffff6ea6807 in CRYPTO_zalloc crypto/mem.c:230
    fffonion#3 0x7ffff6d96c15 in BN_new crypto/bn/bn_lib.c:246
    fffonion#4 0x555555ca9d98  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x755d98)
    fffonion#5 0x555555d7149f in lj_ccall_func /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lj_ccall.c:1402
    fffonion#6 0x555555ca35b7 in lj_cf_ffi_meta___call /usr/src/debug/openresty-plus-1.19.9.1.65/build/LuaJIT-plus-2.1-20240710/src/lib_ffi.c:230
    fffonion#7 0x555555ca7773  (/usr/local/openresty-plus-asan/nginx/sbin/nginx+0x753773)

zhuizhuhaomeng mentioned this pull request

fixed use-after-free in bn.lua. #173

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels