/*
* Destroy any cached session data
*/
- fr_tls_cache_deny(tls_session);
+ fr_tls_cache_deny(request, tls_session);
if (eap_tls_compose(request, eap_session, EAP_TLS_FAIL,
eap_tls_session->base_flags, NULL, 0, 0) < 0) return -1;
case FR_TLS_RESULT_ERROR:
REDEBUG("TLS receive handshake failed during operation");
- fr_tls_cache_deny(tls_session);
+ fr_tls_cache_deny(request, tls_session);
eap_tls_session->state = EAP_TLS_FAIL;
goto finish;
(void) fr_tls_session_async_handshake_push(request, tls_session);
if (tls_session->result != FR_TLS_RESULT_SUCCESS) {
REDEBUG("TLS receive handshake failed during operation");
- fr_tls_cache_deny(tls_session);
+ fr_tls_cache_deny(request, tls_session);
eap_tls_session->state = EAP_TLS_FAIL;
return UNLANG_ACTION_CALCULATE_RESULT;
}
tls_cache->clear.state = FR_TLS_CACHE_CLEAR_REQUESTED;
/*
- * Go and do the delete now, instead of at some
- * indeterminate point in the future.
+ * We store a copy of the pointer for the session
+ * in tls_session->session. If the session is
+ * being freed then this pointer must be invalid
+ * so clear it to prevent crashes in other areas
+ * of the code.
*/
- ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */
+ if (tls_session->session == sess) tls_session->session = NULL;
+
+ /*
+ * Previously the code called ASYNC_pause_job();
+ * assuming this callback would always be called
+ * from SSL_read() or another SSL function.
+ *
+ * Unfortunately it appears that the call path
+ * can also be triggered with SSL_CTX_remove_session
+ * if the reference count on the SSL_SESSION
+ * drops to zero.
+ *
+ * We now check the 'can_pause' flag to determine
+ * if we're inside a yieldable SSL_read call.
+ */
+ if (tls_session->can_pause) ASYNC_pause_job();
}
/** Process the result of `session load { ... }`
MEM(tls_cache->load.id = talloc_typed_memdup(tls_cache, (uint8_t const *)key, key_len));
RDEBUG3("Requested session load - ID %pV", fr_box_octets_buffer(tls_cache->load.id));
- ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */
+
+ /*
+ * Cache functions are only allowed during the handshake
+ * FIXME: With TLS 1.3 session tickets can be sent
+ * later... Technically every point where we call
+ * SSL_read() may need to be a yield point.
+ */
+ if (unlikely(!tls_session->can_pause)) {
+ cant_pause:
+ fr_assert_msg("Unexpected call to %s. "
+ "tls_session_async_handshake_cont must be in call stack", __FUNCTION__);
+ return NULL;
+ }
+ /*
+ * Jumps back to SSL_read() in session.c
+ *
+ * Be aware that if the request is cancelled
+ * whatever was meant to be done during the
+ * time we yielded may not have been completed.
+ */
+ ASYNC_pause_job();
/*
* load cache { ... } returned, but the parent
*/
fr_tls_verify_cert_request(tls_session, true);
- ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */
+ if (unlikely(!tls_session->can_pause)) goto cant_pause;
+ /*
+ * Jumps back to SSL_read() in session.c
+ *
+ * Be aware that if the request is cancelled
+ * whatever was meant to be done during the
+ * time we yielded may not have been completed.
+ */
+ ASYNC_pause_job();
/*
* Certificate validation returned but the request
*/
static void tls_cache_delete_cb(UNUSED SSL_CTX *ctx, SSL_SESSION *sess)
{
-
-
/*
* Not sure why this happens, but sometimes SSL_SESSION *s
* make it here without the correct ex data.
return 0;
}
-/** Prevent a TLS session from being cached
+/** Prevent a pending TLS session being persisted, and clear any resumed sessions
*
- * Usually called if the session has failed for some reason.
+ * Usually called if authentication has failed for some reason.
*
* Will clear any serialized data out of the tls_session structure
* and should result in tls_cache_delete_cb being called.
*
- * @param[in] tls_session on which to prevent resumption.
+ * @note Calling this function will immediately free the memory used
+ * by the session, but not the external persisted copy of the
+ * session. To clear the persisted copy #fr_tls_cache_pending_push
+ * must be called in a place where the caller is prepared to yield.
+ * In most cases this means whether the handshake is a success or
+ * failure, the last thing the caller of the TLS code should do
+ * is set the result, and call #fr_tls_cache_pending_push.
+ *
+ * @param[in] request to use for running any async cache actions.
+ * @param[in] tls_session on which to prevent resumption.
*/
-void fr_tls_cache_deny(fr_tls_session_t *tls_session)
+void fr_tls_cache_deny(request_t *request, fr_tls_session_t *tls_session)
{
fr_tls_cache_t *tls_cache = tls_session->cache;
+ bool tmp_bind = !fr_tls_session_request_bound(tls_session->ssl);
/*
- * Even for 1.1.0 we don't know when this function
- * will be called, so better to remove the session
- * directly.
+ * This is necessary to allow this function to
+ * be called inside and outside of OpenSSL handshake
+ * code.
+ */
+ if (tmp_bind) {
+ fr_tls_session_request_bind(tls_session->ssl, request);
+ /*
+ * If there's already a request bound, it better be
+ * the one passed to this function.
+ */
+ } else {
+ fr_assert(fr_tls_session_request(tls_session->ssl) == request);
+ }
+
+ /*
+ * SSL_CTX_remove_session frees the previously loaded
+ * session in tls_session. If the reference count reaches zero
+ * the SSL_CTX_sess_remove_cb is called, which in our code is
+ * tls_cache_delete_cb.
+ *
+ * tls_cache_delete_cb calls tls_cache_delete_request
+ * to record the ID of tls_session->session
+ * in our pending cache state structure.
+ *
+ * tls_cache_delete_request does NOT immediately call the
+ * `cache clear {}` section as that must be done in a code area
+ * which is prepared to yield.
+ *
+ * #fr_tls_cache_pending_push MUST be called to actually
+ * clear external data.
*/
if (tls_session->session) SSL_CTX_remove_session(tls_session->ctx, tls_session->session);
+ if (tmp_bind) fr_tls_session_request_unbind(tls_session->ssl);
+
tls_session->allow_session_resumption = false;
+
+ /*
+ * Clear any pending store requests.
+ */
tls_cache_store_state_reset(fr_tls_session_request(tls_session->ssl), tls_cache);
}
*/
fr_tls_verify_cert_request(tls_session, true);
- ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */
+ /*
+ * Cache functions are only allowed during the handshake
+ * FIXME: With TLS 1.3 session tickets can be sent
+ * later... Technically every point where we call
+ * SSL_read() may need to be a yield point.
+ */
+ if (unlikely(!tls_session->can_pause)) {
+ fr_assert_msg("Unexpected call to %s. "
+ "tls_session_async_handshake_cont must be in call stack", __FUNCTION__);
+ return SSL_TICKET_RETURN_IGNORE_RENEW;
+ }
+
+ /*
+ * Jumps back to SSL_read() in session.c
+ *
+ * Be aware that if the request is cancelled
+ * whatever was meant to be done during the
+ * time we yielded may not have been completed.
+ */
+ ASYNC_pause_job();
/*
* If the request was cancelled get everything back into
unlang_action_t fr_tls_cache_pending_push(request_t *request, fr_tls_session_t *tls_session);
-void fr_tls_cache_deny(fr_tls_session_t *tls_session);
+void fr_tls_cache_deny(request_t *request, fr_tls_session_t *tls_session);
int fr_tls_cache_disable_cb(SSL *ssl, int is_forward_secure);
if (action != FR_SIGNAL_CANCEL) return;
+ /*
+ * We might want to set can_pause = false here
+ * but that would trigger asserts in the
+ * cache code.
+ */
+
/*
* If SSL_get_error returns SSL_ERROR_WANT_ASYNC
* it means we're yielded in the middle of a
* If acting as a server SSL_set_accept_state must have
* been called before this function.
*/
+ tls_session->can_pause = true;
tls_session->last_ret = SSL_read(tls_session->ssl, tls_session->clean_out.data + tls_session->clean_out.used,
sizeof(tls_session->clean_out.data) - tls_session->clean_out.used);
+ tls_session->can_pause = false;
if (tls_session->last_ret > 0) {
tls_session->clean_out.used += tls_session->last_ret;
*/
tls_session->result = FR_TLS_RESULT_SUCCESS;
finish:
+ /*
+ * Was bound by caller
+ */
fr_tls_session_request_unbind(tls_session->ssl);
return UNLANG_ACTION_CALCULATE_RESULT;
}
tls_session->result = FR_TLS_RESULT_IN_PROGRESS;
- fr_tls_session_request_bind(tls_session->ssl, request);
+ fr_tls_session_request_bind(tls_session->ssl, request); /* May be unbound in this function or asynchronously */
/*
* This is a logic error. fr_tls_session_async_handshake
REDEBUG("Attempted to continue TLS handshake, but handshake has completed");
error:
tls_session->result = FR_TLS_RESULT_ERROR;
- fr_tls_session_request_unbind(tls_session->ssl);
+ fr_tls_session_request_unbind(tls_session->ssl); /* Was bound in this function */
return UNLANG_ACTION_CALCULATE_RESULT;
}
record_init(&tls_session->dirty_in);
}
- return tls_session_async_handshake_cont(p_result, priority, request, uctx);
+ return tls_session_async_handshake_cont(p_result, priority, request, uctx); /* Must unbind request, possibly asynchronously */
}
/** Push a handshake call onto the stack
request = request_alloc_internal(tls_session, NULL);
- fr_tls_session_request_bind(tls_session->ssl, request);
+ fr_tls_session_request_bind(tls_session->ssl, request); /* Is unbound in this function */
/*
* Add the message callback to identify what type of
ret = SSL_connect(tls_session->ssl);
if (ret <= 0) {
fr_tls_log_io_error(NULL, SSL_get_error(tls_session->ssl, ret), "SSL_connect (%s)", __FUNCTION__);
- fr_tls_session_request_unbind(tls_session->ssl);
+ fr_tls_session_request_unbind(tls_session->ssl); /* Was bound in this function */
talloc_free(tls_session);
return NULL;
tls_session->mtu = conf->fragment_size;
- fr_tls_session_request_unbind(tls_session->ssl);
+ fr_tls_session_request_unbind(tls_session->ssl); /* Was bound in this function */
return tls_session;
}
tls_session->ssl = ssl;
talloc_set_destructor(tls_session, _fr_tls_session_free);
- fr_tls_session_request_bind(tls_session->ssl, request);
+ fr_tls_session_request_bind(tls_session->ssl, request); /* Is unbound in this function */
/*
* Initialize callbacks
if (tmpl_aexpand(tls_session, &context_id, request, conf->cache.id_name, NULL, NULL) < 0) {
RPEDEBUG("Failed expanding session ID");
error:
- fr_tls_session_request_unbind(tls_session->ssl);
+ fr_tls_session_request_unbind(tls_session->ssl); /* Was bound in this function */
talloc_free(tls_session);
return NULL;
}
}
if (conf->cache.mode != FR_TLS_CACHE_DISABLED) {
- tls_session->allow_session_resumption = true; /* otherwise it's false */
+ tls_session->allow_session_resumption = true; /* otherwise it's false */
fr_tls_cache_session_alloc(tls_session);
}
- fr_tls_session_request_unbind(tls_session->ssl);
+ fr_tls_session_request_unbind(tls_session->ssl); /* Was bound in this function */
return tls_session;
}
bool invalid; //!< Whether heartbleed attack was detected.
bool client_cert_ok; //!< whether or not the client certificate was validated
+ bool can_pause; //!< If true, it's ok to pause the request
+ ///< using the OpenSSL async API.
uint8_t alerts_sent;
bool pending_alert;
tls_session = talloc_get_type_abort(SSL_get_ex_data(ssl, FR_TLS_EX_INDEX_TLS_SESSION), fr_tls_session_t);
request = fr_tls_session_request(tls_session->ssl);
+ /*
+ * If this error appears it suggests
+ * that OpenSSL is trying to perform post-handshake
+ * certificate validation which we don't support.
+ */
+ if (!tls_session->can_pause) {
+ fr_assert_msg("Unexpected call to %s. "
+ "tls_session_async_handshake_cont must be in call stack", __FUNCTION__);
+ return 0;
+ }
+
/*
* Bail out as quickly as possible, producing
* as few errors as possible.
*/
fr_tls_verify_cert_request(tls_session, SSL_session_reused(tls_session->ssl));
- ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */
+ /*
+ * Jumps back to SSL_read() in session.c
+ *
+ * Be aware that if the request is cancelled
+ * whatever was meant to be done during the
+ * time we yielded may not have been completed.
+ */
+ ASYNC_pause_job();
/*
* Just try and bail out as quickly as possible.
* the client can't re-use it.
*/
default:
- fr_tls_cache_deny(tls_session);
+ fr_tls_cache_deny(request, tls_session);
+ *p_result = RLM_MODULE_REJECT;
- RETURN_MODULE_REJECT;
+ /*
+ * We'll jump back to the caller
+ * in the unlang stack if this
+ * fails.
+ */
+ return fr_tls_cache_pending_push(request, tls_session); /* Run any pending cache clear operations */
}
}
projects / thirdparty / freeradius-server.git / commitdiff
