Return SSL_ERROR_SYSCALL on unclean EOF.

This regressed in fcf25833bc. 0 return code on
unclean shutdown means the underlying BIO returned EOF, didn't push any error
code, but we haven't seen close_notify yet. The intent seems to be that you go
check errno or some BIO-specific equivalent if you care about close_notify.

Make sure test code routes all SSL_read return codes through SSL_get_error
since that's supposed to work in all cases.

(Note that rv == 0 can still give SSL_ERROR_SSL if the error queue is not
empty.)

Change-Id: I45bf9614573f876d93419ce169a4e0d9ceea9052
Reviewed-on: https://boringssl-review.googlesource.com/2981
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-01-22 16:06:11 -05:00 committed by Adam Langley
parent 1e52ecac4d
commit 9a38e924aa
2 changed files with 42 additions and 21 deletions

View File

@ -2251,13 +2251,18 @@ int SSL_get_error(const SSL *s, int i) {
return SSL_ERROR_SSL; return SSL_ERROR_SSL;
} }
if (i == 0 && (s->shutdown & SSL_RECEIVED_SHUTDOWN) && if (i == 0) {
(s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY)) { if ((s->shutdown & SSL_RECEIVED_SHUTDOWN) &&
return SSL_ERROR_ZERO_RETURN; (s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY)) {
/* The socket was cleanly shut down with a close_notify. */
return SSL_ERROR_ZERO_RETURN;
}
/* An EOF was observed which violates the protocol, and the underlying
* transport does not participate in the error queue. Bubble up to the
* caller. */
return SSL_ERROR_SYSCALL;
} }
assert(i < 0);
if (SSL_want_session(s)) { if (SSL_want_session(s)) {
return SSL_ERROR_PENDING_SESSION; return SSL_ERROR_PENDING_SESSION;
} }

View File

@ -656,25 +656,41 @@ static int do_exchange(SSL_SESSION **out_session,
do { do {
n = SSL_read(ssl, buf, sizeof(buf)); n = SSL_read(ssl, buf, sizeof(buf));
} while (config->async && retry_async(ssl, n, bio)); } while (config->async && retry_async(ssl, n, bio));
if (n < 0) { int err = SSL_get_error(ssl, n);
if (err == SSL_ERROR_ZERO_RETURN ||
(n == 0 && err == SSL_ERROR_SYSCALL)) {
if (n != 0) {
fprintf(stderr, "Invalid SSL_get_error output\n");
return 3;
}
/* Accept shutdowns with or without close_notify.
* TODO(davidben): Write tests which distinguish these two cases. */
break;
} else if (err != SSL_ERROR_NONE) {
if (n > 0) {
fprintf(stderr, "Invalid SSL_get_error output\n");
return 3;
}
SSL_free(ssl); SSL_free(ssl);
BIO_print_errors_fp(stdout); BIO_print_errors_fp(stdout);
return 3; return 3;
} else if (n == 0) { }
break; /* Successfully read data. */
} else { if (n <= 0) {
for (int i = 0; i < n; i++) { fprintf(stderr, "Invalid SSL_get_error output\n");
buf[i] ^= 0xff; return 3;
} }
int w; for (int i = 0; i < n; i++) {
do { buf[i] ^= 0xff;
w = SSL_write(ssl, buf, n); }
} while (config->async && retry_async(ssl, w, bio)); int w;
if (w != n) { do {
SSL_free(ssl); w = SSL_write(ssl, buf, n);
BIO_print_errors_fp(stdout); } while (config->async && retry_async(ssl, w, bio));
return 4; if (w != n) {
} SSL_free(ssl);
BIO_print_errors_fp(stdout);
return 4;
} }
} }
} }