Improve robustness and documentation of session cache implementation

[hanno-becker]

Context: The documentation for the API to set session cache callbacks says:

#if defined(MBEDTLS_SSL_SRV_C)
/**
 * \brief          Set the session cache callbacks (server-side only)
 *                 If not set, no session resuming is done (except if session
 *                 tickets are enabled too).
 *
 *                 ...
 *
 *                 The get callback is called once during the initial handshake
 *                 to enable session resuming. The get function has the
 *                 following parameters: (void *parameter, mbedtls_ssl_session *session)
 *                 If a valid entry is found, it should fill the master of
 *                 the session object with the cached values and return 0,
 *                 return 1 otherwise. Optionally peer_cert can be set as well
 *                 if it is properly present in cache entry.
 *
 *                 ...
 *
 * \param conf           SSL configuration
 * \param p_cache        parmater (context) for both callbacks
 * \param f_get_cache    session get callback
 * \param f_set_cache    session set callback
 */
void mbedtls_ssl_conf_session_cache( mbedtls_ssl_config *conf,
        void *p_cache,
        int (*f_get_cache)(void *, mbedtls_ssl_session *),
        int (*f_set_cache)(void *, const mbedtls_ssl_session *) );
#endif /* MBEDTLS_SSL_SRV_C */

Issue: The documentation needs to state more clearly which fields of the mbedtls_ssl_session structure the callback f_get_cache() assumes to be populated, what the expected state of the other fields is, and in which way f_get_cache() affects the structure on success or failure.

For example, consider the following snippet of mbedtls_ssl_cache_get() from the default session cache implementation shipped with Mbed TLS: https://github.com/ARMmbed/mbedtls/blob/f352f75f6bd5734c8f671323dd6ab32472d5da34/library/ssl_cache.c#L86-L120 In case the target session already has a peer CRT configured for whatever reason, it'll leak in memory; if it's clearly documented that the target session must not have a peer CRT configured, that could be acceptable (although even in that case one might argue that it's more robust to check for an existing peer CRT first), but as it stands, this behaviour cannot be expected.

This behaviour of the default session cache implementation is not a problem with the way Mbed TLS uses it, because f_get_cache() is only called from within ssl_write_server_hello(), which is not re-entrant (even in DTLS, we make a raw backup of the ServerHello for retransmission, instead of calling ssl_write_server_hello() again). Moreover, the code guards against the peer CRT potentially being set despite of a failing call to f_get_cache() when parsing the peer certificate: https://github.com/ARMmbed/mbedtls/blob/f352f75f6bd5734c8f671323dd6ab32472d5da34/library/ssl_tls.c#L5528-L5533 However, one might consider moving this code-path to right after a failed invocation of f_get_cache(), to keep session_negotiate clean even on session resumption failure.

[ciarmcom]

ARM Internal Ref: IOTSSL-2769