[PATCH] Post-b34 Fixes

[Andrew Hughes]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > * Andrew Hughes <gnu.andrew at redhat.com> [2015-01-29 16:12]:
> > > * OPENJDK6-51: Sync patch for 4873188 with 7 version
> > > 
> > > The backport of 4873188 (TLS 1.1 support) accidentally incorporated part
> > > of 7031830 (see below) and placed a chunk of code in a slightly different
> > > position to where it is in 7. Neither issue results in a problem, but
> > > this changeset just makes the diff between 6 & 7 smaller.
> > 
> > I looked at the current code in jdk7 and it looks like this:
> > 
> >         // eventually compress as well.
> >         HandshakeStatus hsStatus =
> >                 writer.writeRecord(eor, ea, writeMAC, writeCipher);
> > 
> >         /*
> >          * We only need to check the sequence number state for
> >          * non-handshaking record.
> >          *
> >          * Note that in order to maintain the handshake status
> >          * properly, we check the sequence number after the last
> >          * record writing process. As we request renegotiation
> >          * or close the connection for wrapped sequence number
> >          * when there is enough sequence number space left to
> >          * handle a few more records, so the sequence number
> >          * of the last record cannot be wrapped.
> >          */
> >         if (connectionState < cs_ERROR && !isOutboundDone() &&
> >                 (hsStatus == HandshakeStatus.NOT_HANDSHAKING)) {
> >             if (checkSequenceNumber(writeMAC, eor.contentType())) {
> >                 hsStatus = getHSStatus(null);
> >             }
> >         }
> > 
> >         /*
> >          * turn off the flag of the first application record if we really
> >          * consumed at least byte.
> >          */
> >         if (isFirstAppOutputRecord && ea.deltaApp() > 0) {
> >             isFirstAppOutputRecord = false;
> >         }
> > 
> >         return hsStatus;
> > 
> > The patch, however, changes the current placement in jdk6:
> > 
> >                  writer.writeRecord(eor, ea, writeMAC, writeCipher);
> >  
> >          /*
> > +         * turn off the flag of the first application record if we really
> > +         * consumed at least byte.
> > +         */
> > +        if (isFirstAppOutputRecord && ea.deltaApp() > 0) {
> > +            isFirstAppOutputRecord = false;
> > +        }
> > +
> > +        /*
> >           * We only need to check the sequence number state for
> >           * non-handshaking record.
> >           *
> > @@ -1279,14 +1287,6 @@
> >              }
> >          }
> >  
> > -        /*
> > -         * turn off the flag of the first application record if we really
> > -         * consumed at least byte.
> > -         */
> > -        if (isFirstAppOutputRecord && ea.deltaApp() > 0) {
> > -            isFirstAppOutputRecord = false;
> > -        }
> > -
> >          return hsStatus;
> > 
> > That looks like the current code in jdk6 matches what's in jdk7u and the
> > patch
> > makes it diverge. Is that right?
> > 
> > Everything else looks fine.
> > 
> > Thanks,
> > Omair
> > 
> > --
> > PGP Key: 66484681 (http://pgp.mit.edu/)
> > Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681
> > 
> 
> No. Good catch.
> 
> It seems IcedTea has both, thanks to the original security patch putting
> it in one place, then the upstream changeset placing it in the other.
> 
> At least our original was right :) I'll post another webrev later.
> --
> Andrew :)
> 

Revised version:

http://cr.openjdk.java.net/~andrew/openjdk6/secfix/webrev.02/
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07