Browse Source

crypto/tls: stop ConfirmHandshake from locking on any Read

ConfirmHandshake should block on a Read until the handshakeConfirmed
state is reached, but past that it shouldn't.
v1.2.3
Filippo Valsorda 8 years ago
committed by Peter Wu
parent
commit
faefac5f1a
4 changed files with 59 additions and 11 deletions
  1. +7
    -2
      13.go
  2. +1
    -1
      _dev/interop.sh
  3. +19
    -5
      _dev/tris-localserver/server.go
  4. +32
    -3
      conn.go

+ 7
- 2
13.go View File

@@ -157,10 +157,15 @@ func (hs *serverHandshakeState) readClientFinished13() error {
hs.finishedHash13.Write(clientFinished.marshal())

c.hs = nil // Discard the server handshake state
c.phase = handshakeConfirmed
atomic.StoreInt32(&c.handshakeConfirmed, 1)
c.in.setCipher(c.vers, hs.appClientCipher)
c.in.traceErr, c.out.traceErr = nil, nil
c.phase = handshakeConfirmed
atomic.StoreInt32(&c.handshakeConfirmed, 1)

// Any read operation after handshakeRunning and before handshakeConfirmed
// will be holding this lock, which we release as soon as the confirmation
// happens, even if the Read call might do more work.
c.confirmMutex.Unlock()

return hs.sendSessionTicket13()
}


+ 1
- 1
_dev/interop.sh View File

@@ -30,6 +30,6 @@ elif [ "$1" = "0-RTT" ]; then
grep "Hello TLS 1.3" output.txt | grep "resumed" | grep "0-RTT"

docker run --rm tls-tris:$2 $IP:5443 | tee output.txt # confirming 0-RTT
grep "Hello TLS 1.3" output.txt | grep "resumed" | grep -v "0-RTT"
grep "Hello TLS 1.3" output.txt | grep "resumed" | grep "0-RTT confirmed"

fi

+ 19
- 5
_dev/tris-localserver/server.go View File

@@ -52,20 +52,34 @@ func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
tlsConn := r.Context().Value(http.TLSConnContextKey).(*tls.Conn)
server := r.Context().Value(http.ServerContextKey).(*http.Server)
if server.Addr == confirmingAddr {

with0RTT := ""
if !tlsConn.ConnectionState().HandshakeConfirmed {
with0RTT = " [0-RTT]"
}
if server.Addr == confirmingAddr || r.URL.Path == "/confirm" {
if err := tlsConn.ConfirmHandshake(); err != nil {
log.Fatal(err)
}
if with0RTT != "" {
with0RTT = " [0-RTT confirmed]"
}
if !tlsConn.ConnectionState().HandshakeConfirmed {
panic("HandshakeConfirmed false after ConfirmHandshake")
}
}

resumed := ""
if r.TLS.DidResume {
resumed = " [resumed]"
}
with0RTT := ""
if !tlsConn.ConnectionState().HandshakeConfirmed {
with0RTT = " [0-RTT]"

http2 := ""
if r.ProtoMajor == 2 {
http2 = " [HTTP/2]"
}
fmt.Fprintf(w, "<!DOCTYPE html><p>Hello TLS %s%s%s _o/\n", tlsVersionToName[r.TLS.Version], resumed, with0RTT)

fmt.Fprintf(w, "<!DOCTYPE html><p>Hello TLS %s%s%s%s _o/\n", tlsVersionToName[r.TLS.Version], resumed, with0RTT, http2)
})

http.HandleFunc("/ch", func(w http.ResponseWriter, r *http.Request) {


+ 32
- 3
conn.go View File

@@ -30,6 +30,8 @@ type Conn struct {
phase handshakeStatus // protected by in.Mutex
// handshakeConfirmed is an atomic bool for phase == handshakeConfirmed
handshakeConfirmed int32
// confirmMutex is held by any read operation before handshakeConfirmed
confirmMutex sync.Mutex

// constant after handshake; protected by handshakeMutex
handshakeMutex sync.Mutex // handshakeMutex < in.Mutex, out.Mutex, errMutex
@@ -1254,13 +1256,27 @@ func (c *Conn) ConfirmHandshake() error {
return err
}

c.in.Lock()
defer c.in.Unlock()
if c.vers < VersionTLS13 {
return nil
}

if c.phase == handshakeConfirmed {
c.confirmMutex.Lock()
if atomic.LoadInt32(&c.handshakeConfirmed) == 1 { // c.phase == handshakeConfirmed
c.confirmMutex.Unlock()
return nil
} else {
defer func() {
// If we transitioned to handshakeConfirmed we already released the lock,
// otherwise do it here.
if c.phase != handshakeConfirmed {
c.confirmMutex.Unlock()
}
}()
}

c.in.Lock()
defer c.in.Unlock()

var input *block
if c.phase == readingEarlyData || c.input != nil {
buf := &bytes.Buffer{}
@@ -1341,6 +1357,19 @@ func (c *Conn) Read(b []byte) (n int, err error) {
return
}

c.confirmMutex.Lock()
if atomic.LoadInt32(&c.handshakeConfirmed) == 1 { // c.phase == handshakeConfirmed
c.confirmMutex.Unlock()
} else {
defer func() {
// If we transitioned to handshakeConfirmed we already released the lock,
// otherwise do it here.
if c.phase != handshakeConfirmed {
c.confirmMutex.Unlock()
}
}()
}

c.in.Lock()
defer c.in.Unlock()



Loading…
Cancel
Save