[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openssl-dev
Subject:    [openssl-dev] [openssl.org #3882] [BUGFIX] lh_SSL_SESSION_delete() not checked
From:       "Richard Levitte via RT" <rt () openssl ! org>
Date:       2015-05-31 20:46:18
Message-ID: rt-4.0.4-23712-1433105178-1432.3882-6-0 () openssl ! org
[Download RAW message or body]

Hmm, but does it? If you look for the comment '/* We *are* in trouble ... */'
in ssl_sess.c, you'll see that there is a similar kind of protection in place
already at the time of insert. So quite frankly and with all respect, I'm not
sure if this particular fix does anything of value any longer.

On Sun May 31 22:28:18 2015, tshort@akamai.com wrote:
> We (Akamai) had a bad session compare function at one point; the
> compare was fixed, but also added this change to protect the LHASH.
>
> So, yes, this can only really happen if one has a bad comparison
> function.
>
> --
> -Todd Short
> // tshort@akamai.com
> // Sent from my iPhone
> // "One if by land, two if by sea, three if by the Internet."
>
>
> > On May 31, 2015, at 4:22 PM, Richard Levitte via RT <rt@openssl.org>
> wrote:
> >
> > I'm not sure how that can happen, as each SSL_SESSION in that lhash
> will have
> > unique content. This is assured by the way lh_insert functions and
> by
> > ssl_session_cmp (which gets called by getrn in lhash.c, via the
> function
> > pointer cf).
> >
> > So while your suggestion will most probably work as a band aid, I'm
> thinking
> > you've really found a bug in ssl_session_cmp or in the lhash code
> itself. Could
> > you verify?
> >
> >> On Sun May 31 21:24:04 2015, tshort@akamai.com wrote:
> >> No,
> >>
> >> The second code sample removes a matching instance, but not
> >> necessarily the same instance. If they are not the same instance,
> then
> >> it would need to be re-inserted in and else clause.
> >>
> >> This is a fine distinction.
> >>
> >> This would leave to having the list and hash not contain the same
> >> contents: Either the number of items is different, or the two sets
> of
> >> items are different.
> >>
> >> There's a similar example in the code, I believe, but I'd have to
> >> search for it.
> >>
> >> --
> >> -Todd Short
> >> // tshort@akamai.com
> >> // Sent from my iPhone
> >> // "One if by land, two if by sea, three if by the Internet."
> >>
> >>
> >>>> On May 31, 2015, at 12:43 PM, Richard Levitte via RT
> >>> <rt@openssl.org> wrote:
> >>>
> >>> You solution does the following:
> >>>
> >>> if (lh_SSL_SESSION_retrieve(p->cache, s) == s) {
> >>> (void)lh_SSL_SESSION_delete(p->cache, s);
> >>> ...
> >>>
> >>> Would you agree that the following does the same?
> >>>
> >>> if (lh_SSL_SESSION_delete(p->cache, s) == s) {
> >>> ...
> >>>
> >>>
> >>>> On Sat May 30 09:48:06 2015, tshort@akamai.com wrote:
> >>>> Hello OpenSSL Org:
> >>>>
> >>>> This is a change that Akamai has made to its
> >>>> implementation of OpenSSL.
> >>>>
> >>>> Version: master branch
> >>>> Description:
> >>>> lh_SSL_SESSION_delete() not checked
> >>>>
> >>>> Fix an OpenSSL issue where the
> >>>> return code of lh_SSL_SESSION_delete()
> >>>> is not checked, causing a
> >>>> stale reference in the lhash.
> >>>>
> >>>> Github link:
> >
>
https://github.com/akamai/openssl/commit/3a114c2f0e3bf241732fef7a2d339a230ca68abc
> >>>> And attachment.
> >>>>
> >>>> Thank you.
> >>>> --
> >>>> -Todd Short
> >>>> // tshort@akamai.com
> >>>> // "One if by land, two if by sea, three if by the Internet."
> >>>
> >>>
> >>> --
> >>> Richard Levitte
> >>> levitte@openssl.org
> >
> >
> > --
> > Richard Levitte
> > levitte@openssl.org
> >
>


--
Richard Levitte
levitte@openssl.org

_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

[prev in list] [next in list] [prev in thread] [next in thread]