Commit graph

507 commits

Author SHA1 Message Date
Andrzej Kurek 755890f529 Remove trailing whitespace 2018-06-27 03:57:15 -04:00
Andrzej Kurek da4029d665 ssl_server2: add buffer overhead for a termination character
Switch to mbedtls style of memory allocation
2018-06-20 07:07:55 -04:00
Andrzej Kurek 30e731decd Added buffer_size and response_size options for ssl-server2.
Added appropriate tests.
2018-06-20 04:22:06 -04:00
Simon Butcher 688a456f58 Merge remote-tracking branch 'public/pr/1734' into development 2018-06-17 17:35:34 +01:00
Simon Butcher 5f57f1e3cc Merge remote-tracking branch 'public/pr/1270' into development 2018-06-15 14:17:31 +01:00
Gilles Peskine d5d983e168 ssl_server2: handle mbedtls_x509_dn_gets failure
If mbedtls_x509_dn_gets fails, the server could end up calling printf
on an uninitialized buffer. Check if the function succeeds. Found by
Coverity.
2018-06-15 14:05:10 +02:00
Gilles Peskine 4481744538 Fix memory leak in ssl_server2 with SNI + async callback
In ssl_server2, the private key objects are normally local variables
of the main function. However this does not hold for private keys in
the SNI configuration. When async callbacks are used, the test code
transfers the ownership of the private keys to the async callbacks.
Therefore the test code must free the SNI private keys through the
async callbacks (but it must not free the straight private keys this
way since they are not even heap-allocated).
2018-06-14 13:26:52 +02:00
Gilles Peskine e247989061 SNI + SSL async callback: make all keys async
When testing async callbacks with SNI, make all the keys async, not
just the first one. Otherwise the test is fragile with respect to
whether a key is used directly or through the async callbacks.
2018-06-14 13:26:52 +02:00
Gilles Peskine 2636fade52 ssl_async_resume: free the operation context on error 2018-06-14 13:26:52 +02:00
Gilles Peskine f5a9996088 ssl_server2: get op_name from context in ssl_async_resume as well 2018-06-14 13:26:52 +02:00
Gilles Peskine 3dae1cfa3a Async callback: use mbedtls_pk_check_pair to compare keys
In the current test code, the object that is used as a public key in
the certificate also contains a private key. However this is because
of the way the stest code is built and does not demonstrate the API in
a useful way. Use mbedtls_pk_check_pair, which is not what real-world
code would do (since the private key would typically be in an external
cryptoprocessor) but is a more representative placeholder.
2018-06-14 13:26:51 +02:00
Gilles Peskine a668c60186 Rename mbedtls_ssl_async_{get,set}_data for clarity
Rename to mbedtls_ssl_get_async_operation_data and
mbedtls_ssl_set_async_operation_data so that they're about
"async operation data" and not about some not-obvious "data".
2018-06-14 13:26:51 +02:00
Gilles Peskine 166ce748cf SSL async callback: cert is not always from mbedtls_ssl_conf_own_cert
The certificate passed to async callbacks may not be the one set by
mbedtls_ssl_conf_own_cert. For example, when using an SNI callback,
it's whatever the callback is using. Document this, and add a test
case (and code sample) with SNI.
2018-06-14 13:26:51 +02:00
Gilles Peskine d6fbfde994 ssl_async_set_key: detect if ctx->slots overflows 2018-06-14 13:26:51 +02:00
Gilles Peskine 4d9ec4dcf7 Fix uninitialized variable in ssl_server2 2018-06-14 13:26:51 +02:00
Gilles Peskine 6331d78675 Don't use the printf format %zd
We target C89 libc, so don't use %zd or %zu. Just use %u, and make
slot numbers `unsigned` for simplicity.
2018-06-14 13:26:51 +02:00
Gilles Peskine 3f3ada8839 Fix memory leak in ssl_server2 with SNI + async callback
In ssl_server2, the private key objects are normally local variables
of the main function. However this does not hold for private keys in
the SNI configuration. When async callbacks are used, the test code
transfers the ownership of the private keys to the async callbacks.
Therefore the test code must free the SNI private keys through the
async callbacks (but it must not free the straight private keys this
way since they are not even heap-allocated).
2018-06-13 18:09:28 +02:00
Gilles Peskine 157f6d8f74 SNI + SSL async callback: make all keys async
When testing async callbacks with SNI, make all the keys async, not
just the first one. Otherwise the test is fragile with respect to
whether a key is used directly or through the async callbacks.
2018-06-13 18:06:51 +02:00
Gilles Peskine 3cde2fca53 ssl_async_resume: free the operation context on error 2018-06-12 14:17:39 +02:00
Darryl Green 11999bb72e Fix minor code style issues 2018-05-15 09:21:57 +01:00
Gilles Peskine 276b9a650c ssl_server2: get op_name from context in ssl_async_resume as well 2018-04-30 16:39:35 +02:00
Gilles Peskine 26d01bcb5c Async callback: use mbedtls_pk_check_pair to compare keys
In the current test code, the object that is used as a public key in
the certificate also contains a private key. However this is because
of the way the stest code is built and does not demonstrate the API in
a useful way. Use mbedtls_pk_check_pair, which is not what real-world
code would do (since the private key would typically be in an external
cryptoprocessor) but is a more representative placeholder.
2018-04-30 12:07:56 +02:00
Gilles Peskine 1febfef561 Rename mbedtls_ssl_async_{get,set}_data for clarity
Rename to mbedtls_ssl_get_async_operation_data and
mbedtls_ssl_set_async_operation_data so that they're about
"async operation data" and not about some not-obvious "data".
2018-04-30 11:54:39 +02:00
Gilles Peskine 807d74a062 SSL async callback: cert is not always from mbedtls_ssl_conf_own_cert
The certificate passed to async callbacks may not be the one set by
mbedtls_ssl_conf_own_cert. For example, when using an SNI callback,
it's whatever the callback is using. Document this, and add a test
case (and code sample) with SNI.
2018-04-30 10:30:49 +02:00
Gilles Peskine 0b53e24c6b ssl_async_set_key: detect if ctx->slots overflows 2018-04-30 10:23:56 +02:00
Gilles Peskine 9de55fab56 Fix uninitialized variable in ssl_server2 2018-04-26 14:33:43 +02:00
Gilles Peskine 5bea9f61b9 Don't use the printf format %zd
We target C89 libc, so don't use %zd or %zu. Just use %u, and make
slot numbers `unsigned` for simplicity.
2018-04-26 13:29:17 +02:00
Gilles Peskine 8f97af7ea3 Don't pass the async config data to async callbacks
The config data is in the SSL config, so callbacks can retrieve it
from there, with the new function mbedtls_ssl_conf_get_async_config_data.
2018-04-26 11:46:10 +02:00
Gilles Peskine a36ac4fe68 New wrapper mbedtls_status_is_ssl_in_progress(ret)
Wrapper function to check whether a return status indicates that an
SSL operation is in progress.
2018-04-26 10:00:40 +02:00
Gilles Peskine c912572a7f ssl_server2: merge redundant async_private_error=resume/pk
Testing the case where the resume callback returns an error at the
beginning and the case where it returns an error at the end is
redundant. Keep the test after the output has been produced, to
validate that the product does not use even a valid output if the
return value is an error code.
2018-04-26 10:00:40 +02:00
Gilles Peskine d3268834f3 Document what the SSL async sign callback needs to do with RSA
Document how the SSL async sign callback must treat its md_alg and
hash parameters when doing an RSA signature: sign-the-hash if md_alg
is nonzero (TLS 1.2), and sign-the-digestinfo if md_alg is zero
(TLS <= 1.1).

In ssl_server2, don't use md_alg=MBEDTLS_MD_NONE to indicate that
ssl_async_resume must perform an encryption, because md_alg is also
MBEDTLS_MD_NONE in TLS <= 1.1. Add a test case to exercise this
case (signature with MBEDTLS_MD_NONE).
2018-04-26 10:00:40 +02:00
Gilles Peskine ceb541b7de ssl_server2: rename delay in ssl_async_operation_context_t
Rename to remaining_delay to convey that it is decremented over time.
2018-04-26 10:00:40 +02:00
Gilles Peskine ad28bf0e58 Documentation improvements 2018-04-26 10:00:40 +02:00
Gilles Peskine df13d5c7a6 Pass the SSL context to async callbacks
When a handshake step starts an asynchronous operation, the
application needs to know which SSL connection the operation is for,
so that when the operation completes, the application can wake that
connection up. Therefore the async start callbacks need to take the
SSL context as an argument. It isn't enough to let them set a cookie
in the SSL connection, the application needs to be able to find the
right SSL connection later.

Also pass the SSL context to the other callbacks for consistency. Add
a new field to the handshake that the application can use to store a
per-connection context. This new field replaces the former
context (operation_ctx) that was created by the start function and
passed to the resume function.

Add a boolean flag to the handshake structure to track whether an
asynchronous operation is in progress. This is more robust than
relying on the application to set a non-null application context.
2018-04-26 10:00:40 +02:00
Gilles Peskine b74a1c73b1 Rename MBEDTLS_SSL_ASYNC_PRIVATE_C to MBEDTLS_SSL_ASYNC_PRIVATE
This is an optional feature, not a module of its own, so don't call it
MBEDTLS_xxx_C and put it in the appropriate section of config.h.
2018-04-26 10:00:39 +02:00
Gilles Peskine f112725487 Style and grammar fixes 2018-04-26 10:00:39 +02:00
Gilles Peskine 12ab5d4cfb Don't shadow the variable p 2018-04-26 10:00:39 +02:00
Gilles Peskine b44692f126 Merge branch 'mbedtls_ssl_get_key_exchange_md_ssl_tls-return_hashlen' into tls_async_server-2.9
Conflict resolution:
* ChangeLog: put the new entry from my branch in the proper place.
* include/mbedtls/error.h: counted high-level module error codes again.
* include/mbedtls/ssl.h: picked different numeric codes for the
  concurrently added errors; made the new error a full sentence per
  current standards.
* library/error.c: ran scripts/generate_errors.pl.
* library/ssl_srv.c:
    * ssl_prepare_server_key_exchange "DHE key exchanges": the conflict
      was due to style corrections in development
      (4cb1f4d49c) which I merged with
      my refactoring.
    * ssl_prepare_server_key_exchange "For key exchanges involving the
      server signing", first case, variable declarations: merged line
      by line:
        * dig_signed_len: added in async
        * signature_len: removed in async
        * hashlen: type changed to size_t in development
        * hash: size changed to MBEDTLS_MD_MAX_SIZE in async
        * ret: added in async
    * ssl_prepare_server_key_exchange "For key exchanges involving the
      server signing", first cae comment: the conflict was due to style
      corrections in development (4cb1f4d49c)
      which I merged with my comment changes made as part of refactoring
      the function.
    * ssl_prepare_server_key_exchange "Compute the hash to be signed" if
      `md_alg != MBEDTLS_MD_NONE`: conflict between
      ebd652fe2d
      "ssl_write_server_key_exchange: calculate hashlen explicitly" and
      46f5a3e9b4 "Check return codes from
      MD in ssl code". I took the code from commit
      ca1d742904 made on top of development
      which makes mbedtls_ssl_get_key_exchange_md_ssl_tls return the
      hash length.
* programs/ssl/ssl_server2.c: multiple conflicts between the introduction
  of MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS and new auxiliary functions and
  definitions for async support, and the introduction of idle().
    * definitions before main: concurrent additions, kept both.
    * main, just after `handshake:`: in the loop around
      mbedtls_ssl_handshake(), merge the addition of support for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS and SSL_ASYNC_INJECT_ERROR_CANCEL
      with the addition of the idle() call.
    * main, if `opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM`: take the
      code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS.
    * main, loop around mbedtls_ssl_read() in the datagram case:
      take the code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS; revert to a do...while loop.
    * main, loop around mbedtls_ssl_write() in the datagram case:
      take the code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS; revert to a do...while loop.
2018-04-26 10:00:27 +02:00
Gilles Peskine fcca9d8cef SSL asynchronous decryption (server side): tests
Test SSL asynchronous private operation for the case of a
decryption operation on a server.
2018-04-24 09:36:37 +02:00
Gilles Peskine 60ee4cadce SSL asynchronous signature: basic test of recovery after error
Add test cases where the server goes through an async operation which
fails, then the server makes a successful connection.
2018-04-24 09:36:36 +02:00
Gilles Peskine 3665f1db9d SSL asynchronous signature: test cases
Add test cases for SSL asynchronous signature to ssl-opt.sh:

* Delay=0,1 to test the sequences of calls to f_async_resume
* Test fallback when the async callbacks don't support that key
* Test error injection at each stage
* Test renegotiation
2018-04-24 09:36:36 +02:00
Gilles Peskine 9eb5e9a16e SSL asynchronous private key operation callbacks: test server
New options in ssl_server2 to use the asynchronous private key
operation feature.

Features: resume delay to call resume more than once; error injection
at each stage; renegotiation support.
2018-04-24 09:32:29 +02:00
Hanno Becker 2bd57578af Merge branch 'development' into iotssl-1204 2018-03-28 14:52:35 +01:00
Hanno Becker ef52796537 Fix missing return statement ssl_server2 idling
Also, introduce MBEDTLS_EINTR locally in net_sockets.c
for the platform-dependent return code macro used by
the `select` call to indicate that the poll was interrupted
by a signal handler: On Unix, the corresponding macro is EINTR,
while on Windows, it's WSAEINTR.
2018-03-15 15:52:31 +00:00
Hanno Becker 9b2b66ebd2 Minor style corrections
Move function block brace outside conditional compilation
to not confuse some editors, and correct indentation.
2018-03-15 12:21:15 +00:00
Hanno Becker adfa64f0c4 Abort idle-loop in ssl_server2 if sockets gets invalid
Previously, the idling loop in ssl_server2 didn't check whether
the underlying call to mbedtls_net_poll signalled that the socket
became invalid. This had the consequence that during idling, the
server couldn't be terminated through a SIGTERM, as the corresponding
handler would only close the sockets and expect the remainder of
the program to shutdown gracefully as a consequence of this.
This was subsequently attempted to be fixed through a change
in ssl-opt.sh by terminating the server through a KILL signal,
which however lead to other problems when the latter was run
under valgrind.

This commit changes the idling loop in ssl_server2 and ssl_client2
to obey the return code of mbedtls_net_poll and gracefully shutdown
if an error occurs, e.g. because the socket was closed.

As a consequence, the server termination via a KILL signal in
ssl-opt.sh is no longer necessary, with the previous `kill; wait`
pattern being sufficient. The commit reverts the corresponding
change.
2018-03-15 11:43:41 +00:00
Hanno Becker b6f880b63b Revert whitespace change to ease merging 2018-03-13 12:48:50 +00:00
Hanno Becker ddc3ebbc3f Exemplify use of mbedtls_ssl_check_pending in ssl_server2.c 2018-03-13 11:48:32 +00:00
Manuel Pégourié-Gonnard 4fa619fe56 Fix race condition in error printing in ssl_server2.c
The race goes this way:
1. ssl_recv() succeeds (ie no signal received yet)
2. processing the message leads to aborting handshake with ret != 0
3. reset ret if we were signaled
4. print error if ret is still non-zero
5. go back to net_accept() which can be interrupted by a signal
We print the error message only if the signal is received between steps 3 and
5, not when it arrives between steps 1 and 3.

This can cause failures in ssl-opt.sh where we check for the presence of "Last
error was..." in the server's output: if we perform step 2, the client will be
notified and exit, then ssl-opt.sh will send SIGTERM to the server, but if it
didn't get a chance to run and pass step 3 in the meantime, we're in trouble.

The purpose of step 3 was to avoid spurious "Last error" messages in the
output so that ssl-opt.sh can check for a successful run by the absence of
that message. However, it is enough to suppress that message when the last
error we get is the one we expect from being interrupted by a signal - doing
more could hide real errors.

Also, improve the messages printed when interrupted to make it easier to
distinguish the two cases - this could be used in a testing script wanted to
check that the server doesn't see the client as disconnecting unexpectedly.
2018-01-25 11:27:24 +01:00
Gilles Peskine 68306ed31f Merge remote-tracking branch 'upstream-public/pr/1094' into development 2017-11-23 20:02:46 +01:00