Hi Baptiste, On Wed, Nov 28, 2018 at 03:33:20PM +0100, Baptiste wrote: > Hi there, > > There is a small bug with the function involved in the ssl_fc_cipherlist_* > fetches. > ssl_sock_parse_clienthello() improperly parses the session id, which leads > to not return any client side cipher list when a client send a SSL session > ID to resume a session. > This patch aims at fixing this issue.
Thanks for this. Would you happen to have a test case for this ? I *think* it's correct based on some other sample fetch functions which also take a single byte for the session ID length, but if you have a simple test case it would be great. Also CCing Thierry who developed it in case he can test it. > From f2c79803c6bcb69866f54c8a5833bd0178bea64c Mon Sep 17 00:00:00 2001 > From: Baptiste Assmann <bed...@gmail.com> > Date: Wed, 28 Nov 2018 15:20:25 +0100 > Subject: [PATCH] BUG/MINOR: ssl: ssl_sock_parse_clienthello ignores session id > > In ssl_sock_parse_clienthello(), the code considers that SSL Sessionid > size is '1', and then considers that the SSL cipher suite is availble > right after the session id size information. > This actually works in a single case, when the client does not send a > session id. > > This patch fixes this issue by introducing the a propoer way to parse > the session id and move forward the cursor by the session id length when > required. > --- > src/ssl_sock.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/src/ssl_sock.c b/src/ssl_sock.c > index a73fb2d..c48c4af 100644 > --- a/src/ssl_sock.c > +++ b/src/ssl_sock.c > @@ -1561,10 +1561,19 @@ void ssl_sock_parse_clienthello(int write_p, int > version, int content_type, > > /* Expect 2 bytes for protocol version (1 byte for major and 1 byte > * for minor, the random, composed by 4 bytes for the unix time and > - * 28 bytes for unix payload, and them 1 byte for the session id. So > - * we jump 1 + 1 + 4 + 28 + 1 bytes. > + * 28 bytes for unix payload. So we jump 1 + 1 + 4 + 28. > */ > - msg += 1 + 1 + 4 + 28 + 1; > + msg += 1 + 1 + 4 + 28;; Just a minor detail, extra semi-column above. I'll fix it once merged, don't resend for this. > + if (msg > end) > + return; > + > + /* Next, is session id: > + * if present, we have to jump by is length + 1 for the size information extra "is" here, same, no need to resend. > + * if not present, we have to jump by 1 only > + */ > + if (msg[0] > 0) > + msg += msg[0]; > + msg += 1; > if (msg > end) > return; Thank you, Willy
