Skip to content

Commit

Permalink
Always throw error when OpenSSL returns SSL_ERROR_SYSCALL.
Browse files Browse the repository at this point in the history
Previously an error was only thrown when errno was set but in practice this is usually not the case. This may have something to do with getting errno late but attempts to get it earlier have not been successful. It appears that errno usually gets cleared and spot research seems to indicate that other users have similar issues.

An error at this point indicates unexpected EOF so it seems better to just throw an error all the time and be consistent.

To test this properly our test server needs to call SSL_shutdown() except when the client expects this error.
  • Loading branch information
dwsteele committed Apr 14, 2020
1 parent 9f2d647 commit 71fb28b
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 5 deletions.
8 changes: 8 additions & 0 deletions doc/xml/release.xml
Expand Up @@ -50,6 +50,14 @@
<p>Split session functionality of <code>TlsClient</code> out into <code>TlsSession</code>.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>

<p>Always throw error when <proper>OpenSSL</proper> returns <code>SSL_ERROR_SYSCALL</code>.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
Expand Down
8 changes: 3 additions & 5 deletions src/common/io/tls/session.c
Expand Up @@ -101,17 +101,15 @@ tlsSessionError(TlsSession *this, int code)
break;
}

// A syscall failed (this usually indicates eof)
// A syscall failed (usually indicates unexpected eof)
case SSL_ERROR_SYSCALL:
{
// Get the error before closing so it is not cleared
int errNo = errno;
tlsSessionClose(this);

// Throw the sys error if there is one
THROW_ON_SYS_ERROR(errNo, KernelError, "tls failed syscall");

break;
// Throw the sys error
THROW_SYS_ERROR_CODE(errNo, KernelError, "tls failed syscall");
}

// Some other tls error that cannot be handled
Expand Down
9 changes: 9 additions & 0 deletions test/src/common/harnessTls.c
Expand Up @@ -170,6 +170,15 @@ Close the connection
***********************************************************************************************************************************/
void
harnessTlsServerClose(void)
{
SSL_shutdown(testClientSSL);
SSL_free(testClientSSL);
close(testClientSocket);
}

/**********************************************************************************************************************************/
void
harnessTlsServerAbort(void)
{
SSL_free(testClientSSL);
close(testClientSocket);
Expand Down
3 changes: 3 additions & 0 deletions test/src/common/harnessTls.h
Expand Up @@ -31,6 +31,9 @@ void harnessTlsServerExpect(const char *expected);
void harnessTlsServerReply(const char *reply);
void harnessTlsServerClose(void);

// Abort the server session (i.e. don't perform proper TLS shutdown)
void harnessTlsServerAbort(void);

/***********************************************************************************************************************************
Getters/Setters
***********************************************************************************************************************************/
Expand Down
14 changes: 14 additions & 0 deletions test/src/module/common/ioTlsTest.c
Expand Up @@ -82,6 +82,11 @@ testTlsServer(void)
harnessTlsServerReply("0123456789AB");
harnessTlsServerClose();

// Test aborted connection before read complete
harnessTlsServerAccept();
harnessTlsServerReply("0123456789AB");
harnessTlsServerAbort();

// Need data in read buffer to test tlsWriteContinue()
harnessTlsServerAccept();
harnessTlsServerReply("0123456789AB");
Expand Down Expand Up @@ -397,7 +402,16 @@ testRun(void)
TEST_ERROR(tlsSessionError(session, SSL_ERROR_WANT_X509_LOOKUP), ServiceError, "tls error [4]");

// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("aborted connection before read complete");

TEST_ASSIGN(session, tlsClientOpen(client), "open client again (was closed by server)");

output = bufNew(13);
TEST_ERROR(ioRead(tlsSessionIoRead(session), output), KernelError, "tls failed syscall");

// -----------------------------------------------------------------------------------------------------------------
TEST_ASSIGN(session, tlsClientOpen(client), "open client again (was closed by server)");

TEST_RESULT_BOOL(tlsSessionWriteContinue(session, -1, SSL_ERROR_WANT_READ, 1), true, "continue on WANT_READ");
TEST_RESULT_BOOL(tlsSessionWriteContinue(session, 0, SSL_ERROR_NONE, 1), true, "continue on WANT_READ");
TEST_ERROR(
Expand Down

0 comments on commit 71fb28b

Please sign in to comment.