log session_id #1147
log session_id #1147
Conversation
|
trying a rebuild, i don't understand why this aborted. |
|
|
||
| switch (sock->ssl->handshake.server.async_resumption.state) { | ||
| case ASYNC_RESUMPTION_STATE_COMPLETE: | ||
| id = SSL_SESSION_get_id(sock->ssl->ssl->session, &id_len); |
There was a problem hiding this comment.
Please use SSL_get_session to avoid issues with OpenSSL 1.1.0 (IIRC direct access to members is forbidden in 1.1.x).
Also, I believe that there's a chance for the session pointer to be NULL, and think that doing a NULL check is necessary.
| switch (sock->ssl->handshake.server.async_resumption.state) { | ||
| case ASYNC_RESUMPTION_STATE_COMPLETE: | ||
| id = SSL_SESSION_get_id(sock->ssl->ssl->session, &id_len); | ||
| #define BASE64_LENGTH(len) (((len) + 2) / 3 * 4 + 1) |
There was a problem hiding this comment.
How about promoting this macro into a inline function defined in string_.h?
Regarding the name of the inline function, I think it should be named something like h2o_base64_encode_capacity or something that reflect the fact that the returned value is not the exact number of bytes required to store the output. The returned value always takes trailing = and NUL character into account.
| key.len = h2o_base64_encode(key.base, id, id_len, 1); | ||
| return key; | ||
| default: | ||
| return h2o_iovec_init(H2O_STRLIT("INVALID_SESSION_ID")); |
There was a problem hiding this comment.
I think we should return - in case the session ID does not exist. - is the character used to express the absense of the values in the access log.
missing: <<The returned value always takes trailing = and NUL character into account.>>
- get session via SSL_get1_session - check for empty session - use "-" for nil log entry
|
i tried to fix the issues, but i'm unsure about the correct algo for http://stackoverflow.com/questions/1533113/calculate-the-size-to-a-base-64-encoded-message |
missing cleanup when the connection upgrade fails
On upgrade error, make sure we're closing the HTTP/1 socket before giving control to the HTTP/2 error handling.
|
moved to #1164 , sorry for the mess |
it should be possible to log the TLS session_id, this PR implements that.
however i'm unsure if its the right way and if the case statement is needed