kris/th5
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

crypto/tls: do not drain 0-RTT data on Close

Browse Source 
There is no reason a server can't just send a CloseNotify in its first
flight, and then close the connection without reading the 0-RTT data.

Also, it's not expected of Close to block on reading, and interlocking
with a Read can cause a deadlock.

Fixes NCC-2016-001
This commit is contained in:
Filippo Valsorda 2016-12-05 13:00:53 -05:00 committed by Peter Wu
parent 3e31621f57
commit f3fe024dc7
4 changed files with 5 additions and 149 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
.travis.yml
View File

@ -10,10 +10,11 @@ go:
env:
- MODE=interop CLIENT=boring
- MODE=interop CLIENT=bogo
- MODE=interop CLIENT=tstclnt ZRTT=1
- MODE=interop CLIENT=tstclnt
- MODE=interop CLIENT=picotls ZRTT=1
- MODE=interop CLIENT=mint
- MODE=gotest
- MODE=interop CLIENT=tstclnt ZRTT=1
- MODE=interop CLIENT=boring REVISION=origin/master
- MODE=interop CLIENT=tstclnt REVISION=default ZRTT=1
@ -22,6 +23,7 @@ matrix:
allow_failures:
- env: MODE=interop CLIENT=boring REVISION=origin/master
- env: MODE=interop CLIENT=tstclnt REVISION=default ZRTT=1
- env: MODE=interop CLIENT=tstclnt ZRTT=1 # crashes on close_notify in 0.5RTT
install:
- if [ "$MODE" = "interop" ]; then ./_dev/tris-localserver/start.sh -d && docker ps -a; fi

2
_dev/tstclnt/Dockerfile
View File

@ -18,7 +18,7 @@ ENV USE_64=1 NSS_ENABLE_TLS_1_3=1
# ARG REVISION=b6dfef6d0ff0
# tstclnt resumption
ARG REVISION=de6e7c3a39d5
ARG REVISION=2ed8aef0b360
RUN cd nss && hg pull
RUN cd nss && hg checkout -C $REVISION

136
_dev/tstclnt/retransmit.patch
View File

@ -1,136 +0,0 @@
diff --git a/cmd/tstclnt/tstclnt.c b/cmd/tstclnt/tstclnt.c
index eb114e9..419e96f 100644
--- a/cmd/tstclnt/tstclnt.c
+++ b/cmd/tstclnt/tstclnt.c
@@ -169,19 +169,6 @@ printSecurityInfo(PRFileDesc *fd)
}
}
-void
-handshakeCallback(PRFileDesc *fd, void *client_data)
-{
- const char *secondHandshakeName = (char *)client_data;
- if (secondHandshakeName) {
- SSL_SetURL(fd, secondHandshakeName);
- }
- printSecurityInfo(fd);
- if (renegotiationsDone < renegotiationsToDo) {
- SSL_ReHandshake(fd, (renegotiationsToDo < 2));
- ++renegotiationsDone;
- }
-}
static void
PrintUsageHeader(const char *progName)
@@ -923,13 +910,19 @@ PRUint16 portno = 443;
int override = 0;
char *requestString = NULL;
PRInt32 requestStringLen = 0;
+PRBool requestSent = PR_FALSE;
PRBool enableZeroRtt = PR_FALSE;
static int
-writeBytesToServer(PRFileDesc *s, PRPollDesc *pollset, const char *buf, int nb)
+writeBytesToServer(PRFileDesc *s, const char *buf, int nb)
{
SECStatus rv;
const char *bufp = buf;
+ PRPollDesc pollset[1];
+
+ pollset[SSOCK_FD].in_flags = PR_POLL_WRITE | PR_POLL_EXCEPT;
+ pollset[SSOCK_FD].out_flags = 0;
+ pollset[SSOCK_FD].fd = s;
FPRINTF(stderr, "%s: Writing %d bytes to server\n",
progName, nb);
@@ -975,6 +968,36 @@ writeBytesToServer(PRFileDesc *s, PRPollDesc *pollset, const char *buf, int nb)
return 0;
}
+void
+handshakeCallback(PRFileDesc *fd, void *client_data)
+{
+ const char *secondHandshakeName = (char *)client_data;
+ if (secondHandshakeName) {
+ SSL_SetURL(fd, secondHandshakeName);
+ }
+ printSecurityInfo(fd);
+ if (renegotiationsDone < renegotiationsToDo) {
+ SSL_ReHandshake(fd, (renegotiationsToDo < 2));
+ ++renegotiationsDone;
+ }
+ if (requestString && requestSent) {
+ /* This data was sent in 0-RTT. */
+ SSLChannelInfo info;
+ SECStatus rv;
+
+ rv = SSL_GetChannelInfo(fd, &info, sizeof(info));
+ if (rv != SECSuccess)
+ return;
+
+ if (!info.earlyDataAccepted) {
+ FPRINTF(stderr, "Early data rejected. Re-sending\n");
+ writeBytesToServer(fd, requestString, requestStringLen);
+ }
+ }
+}
+
+#define REQUEST_WAITING (requestString && !requestSent)
+
static int
run_client(void)
{
@@ -988,7 +1011,8 @@ run_client(void)
PRFileDesc *std_out;
PRPollDesc pollset[2];
PRBool wrStarted = PR_FALSE;
- char *requestStringInt = requestString;
+
+ requestSent = PR_FALSE;
/* Create socket */
s = PR_OpenTCPSocket(addr.raw.family);
@@ -1245,7 +1269,7 @@ run_client(void)
pollset[SSOCK_FD].in_flags = PR_POLL_EXCEPT |
(clientSpeaksFirst ? 0 : PR_POLL_READ);
pollset[STDIN_FD].fd = PR_GetSpecialFD(PR_StandardInput);
- if (!requestStringInt) {
+ if (!REQUEST_WAITING) {
pollset[STDIN_FD].in_flags = PR_POLL_READ;
npds = 2;
} else {
@@ -1295,7 +1319,7 @@ run_client(void)
*/
FPRINTF(stderr, "%s: ready...\n", progName);
while ((pollset[SSOCK_FD].in_flags | pollset[STDIN_FD].in_flags) ||
- requestStringInt) {
+ REQUEST_WAITING) {
char buf[4000]; /* buffer for stdin */
int nb; /* num bytes read from stdin. */
@@ -1333,13 +1357,12 @@ run_client(void)
"%s: PR_Poll returned 0x%02x for socket out_flags.\n",
progName, pollset[SSOCK_FD].out_flags);
}
- if (requestStringInt) {
- error = writeBytesToServer(s, pollset,
- requestStringInt, requestStringLen);
+ if (REQUEST_WAITING) {
+ error = writeBytesToServer(s, requestString, requestStringLen);
if (error) {
goto done;
}
- requestStringInt = NULL;
+ requestSent = PR_TRUE;
pollset[SSOCK_FD].in_flags = PR_POLL_READ;
}
if (pollset[STDIN_FD].out_flags & PR_POLL_READ) {
@@ -1356,7 +1379,7 @@ run_client(void)
/* EOF on stdin, stop polling stdin for read. */
pollset[STDIN_FD].in_flags = 0;
} else {
- error = writeBytesToServer(s, pollset, buf, nb);
+ error = writeBytesToServer(s, buf, nb);
if (error) {
goto done;
}

12
conn.go
View File

@ -1423,17 +1423,7 @@ func (c *Conn) Close() error {
var alertErr error
c.handshakeMutex.Lock()
for c.phase == readingEarlyData {
if err := c.readRecord(recordTypeApplicationData); err != nil {
alertErr = err
}
}
if alertErr == nil && c.phase == waitingClientFinished {
if err := c.hs.readClientFinished13(); err != nil {
alertErr = err
}
}
if alertErr == nil && c.phase != handshakeRunning {
if c.phase != handshakeRunning {
alertErr = c.closeNotify()
}
c.handshakeMutex.Unlock()