validate and expose nextUpdate field in OCSP response #1694
Conversation
ElvinEfendi
force-pushed
the
ocsp-nextupdate
branch
from
c122a4e to
faad7f1
Compare
|
For tests see openresty/lua-resty-core#296 (comment) |
ElvinEfendi
force-pushed
the
ocsp-nextupdate
branch
from
faad7f1 to
9892463
Compare
src/ngx_http_lua_ssl_ocsp.c
Outdated
|
|
||
| static time_t ngx_http_lua_ssl_stapling_time(ASN1_GENERALIZEDTIME *asn1time); |
There was a problem hiding this comment.
Please always use std type names like long in our ABI functions to avoid any potential compatibility issues.
| @@ -383,6 +384,16 @@ ngx_http_lua_ffi_ssl_validate_ocsp_response(const u_char *resp, | |||
| goto error; | |||
| } | |||
|
|
|||
| if (nextupdate) { | |||
There was a problem hiding this comment.
Shall we explicitly test if nextupdate is equal to zero instead of testing it as a boolean value?
There was a problem hiding this comment.
It's a double pointer to ASN1_GENERALIZEDTIME type, I don't know if we can easily compare it to zero. Also NGINX follows this approach: /~https://github.com/nginx/nginx/blob/7547581bbcb7b737710f9141260d822a08685b83/src/event/ngx_event_openssl_stapling.c#L2392
| @@ -437,6 +448,39 @@ ngx_http_lua_ssl_empty_status_callback(ngx_ssl_conn_t *ssl_conn, void *data) | |||
| { | |||
| return SSL_TLSEXT_ERR_OK; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
We use 2 blank lines to separate function definitions.
|
|
||
| /* fake weekday prepended to match C asctime() format */ | ||
|
|
||
| BIO_write(bio, "Tue ", sizeof("Tue ") - 1); |
There was a problem hiding this comment.
Shall we check the return values of BIO_write() for any errors here?
There was a problem hiding this comment.
Since the input is hardcoded, there's probably not much value in testing that. But if you feel strongly about it I can check the return value too.
ElvinEfendi
force-pushed
the
ocsp-nextupdate
branch
from
9892463 to
5c0e3c5
Compare
zhuizhuhaomeng
force-pushed
the
master
branch
from
f924579 to
fef2581
Compare
|
@ElvinEfendi I would like to merge this PR. |
zhuizhuhaomeng
force-pushed
the
ocsp-nextupdate
branch
from
5c0e3c5 to
4049c27
Compare
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.
Required by openresty/lua-resty-core#296
This is an alternative solution to #1041. I like the approach in this PR better because it adds minimal amount of code and it makes sense for validation API to return this data.
The code has been fork lifted from Nginx source and adjusted here.
In addition to exposing
nextUpdatefield, this way we will also be validatingnextUpdatefield.