feature: Added ocsp.get_ocsp_nextupdate

[alubbe]

This PR is a possible solution to #75 and is complemented by openresty/lua-nginx-module#1041.

It is missing more tests, mainly because none of the existing mock ocsp responses contain the nextUpdate field. I will need help on how to add those. I can confirm that this works as intended on the OCSP responses sent by Let's Encrypt.

Looking forward to your feedback.

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

[alubbe]

I don't quite understand how all previous ocsp tests are now failing. How did my C code influence the other functions? Was is it reusing this nginx-internal function under its same name? /~https://github.com/openresty/lua-nginx-module/pull/1041/files#diff-50267b7dd63c740bc5c1d29c7387e789R437

[agentzh]

@alubbe You'll have to debug things locally. Using travis ci to debug failures is painful since you cannot do things interactively.

[agentzh]

Travis CI is useful for catching things, not really for debugging failures.

[alubbe]

@agentzh You're touching on two points here.
Firstly, I've not once managed to run the test suite locally in the same env that travis uses. The closest I've come was in an ethereal docker container using manual commands, but even that worked only for a subset of the test. I might be spoiled by the simplicity of running tests in ruby, js, python and rust, but I've tried earnestly and failed to run the openresty tests as a whole. It'd be great for there to be a maintained dockerfile for the test suite.

Secondly, I don't know where to start with the failures. Because I cannot reproduce travis, I instead applied this and the other PR to 1.11.2, compiled it locally, created a new openresty project, copied over the certificates manually and ran the commands of the test run manually. Everything works as expected locally. Why are tests failing that I didn't knowingly touch? Any pointers would be very helpful.

Lastly, how would you like to go about adding nextUpdate fields to your mock certs? Would you accept a PR that modifies them or do you want to be the one to do it?

[agentzh]

@alubbe We don't have any problems running the test suite in various different environments (Mac OS X, FreeBSD, Fedora Linux, Amazon Linux). So I wonder maybe you just mess up some configuration details yourself. We never use docker for our everyday development and testing.

[agentzh]

@alubbe I never like the test toolchain in the python world BTW. And I also had a lot of pain when doing this in the JS world a few years ago. So do not blame our testing toolchain.

[agentzh]

@alubbe The depth and breadth of our test suite are rare. We even some times catch deep bugs in both the nginx core and luajit in their new releases. So don't blame the toolchain when you cannot get the tests pass. The tests are designed to fail easily instead of pass easily. That's the whole point of testing.

[agentzh]

@alubbe We've already been putting years of efforts in making the test toolchain as handy as possible. So if you think there's anything that can be improved. Just let us know. You can always create tickets in the openresty/test-nginx repo. Again, we welcome constructive suggestions instead of destructive ones since the latter are seldom or never helpful.

[alubbe]

@agentzh I'm sorry if I came off destructive or dismissive, I did not mean to. I highly appreciate the work and effort that you and the team put into openresty, and also its test suite!

I'm also sorry that this distracted from the other questions I was asking. So I'll try again:

1. The failing tests are ones where I touched neither the test or the implementations. Locally, everything passes. I really need a hand here to understand why this is happening - the changeset in C and lua is pretty small and, to me, seems isolated from the rest.
2. How would you like to go about adding nextUpdate fields to your mock certs? I do not know how they were generated and, in particular, if you used a private key somewhere. Would you accept a PR that modifies them or do you want to be the one to do it?

[agentzh]

@alubbe I can easily reproduce those test failures in the t/ocsp.t file of Travis CI in my local environment when using your branch in this PR.

Switching to master branch makes the whole test suite of lua-resty-core pass again.

By adding a --- ONLY section to TEST 1 of the t/ocsp.t file and run this file with prove again. This test case is indeed failing:

$ prove -I../test-nginx/lib t/ocsp.t
t/ocsp.t .. # I found ONLY: maybe you're debugging?
t/ocsp.t .. 1/38
#   Failed test 'TEST 1: get OCSP responder (good case) - response_body - response is expected (repeated req 0, req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1382.
# @@ -1,2 +1,2 @@
#  connected: 1
# -ssl handshake: userdata
# +failed to do SSL handshake: handshake failed

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "lua ssl server name: "test.com"" should match a line in error.log (req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1146.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "OCSP url found: http://127.0.0.1:8888/ocsp?foo=1," should match a line in error.log (req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1146.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "[error]" should not match any line in error.log but matches line "2017/04/19 13:24:47 [error] 45980\#0: *4 lua entry thread aborted: runtime error: /home/agentzh/git/lua-resty-core/lib/ngx/ocsp.lua:41: declaration specifier expected near 'time_t' at line 14" (req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1201.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "[error]" should not match any line in error.log but matches line "2017/04/19 13:24:47 [error] 45980\#0: *1 SSL_do_handshake() failed (SSL: error:14077438:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert internal error), client: 127.0.0.1, server: localhost, request: \"GET /t HTTP/1.1\", host: \"localhost\"" (req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1201.
WARNING: TEST 1: get OCSP responder (good case) - 2017/04/19 13:24:47 [crit] 45980#0: *3 SSL_do_handshake() failed (SSL: error:1408A179:SSL routines:ssl3_get_client_hello:cert cb error) while SSL handshaking, client: unix:, server: unix:/home/agentzh/git/lua-resty-core/t/servroot/html/nginx.sock at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1236.

#   Failed test 'TEST 1: get OCSP responder (good case) - response_body - response is expected (repeated req 1, req 0)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1382.
# @@ -1,2 +1,2 @@
#  connected: 1
# -ssl handshake: userdata
# +failed to do SSL handshake: handshake failed

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "lua ssl server name: "test.com"" should match a line in error.log (req 1)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1146.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "OCSP url found: http://127.0.0.1:8888/ocsp?foo=1," should match a line in error.log (req 1)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1146.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "[error]" should not match any line in error.log but matches line "2017/04/19 13:24:47 [error] 45980\#0: *8 lua entry thread aborted: runtime error: ssl_certificate_by_lua:3: loop or previous error loading module 'ngx.ocsp'" (req 1)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1201.

#   Failed test 'TEST 1: get OCSP responder (good case) - pattern "[error]" should not match any line in error.log but matches line "2017/04/19 13:24:47 [error] 45980\#0: *5 SSL_do_handshake() failed (SSL: error:14077438:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert internal error), client: 127.0.0.1, server: localhost, request: \"GET /t HTTP/1.1\", host: \"localhost\"" (req 1)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1201.
WARNING: TEST 1: get OCSP responder (good case) - 2017/04/19 13:24:47 [crit] 45980#0: *7 SSL_do_handshake() failed (SSL: error:1408A179:SSL routines:ssl3_get_client_hello:cert cb error) while SSL handshaking, client: unix:, server: unix:/home/agentzh/git/lua-resty-core/t/servroot/html/nginx.sock at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1236.
t/ocsp.t .. Failed 32/38 subtests

Then checking the nginx error logs produced by this individual test block in the file t/servroot/logs/error.log, I'm seeing the following messages:

2017/04/19 13:24:47 [error] 45980#0: *4 lua entry thread aborted: runtime error: /home/agentzh/git/lua-resty-core/lib/ngx/ocsp.lua:41: declaration specifier expected near 'time_t' at line 14
stack traceback:
coroutine 0:
    [C]: in function 'require'
    ssl_certificate_by_lua:3: in function <ssl_certificate_by_lua:1>, context: ssl_certificate_by_lua*, client: unix:, server: unix:/home/agentzh/git/lua-resty-core/t/servroot/html/nginx.sock

So obviously LuaJIT does not recognize the time_t type. To verify this:

$ resty -e 'require "ffi".new[[ time_t[1] ]]'
(command line -e):1: declaration specifier expected near 'time_t'
stack traceback:
	(command line -e):1: in function 'inline_gen'
	init_worker_by_lua:36: in function <init_worker_by_lua:35>
	[C]: in function 'xpcall'
	init_worker_by_lua:44: in function <init_worker_by_lua:42>

I just wonder how you could make these tests pass on your side.

I documented the steps I used to troubleshoot the test failure. Hopefully you can learn from this.

[agentzh]

@alubbe Regarding the test certificates, I suggest you add a shell script under t/cert/ to include openssl command lines used to generate those certificates and keys. So that other people can easily re-generate them in the future if they have to.

[alubbe]

Thanks @agentzh that's exactly how I've been running individual tests. Good to know I'm doing that correctly.

And I've found the culprit. Locally, I was compiling another module for benchmarking that was adding the time_t definition globally - what are the chances...

[agentzh]

@alubbe So...it is really not our test suite or test toolchain's fault ;)

[alubbe]

Nope, it's the novice user ;)

Btw, before we forget, do we need /~https://github.com/openresty/lua-nginx-module/pull/782/files#diff-c4f8697714ffd4ae8e3d403bc9a55715R17 ?

[alubbe]

Thwarted again by a flaky test - could someone restart the failed test run?

[agentzh]

@alubbe Maybe you should just enable travis ci in your own fork? It's easier.

[alubbe]

@alubbe Regarding the test certificates, I suggest you add a shell script under t/cert/ to include openssl command lines used to generate those certificates and keys. So that other people can easily re-generate them in the future if they have to.

@agentzh Given my lack of familiarity with the openssl cli, I don't feel comfortable putting together those scripts, esp. seeing as re-generating the certs shouldn't fail existing tests. Would it be possible for you or other maintainers to create the script file and then I can build upon that?

[agentzh]

@alubbe Regarding openssl cli, just google for commands. There are many blog posts and documentation over the Internet.

Regarding re-generation, no, you only need to record the commands in a bash script. It's not your responsibility to automatically accommodate the existing tests when the certs and keys are re-generated in the future.

[alubbe]

Gotcha. I'll start by recording only the command needed for this one additional ocsp response, if that's alright. We could open a second PR to add the ones for the older files.

For this PR, we would just need to add -ndays 2 (or some other value) to the commands used to generate the OCSP responses.

I've read through several blog posts and the man pages, and I found the last example here the most relevant: https://wiki.openssl.org/index.php/Manual:Ocsp(1)#EXAMPLES

Having said that, I needed an index file. What did you use for that in the past? I simpled created an empty index.txt and an index.txt.attr with the content unique_subject = no.

This is the command that I ended up using:

# pwd is t/cert
openssl ocsp -index index.txt -rsigner chain/chain.pem -rkey chain/test-com.key.pem -CA chain/root-ca.crt -reqin ocsp/ocsp-req.der -respout ocsp/ocsp-resp-nextupdate.der -ndays 2

This gave me a working OCSP response, however when calling validate_ocsp_response in lua I get this error: certificate status "unknown" in the OCSP response.

What am I missing?

[dav-is]

@alubbe Were you able to figure out a way to cache OCSP responses without this PR?

[alubbe]

I'm afraid no :(

[dav-is]

Would it be possible to push this forward? 🙏

[alubbe]

I'm afraid I'm still as stuck as I was before - if anyone can read through my comments and has a new idea, that could be a starting point