From: Fabiano Rosas <farosas@suse.de>
To: qemu-devel@nongnu.org
Cc: "Peter Xu" <peterx@redhat.com>,
"Daniel P . Berrangé" <berrange@redhat.com>
Subject: [PULL 03/22] crypto: Remove qcrypto_tls_session_get_handshake_status
Date: Fri, 14 Feb 2025 17:31:40 -0300 [thread overview]
Message-ID: <20250214203159.30168-4-farosas@suse.de> (raw)
In-Reply-To: <20250214203159.30168-1-farosas@suse.de>
The correct way of calling qcrypto_tls_session_handshake() requires
calling qcrypto_tls_session_get_handshake_status() right after it so
there's no reason to have a separate method.
Refactor qcrypto_tls_session_handshake() to inform the status in its
own return value and alter the callers accordingly.
No functional change.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
crypto/tlssession.c | 63 +++++++++++------------------
include/crypto/tlssession.h | 32 ++++-----------
io/channel-tls.c | 7 ++--
tests/unit/test-crypto-tlssession.c | 12 ++----
4 files changed, 39 insertions(+), 75 deletions(-)
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index d769d7a304..6d8f8df623 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -546,45 +546,35 @@ qcrypto_tls_session_handshake(QCryptoTLSSession *session,
Error **errp)
{
int ret = gnutls_handshake(session->handle);
- if (ret == 0) {
+ if (!ret) {
session->handshakeComplete = true;
- } else {
- if (ret == GNUTLS_E_INTERRUPTED ||
- ret == GNUTLS_E_AGAIN) {
- ret = 1;
- } else {
- if (session->rerr || session->werr) {
- error_setg(errp, "TLS handshake failed: %s: %s",
- gnutls_strerror(ret),
- error_get_pretty(session->rerr ?
- session->rerr : session->werr));
- } else {
- error_setg(errp, "TLS handshake failed: %s",
- gnutls_strerror(ret));
- }
- ret = -1;
- }
- }
- error_free(session->rerr);
- error_free(session->werr);
- session->rerr = session->werr = NULL;
-
- return ret;
-}
-
-
-QCryptoTLSSessionHandshakeStatus
-qcrypto_tls_session_get_handshake_status(QCryptoTLSSession *session)
-{
- if (session->handshakeComplete) {
return QCRYPTO_TLS_HANDSHAKE_COMPLETE;
- } else if (gnutls_record_get_direction(session->handle) == 0) {
- return QCRYPTO_TLS_HANDSHAKE_RECVING;
+ }
+
+ if (ret == GNUTLS_E_INTERRUPTED || ret == GNUTLS_E_AGAIN) {
+ int direction = gnutls_record_get_direction(session->handle);
+ return direction ? QCRYPTO_TLS_HANDSHAKE_SENDING :
+ QCRYPTO_TLS_HANDSHAKE_RECVING;
+ }
+
+ if (session->rerr || session->werr) {
+ error_setg(errp, "TLS handshake failed: %s: %s",
+ gnutls_strerror(ret),
+ error_get_pretty(session->rerr ?
+ session->rerr : session->werr));
} else {
- return QCRYPTO_TLS_HANDSHAKE_SENDING;
+ error_setg(errp, "TLS handshake failed: %s",
+ gnutls_strerror(ret));
}
+
+ error_free(session->rerr);
+ error_free(session->werr);
+ session->rerr = session->werr = NULL;
+
+ return -1;
}
+
int
qcrypto_tls_session_bye(QCryptoTLSSession *session, Error **errp)
{
@@ -726,13 +716,6 @@ qcrypto_tls_session_handshake(QCryptoTLSSession *sess,
}
-QCryptoTLSSessionHandshakeStatus
-qcrypto_tls_session_get_handshake_status(QCryptoTLSSession *sess)
-{
- return QCRYPTO_TLS_HANDSHAKE_COMPLETE;
-}
-
-
int
qcrypto_tls_session_bye(QCryptoTLSSession *session, Error **errp)
{
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index c0f64ce989..d77ae0d423 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -75,12 +75,14 @@
* GINT_TO_POINTER(fd));
*
* while (1) {
- * if (qcrypto_tls_session_handshake(sess, errp) < 0) {
+ * int ret = qcrypto_tls_session_handshake(sess, errp);
+ *
+ * if (ret < 0) {
* qcrypto_tls_session_free(sess);
* return -1;
* }
*
- * switch(qcrypto_tls_session_get_handshake_status(sess)) {
+ * switch(ret) {
* case QCRYPTO_TLS_HANDSHAKE_COMPLETE:
* if (qcrypto_tls_session_check_credentials(sess, errp) < )) {
* qcrypto_tls_session_free(sess);
@@ -170,7 +172,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoTLSSession, qcrypto_tls_session_free)
*
* Validate the peer's credentials after a successful
* TLS handshake. It is an error to call this before
- * qcrypto_tls_session_get_handshake_status() returns
+ * qcrypto_tls_session_handshake() returns
* QCRYPTO_TLS_HANDSHAKE_COMPLETE
*
* Returns 0 if the credentials validated, -1 on error
@@ -226,7 +228,7 @@ void qcrypto_tls_session_set_callbacks(QCryptoTLSSession *sess,
* registered with qcrypto_tls_session_set_callbacks()
*
* It is an error to call this before
- * qcrypto_tls_session_get_handshake_status() returns
+ * qcrypto_tls_session_handshake() returns
* QCRYPTO_TLS_HANDSHAKE_COMPLETE
*
* Returns: the number of bytes sent,
@@ -256,7 +258,7 @@ ssize_t qcrypto_tls_session_write(QCryptoTLSSession *sess,
* opposed to an error.
*
* It is an error to call this before
- * qcrypto_tls_session_get_handshake_status() returns
+ * qcrypto_tls_session_handshake() returns
* QCRYPTO_TLS_HANDSHAKE_COMPLETE
*
* Returns: the number of bytes received,
@@ -289,8 +291,7 @@ size_t qcrypto_tls_session_check_pending(QCryptoTLSSession *sess);
* the underlying data channel is non-blocking, then
* this method may return control before the handshake
* is complete. On non-blocking channels the
- * qcrypto_tls_session_get_handshake_status() method
- * should be used to determine whether the handshake
+ * return value determines whether the handshake
* has completed, or is waiting to send or receive
* data. In the latter cases, the caller should setup
* an event loop watch and call this method again
@@ -306,23 +307,6 @@ typedef enum {
QCRYPTO_TLS_HANDSHAKE_RECVING,
} QCryptoTLSSessionHandshakeStatus;
-/**
- * qcrypto_tls_session_get_handshake_status:
- * @sess: the TLS session object
- *
- * Check the status of the TLS handshake. This
- * is used with non-blocking data channels to
- * determine whether the handshake is waiting
- * to send or receive further data to/from the
- * remote peer.
- *
- * Once this returns QCRYPTO_TLS_HANDSHAKE_COMPLETE
- * it is permitted to send/receive payload data on
- * the channel
- */
-QCryptoTLSSessionHandshakeStatus
-qcrypto_tls_session_get_handshake_status(QCryptoTLSSession *sess);
-
typedef enum {
QCRYPTO_TLS_BYE_COMPLETE,
QCRYPTO_TLS_BYE_SENDING,
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 517ce190a4..ecde6b57bf 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -162,16 +162,17 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
GMainContext *context)
{
Error *err = NULL;
- QCryptoTLSSessionHandshakeStatus status;
+ int status;
- if (qcrypto_tls_session_handshake(ioc->session, &err) < 0) {
+ status = qcrypto_tls_session_handshake(ioc->session, &err);
+
+ if (status < 0) {
trace_qio_channel_tls_handshake_fail(ioc);
qio_task_set_error(task, err);
qio_task_complete(task);
return;
}
- status = qcrypto_tls_session_get_handshake_status(ioc->session);
if (status == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
trace_qio_channel_tls_handshake_complete(ioc);
if (qcrypto_tls_session_check_credentials(ioc->session,
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 3395f73560..554054e934 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -158,8 +158,7 @@ static void test_crypto_tls_session_psk(void)
rv = qcrypto_tls_session_handshake(serverSess,
&error_abort);
g_assert(rv >= 0);
- if (qcrypto_tls_session_get_handshake_status(serverSess) ==
- QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
+ if (rv == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
serverShake = true;
}
}
@@ -167,8 +166,7 @@ static void test_crypto_tls_session_psk(void)
rv = qcrypto_tls_session_handshake(clientSess,
&error_abort);
g_assert(rv >= 0);
- if (qcrypto_tls_session_get_handshake_status(clientSess) ==
- QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
+ if (rv == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
clientShake = true;
}
}
@@ -352,8 +350,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
rv = qcrypto_tls_session_handshake(serverSess,
&error_abort);
g_assert(rv >= 0);
- if (qcrypto_tls_session_get_handshake_status(serverSess) ==
- QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
+ if (rv == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
serverShake = true;
}
}
@@ -361,8 +358,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
rv = qcrypto_tls_session_handshake(clientSess,
&error_abort);
g_assert(rv >= 0);
- if (qcrypto_tls_session_get_handshake_status(clientSess) ==
- QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
+ if (rv == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
clientShake = true;
}
}
--
2.35.3
next prev parent reply other threads:[~2025-02-14 20:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-14 20:31 [PULL 00/22] Migration patches for 2025-02-14 Fabiano Rosas
2025-02-14 20:31 ` [PULL 01/22] crypto: Allow gracefully ending the TLS session Fabiano Rosas
2025-02-14 20:31 ` [PULL 02/22] io: tls: Add qio_channel_tls_bye Fabiano Rosas
2025-02-14 20:31 ` Fabiano Rosas [this message]
2025-02-14 20:31 ` [PULL 04/22] io: Add flags argument to qio_channel_readv_full_all_eof Fabiano Rosas
2025-02-14 20:31 ` [PULL 05/22] io: Add a read flag for relaxed EOF Fabiano Rosas
2025-02-14 20:31 ` [PULL 06/22] migration/multifd: Terminate the TLS connection Fabiano Rosas
2025-02-14 20:31 ` [PULL 07/22] migration/multifd: Add a compat property for TLS termination Fabiano Rosas
2025-02-14 20:31 ` [PULL 08/22] migration: Check migration error after loadvm Fabiano Rosas
2025-02-14 20:31 ` [PULL 09/22] migration: Set migration error outside of migrate_cancel Fabiano Rosas
2025-02-14 20:31 ` [PULL 10/22] migration: Unify migration_cancel and migrate_fd_cancel Fabiano Rosas
2025-02-14 20:31 ` [PULL 11/22] migration: Change migrate_fd_ to migration_ Fabiano Rosas
2025-02-14 20:31 ` [PULL 12/22] migration: Fix hang after error in destination setup phase Fabiano Rosas
2025-02-14 20:31 ` [PULL 13/22] migration: Reject qmp_migrate_cancel after postcopy Fabiano Rosas
2025-02-14 20:31 ` [PULL 14/22] migration: Don't set FAILED state when cancelling Fabiano Rosas
2025-02-14 20:31 ` [PULL 15/22] tests/qtest/migration: Introduce migration_test_add_suffix Fabiano Rosas
2025-02-14 20:31 ` [PULL 16/22] tests/qtest/migration: Add a cancel test Fabiano Rosas
2025-02-14 20:31 ` [PULL 17/22] migration: Update migrate_cancel documentation Fabiano Rosas
2025-02-14 20:31 ` [PULL 18/22] migration: use parameters.mode in cpr_state_save Fabiano Rosas
2025-02-14 20:31 ` [PULL 19/22] guestperf: Support deferred migration for multifd Fabiano Rosas
2025-02-14 20:31 ` [PULL 20/22] guestperf: Nitpick the inconsistent parameters Fabiano Rosas
2025-02-14 20:31 ` [PULL 21/22] guestperf: Introduce multifd compression option Fabiano Rosas
2025-02-14 20:31 ` [PULL 22/22] guestperf: Add test result data into report Fabiano Rosas
2025-02-17 8:25 ` [PULL 00/22] Migration patches for 2025-02-14 Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250214203159.30168-4-farosas@suse.de \
--to=farosas@suse.de \
--cc=berrange@redhat.com \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.