Create loopback test framework for libssl

RESOLVED WONTFIX

Status

P1
normal
RESOLVED WONTFIX
7 years ago
5 years ago

People

(Reporter: briansmith, Assigned: briansmith)

Tracking

trunk
3.13.2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

We need a test framework that allows a single test to control in fine detail both sides of the connection, including the precise timing of messages sent/received.
I did some work on this that allows for writing tests like the one below. This bug is about the framework for the tests: DEFINE_TEST, setupClientServer, CHECK_*, the hooks in libssl that allow the test framework to define SSL_SetImmediatelyRenegotiate, etc.

The test below tests that asynchronous certificate validation (bug 542832) still allows us to see a hello_request that appears in the same packet as the Finished message. 

DEFINE_TEST(async_cert_restart_server_sends_hello_request_first_in_same_record)
{
    PRFileDesc * client;
    PRFileDesc * server;
    PRBool clientWaitingForServerAuth = PR_FALSE; // XXX: use
    PRBool clientDone = PR_FALSE;
    PRBool serverDone = PR_FALSE;
    SSL3HandshakeState *client_hs;
    SSL3HandshakeState *server_hs;

    setupClientServer(&client, clientHooks, 
                      &server, serverHooks,
                      ssl_kea_rsa);
    CHECK_NOT_NULL(client_hs = &ssl_FindSocket(client)->ssl3.hs);
    CHECK_NOT_NULL(server_hs = &ssl_FindSocket(server)->ssl3.hs);

    CHECK_SECSuccess(SSL_AuthCertificateHook(client, return_SECWouldBlock,
                                             &clientWaitingForServerAuth));
    CHECK_SECSuccess(SSL_HandshakeCallback(client, setHandshakeDone,
                                           &clientDone));
    CHECK_SECSuccess(SSL_HandshakeCallback(server, setHandshakeDone,
                                           &serverDone));
    /*************************************************************************/
    SSL_SetImmediatelyRenegotiate(serverHooks);
    /*************************************************************************/

    CHECK_TRUE(!client_hs->authCertificatePending);
    CHECK_NULL(client_hs->authCertificateRestartTarget);

    CHECK_SECFailure(SSL_ForceHandshake(client)); /* client hello */
    CHECK_TRUE(PORT_GetError() == PR_WOULD_BLOCK_ERROR);
    CHECK_WS(client, wait_server_hello);
    CHECK_TRUE(!client_hs->authCertificatePending);
    CHECK_NULL(client_hs->authCertificateRestartTarget);

    CHECK_WS(server, wait_client_hello);
    CHECK_TRUE(!server_hs->authCertificatePending);
    CHECK_NULL(server_hs->authCertificateRestartTarget);

    CHECK_SECFailure(SSL_ForceHandshake(server)); /* server hello...done */
    CHECK_TRUE(PORT_GetError() == PR_WOULD_BLOCK_ERROR);
    CHECK_WS(server, wait_client_key);
    CHECK_TRUE(!server_hs->authCertificatePending);
    CHECK_NULL(server_hs->authCertificateRestartTarget);

    CHECK_SECFailure(SSL_ForceHandshake(client)); /* client 2nd round */
    CHECK_TRUE(PORT_GetError() == PR_WOULD_BLOCK_ERROR);
    CHECK_WS(client, wait_change_cipher);
    CHECK_TRUE(client_hs->authCertificatePending);
    CHECK_NULL(client_hs->authCertificateRestartTarget);

    CHECK_SECSuccess(SSL_ForceHandshake(server)); /* server finished */
    CHECK_TRUE(serverDone);
    /*************************************************************************/
    CHECK_WS(server, wait_client_hello);
    /*************************************************************************/
    CHECK_TRUE(!server_hs->authCertificatePending);
    CHECK_NULL(server_hs->authCertificateRestartTarget);

    CHECK_SECSuccess(SSL_ForceHandshake(client)); /* consume server finished */
    CHECK_TRUE(!clientDone); /* client handshake callback not called yet */
    CHECK_WS(client, wait_finished);
    CHECK_TRUE(client_hs->authCertificatePending);
    CHECK_NOT_NULL(client_hs->authCertificateRestartTarget);

    CHECK_SECSuccess(SSL_RestartHandshakeAfterAuthCertificate(client));
    CHECK_TRUE(clientDone); /* client handshake callback not called yet */
    CHECK_WS(client, idle_handshake);
    CHECK_TRUE(!client_hs->authCertificatePending);
    CHECK_NULL(client_hs->authCertificateRestartTarget);

    CHECK_SECFailure(SSL_ForceHandshake(client));
    CHECK_TRUE(PORT_GetError() == PR_WOULD_BLOCK_ERROR);
    CHECK_WS(client, wait_server_hello);
}
Created attachment 574835 [details] [diff] [review]
Improve SECU_Print* to include error message name and fix constness

Adding the error name to error messages make it *much* easier to investigate errors. The constness changes were done so that the loopback framework could use const in the correct places.

Note that I am posting Mercurial/git-format patches to this bug, mostly because they are much easier to manage when there are patches on top of other patches.
Attachment #574835 - Flags: superreview?(rrelyea)
Attachment #574835 - Flags: review?(emaldona)
Created attachment 574849 [details] [diff] [review]
The beginnings of a loopback testing framework for libssl

This simple framework makes it possible to test interactions between the client and the server during the handshake in a way that is under the test's control. For example, it allows us to test things like "what happens if the server sends application data before the client has validated the server's cert" in a way that isn't prone to any race conditions.

It creates a socket pair in the process, where one side gets configured as an SSL client and the other side gets configured as an SSL server.

It also includes the beginnings of a very simple hook mechanism. The idea is that, wherever we need testing code injected into libssl, we can create a new hook point. Then, libssl will call that hook at that point, if a hook is provided. For example, one of the tests that I will post uses such a hook to insert a hello_request record into the same record as the server's finished message. I am planning to expand this hooking mechanism so that it can let us to quickly simulate server-side features (e.g. the server side of OCSP stapling, server-side NPN support), without investing the time in production-quality server side code, and without littering libssl itself with non-production-quality code. I am also hoping that people will be able to use it to write self-contained test cases for bug reports.

See bug 542832 for examples of tests written using this.
Attachment #574849 - Flags: feedback?(wtc)
Attachment #574849 - Flags: feedback?(rrelyea)
Attachment #574849 - Flags: feedback?(ekr)
Attachment #574849 - Flags: feedback?(agl)

Updated

7 years ago
Attachment #574835 - Flags: review?(emaldona) → review+

Comment 4

7 years ago
Comment on attachment 574835 [details] [diff] [review]
Improve SECU_Print* to include error message name and fix constness

r+
Attachment #574835 - Flags: superreview?(rrelyea) → superreview+
Created attachment 584631 [details] [diff] [review]
Improve SECU_Print* to include error message name and fix constness [v2]
Attachment #574835 - Attachment is obsolete: true
Comment on attachment 584631 [details] [diff] [review]
Improve SECU_Print* to include error message name and fix constness [v2]

Carrying forward r+ and sr+. The only changes were to whitespace.
Attachment #584631 - Flags: superreview+
Attachment #584631 - Flags: review+
Created attachment 584633 [details] [diff] [review]
The beginnings of a loopback testing framework for libssl [v2]

Work in progress, needed to run the tests for bug 542832.

I am going to update this based on feedback from rrelyea and add some documentation to it.
Attachment #574849 - Attachment is obsolete: true
Attachment #574849 - Flags: feedback?(wtc)
Attachment #574849 - Flags: feedback?(rrelyea)
Attachment #574849 - Flags: feedback?(ekr)
Attachment #574849 - Flags: feedback?(agl)
Created attachment 588356 [details] [diff] [review]
The beginnings of a loopback testing framework for libssl [v3]
Attachment #584633 - Attachment is obsolete: true

Comment 9

7 years ago
Comment on attachment 588356 [details] [diff] [review]
The beginnings of a loopback testing framework for libssl [v3]

r-, but only for some simple issues.

First, the private exports for testing should have _ symbols in their name (_ssl_SendHelloRequest()). Adding a comment in the def file that they are strictly for testing would also be a good idea.

Second. If you add the _ symbols, you can have both the _ssl_SendHelloRequest() and an ssl_SendHelloRequest(), the latter can include the flush and setting of ws state. That would simplify your patch to ssl3con.c

Third. You have this mechanism for creating tests with .h files, but I'm not exactly clear how it's supposed to work. In sslloopbacktests.h You have a #define X(x) in the header file, which is set based on some declaration... then you just #undefine it without using it. Is the goal here that you edit sslloopbacktests.h add add each test by hand? It might be more clear if it includes a 'definetests.h' which just has a bunch of entries:
   X(mytest1)
   X(mytest2)
   X(mytest3)
etc.

Forth, the frame work looks like it reaches right into the private ssl data structure. This should be avoided if at all possible (without introducing too much pain). We try pretty hard to keep a separation between the commands in cmd and the private NSS data. That way our old test commands will run on newer versions of NSS (something we use to actually test once in a while to make sure we didn't break binary compatibility). The SSL data structure is private, and we can and do change it between releases.

bob
Attachment #588356 - Flags: review-
Depends on: 863888
Comment on attachment 584631 [details] [diff] [review]
Improve SECU_Print* to include error message name and fix constness [v2]

Part 1 was moved to bug 863888 and checked in.
Attachment #584631 - Attachment is obsolete: true
I realized after I started this work that 75% of what I'd done was poorly re-implement GTest. It would be better to create a GTest-based framework for testing libssl, which should happen separately from this bug.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.