Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 02 Nov 2016 17:17:04 -0700
From:      Cy Schubert <Cy.Schubert@komquats.com>
To:        Oliver Pinter <oliver.pinter@hardenedbsd.org>
Cc:        Cy Schubert <Cy.Schubert@komquats.com>, Xin LI <delphij@freebsd.org>, svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   Re: svn commit: r308200 - in stable: 10/crypto/openssl/ssl  9/crypto/openssl/ssl
Message-ID:  <201611030017.uA30H4UJ078504@slippy.cwsent.com>
In-Reply-To: Message from Oliver Pinter <oliver.pinter@hardenedbsd.org> of "Wed, 02 Nov 2016 15:01:48 %2B0100." <CAPQ4ffuO_%2B2z1WTKJ78m3ayr5Hvh0EHiMJZ-4soba=kV5SS%2BUQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
In message <CAPQ4ffuO_+2z1WTKJ78m3ayr5Hvh0EHiMJZ-4soba=kV5SS+UQ@mail.gmail.c
om>
, Oliver Pinter writes:
> On 11/2/16, Cy Schubert <Cy.Schubert@komquats.com> wrote:
> > In message <201611020709.uA279WM3070566@repo.freebsd.org>, Xin LI writes:
> >> Author: delphij
> >> Date: Wed Nov  2 07:09:31 2016
> >> New Revision: 308200
> >> URL: https://svnweb.freebsd.org/changeset/base/308200
> >>
> >> Log:
> >>   Backport OpenSSL commit af58be768ebb690f78530f796e92b8ae5c9a4401:
> >>
> >>     Don't allow too many consecutive warning alerts
> >>
> >>     Certain warning alerts are ignored if they are received. This can mean
> >> th
> >> at
> >>     no progress will be made if one peer continually sends those warning
> >> aler
> >> ts.
> >>     Implement a count so that we abort the connection if we receive too
> >> many.
> >>
> >>     Issue reported by Shi Lei.
> >>
> >>   This is a direct commit to stable/10 and stable/9.
> >>
> >>   Security:	CVE-2016-8610
> >>
> >> Modified:
> >>   stable/10/crypto/openssl/ssl/d1_pkt.c
> >>   stable/10/crypto/openssl/ssl/s3_pkt.c
> >>   stable/10/crypto/openssl/ssl/ssl.h
> >>   stable/10/crypto/openssl/ssl/ssl3.h
> >>   stable/10/crypto/openssl/ssl/ssl_locl.h
> >>
> >> Changes in other areas also in this revision:
> >> Modified:
> >>   stable/9/crypto/openssl/ssl/d1_pkt.c
> >>   stable/9/crypto/openssl/ssl/s3_pkt.c
> >>   stable/9/crypto/openssl/ssl/ssl.h
> >>   stable/9/crypto/openssl/ssl/ssl3.h
> >>   stable/9/crypto/openssl/ssl/ssl_locl.h
> >>
> >> Modified: stable/10/crypto/openssl/ssl/d1_pkt.c
> >> ==========================================================================
> ===
> >> =
> >> --- stable/10/crypto/openssl/ssl/d1_pkt.c	Wed Nov  2 06:58:47 2016
> >> 	(r308199)
> >> +++ stable/10/crypto/openssl/ssl/d1_pkt.c	Wed Nov  2 07:09:31 2016
> >> 	(r308200)
> >> @@ -924,6 +924,13 @@ int dtls1_read_bytes(SSL *s, int type, u
> >>          goto start;
> >>      }
> >>
> >> +    /*
> >> +     * Reset the count of consecutive warning alerts if we've got a
> >> non-empt
> >> y
> >> +     * record that isn't an alert.
> >> +     */
> >> +    if (rr->type != SSL3_RT_ALERT && rr->length != 0)
> >> +        s->s3->alert_count = 0;
> >> +
> >>      /* we now have a packet which can be read and processed */
> >>
> >>      if (s->s3->change_cipher_spec /* set when we receive
> >> ChangeCipherSpec,
> >> @@ -1190,6 +1197,14 @@ int dtls1_read_bytes(SSL *s, int type, u
> >>
> >>          if (alert_level == SSL3_AL_WARNING) {
> >>              s->s3->warn_alert = alert_descr;
> >> +
> >> +            s->s3->alert_count++;
> >> +            if (s->s3->alert_count == MAX_WARN_ALERT_COUNT) {
> >> +                al = SSL_AD_UNEXPECTED_MESSAGE;
> >> +                SSLerr(SSL_F_DTLS1_READ_BYTES,
> >> SSL_R_TOO_MANY_WARN_ALERTS);
> >> +                goto f_err;
> >> +            }
> >> +
> >>              if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
> >>  #ifndef OPENSSL_NO_SCTP
> >>                  /*
> >>
> >> Modified: stable/10/crypto/openssl/ssl/s3_pkt.c
> >> ==========================================================================
> ===
> >> =
> >> --- stable/10/crypto/openssl/ssl/s3_pkt.c	Wed Nov  2 06:58:47 2016
> >> 	(r308199)
> >> +++ stable/10/crypto/openssl/ssl/s3_pkt.c	Wed Nov  2 07:09:31 2016
> >> 	(r308200)
> >> @@ -1057,6 +1057,13 @@ int ssl3_read_bytes(SSL *s, int type, un
> >>              return (ret);
> >>      }
> >>
> >> +    /*
> >> +     * Reset the count of consecutive warning alerts if we've got a
> >> non-empt
> >> y
> >> +     * record that isn't an alert.
> >> +     */
> >> +    if (rr->type != SSL3_RT_ALERT && rr->length != 0)
> >> +        s->s3->alert_count = 0;
> >> +
> >>      /* we now have a packet which can be read and processed */
> >>
> >>      if (s->s3->change_cipher_spec /* set when we receive
> >> ChangeCipherSpec,
> >> @@ -1271,6 +1278,14 @@ int ssl3_read_bytes(SSL *s, int type, un
> >>
> >>          if (alert_level == SSL3_AL_WARNING) {
> >>              s->s3->warn_alert = alert_descr;
> >> +
> >> +            s->s3->alert_count++;
> >> +            if (s->s3->alert_count == MAX_WARN_ALERT_COUNT) {
> >> +                al = SSL_AD_UNEXPECTED_MESSAGE;
> >> +                SSLerr(SSL_F_SSL3_READ_BYTES,
> >> SSL_R_TOO_MANY_WARN_ALERTS);
> >> +                goto f_err;
> >> +            }
> >> +
> >>              if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
> >>                  s->shutdown |= SSL_RECEIVED_SHUTDOWN;
> >>                  return (0);
> >>
> >> Modified: stable/10/crypto/openssl/ssl/ssl.h
> >> ==========================================================================
> ===
> >> =
> >> --- stable/10/crypto/openssl/ssl/ssl.h	Wed Nov  2 06:58:47 2016
> >> 	(r308199)
> >> +++ stable/10/crypto/openssl/ssl/ssl.h	Wed Nov  2 07:09:31 2016
> >> 	(r308200)
> >> @@ -2717,6 +2717,7 @@ void ERR_load_SSL_strings(void);
> >>  # define SSL_R_TLS_HEARTBEAT_PENDING                      366
> >>  # define SSL_R_TLS_ILLEGAL_EXPORTER_LABEL                 367
> >>  # define SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST             157
> >> +# define SSL_R_TOO_MANY_WARN_ALERTS                       409
> >>  # define SSL_R_TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST 233
> >>  # define SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG    234
> >>  # define SSL_R_TRIED_TO_USE_UNSUPPORTED_CIPHER            235
> >>
> >> Modified: stable/10/crypto/openssl/ssl/ssl3.h
> >> ==========================================================================
> ===
> >> =
> >> --- stable/10/crypto/openssl/ssl/ssl3.h	Wed Nov  2 06:58:47 2016
> >> 	(r308199)
> >> +++ stable/10/crypto/openssl/ssl/ssl3.h	Wed Nov  2 07:09:31 2016
> >> 	(r308200)
> >> @@ -587,6 +587,8 @@ typedef struct ssl3_state_st {
> >>      char is_probably_safari;
> >>  #   endif                       /* !OPENSSL_NO_EC */
> >>  #  endif                        /* !OPENSSL_NO_TLSEXT */
> >> +    /* Count of the number of consecutive warning alerts received */
> >> +    unsigned int alert_count;
> >>  } SSL3_STATE;
> >>
> >>  # endif
> >>
> >> Modified: stable/10/crypto/openssl/ssl/ssl_locl.h
> >> ==========================================================================
> ===
> >> =
> >> --- stable/10/crypto/openssl/ssl/ssl_locl.h	Wed Nov  2 06:58:47 201
> 6
> >> 	(r308199)
> >> +++ stable/10/crypto/openssl/ssl/ssl_locl.h	Wed Nov  2 07:09:31 201
> 6
> >> 	(r308200)
> >> @@ -389,6 +389,8 @@
> >>   */
> >>  # define SSL_MAX_DIGEST 6
> >>
> >> +# define MAX_WARN_ALERT_COUNT    5
> >> +
> >>  # define TLS1_PRF_DGST_MASK      (0xff << TLS1_PRF_DGST_SHIFT)
> >>
> >>  # define TLS1_PRF_DGST_SHIFT 10
> >>
> >>
> >
> > Hi delphij@,
> >
> > This broke stable10 builds.
> >
> > --- d1_pkt.So ---
> > /opt/src/svn-stable10/secure/lib/libssl/../../../crypto/openssl/ssl/d1_pkt.
> c
> > :932:16: error: no member named 'alert_count' in 'struct ssl3_state_st'
> >         s->s3->alert_count = 0;
> >         ~~~~~  ^
> > /opt/src/svn-stable10/secure/lib/libssl/../../../crypto/openssl/ssl/d1_pkt.
> c
> > :1201:20: error: no member named 'alert_count' in 'struct ssl3_state_st'
> >             s->s3->alert_count++;
> >             ~~~~~  ^
> > /opt/src/svn-stable10/secure/lib/libssl/../../../crypto/openssl/ssl/d1_pkt.
> c
> > :1202:24: error: no member named 'alert_count' in 'struct ssl3_state_st'
> >             if (s->s3->alert_count == MAX_WARN_ALERT_COUNT) {
> >                 ~~~~~  ^
> > /opt/src/svn-stable10/secure/lib/libssl/../../../crypto/openssl/ssl/d1_pkt.
> c
> > :1204:48: error: use of undeclared identifier 'SSL_R_TOO_MANY_WARN_ALERTS'
> >                 SSLerr(SSL_F_DTLS1_READ_BYTES, SSL_R_TOO_MANY_WARN_ALERTS);
> >                                                ^
> > /usr/obj/opt/src/svn-stable10/tmp/usr/include/openssl/err.h:217:54: note:
> > expanded from macro 'SSLerr'
> > # define SSLerr(f,r)  ERR_PUT_error(ERR_LIB_SSL,(f),(r),__FILE__,__LINE__)
> >                                                      ^
> > /usr/obj/opt/src/svn-stable10/tmp/usr/include/openssl/err.h:135:61: note:
> > expanded from macro 'ERR_PUT_error'
> > #  define ERR_PUT_error(a,b,c,d,e)        ERR_put_error(a,b,c,d,e)
> >                                                             ^
> > 4 errors generated.
> > *** [d1_pkt.So] Error code 1
> >
> > make[4]: stopped in /opt/src/svn-stable10/secure/lib/libssl
> > 1 error
> >
> > make[4]: stopped in /opt/src/svn-stable10/secure/lib/libssl
> > *** [secure/lib/libssl__L] Error code 2
> >
> > make[3]: stopped in /opt/src/svn-stable10
> 
> Are you sure about this? Our build tests finished properly on
> stable/10: http://jenkins.hardenedbsd.org:8180/jenkins/job/HardenedBSD-stable
> -10-STABLE-master-amd64/74/
> and the release build too.

Rebuilt this time without -DNO_CLEAN allowed it to pass. Sorry for the 
noise.


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org

	The need of the many outweighs the greed of the few.





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201611030017.uA30H4UJ078504>