[PATCH 04 of 11] SSL: explicit session id length checking

[Sergey Kandaurov]

> On 26 Aug 2022, at 07:01, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1661481949 -10800
> #      Fri Aug 26 05:45:49 2022 +0300
> # Node ID f4ae0f4ee928cf20346530e96f1431314ecd0171
> # Parent  86d827338fdd13ea899d618b0bcb2be23469cbac
> SSL: explicit session id length checking.
> 
> Session ids are not expected to be longer than 32 bytes, but this is
> theoretically possible with TLSv1.3, where session ids are essentially
> arbitrary and sent as session tickets.  Since on 64-bit platforms we
> use fixed 32-byte buffer for session ids, added an explicit length check
> to make sure the buffer is large enough.
> 

I don't follow how session ids could be "essentially arbitrary"
(except a library bug that justifies such safety belt).
For TLSv1.3, this callback is used to update session cache as part
of constructing NewSessionTicket.  It's called after generating
dummy session ids, which, and regardless of protocol version, are
capped to SSL3_SSL_SESSION_ID_LENGTH (32).

> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -3842,6 +3842,14 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
>     p = buf;
>     i2d_SSL_SESSION(sess, &p);
> 
> +    session_id = (u_char *) SSL_SESSION_get_id(sess, &session_id_length);
> +
> +    /* do not cache sessions with too long session id */
> +
> +    if (session_id_length > 32) {
> +        return 0;
> +    }
> +

The check can be moved above the cpu expensive i2d_SSL_SESSION call.
Or rather move i2d_SSL_SESSION closer to corresponding ngx_memcpy().
(but see my reply on the next patch)

>     c = ngx_ssl_get_connection(ssl_conn);
> 
>     ssl_ctx = c->ssl->session_ctx;
> @@ -3886,8 +3894,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
>         }
>     }
> 
> -    session_id = (u_char *) SSL_SESSION_get_id(sess, &session_id_length);
> -
> #if (NGX_PTR_SIZE == 8)
> 
>     id = sess_id->sess_id;
> 

-- 
Sergey Kandaurov