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é <berra...@redhat.com>
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
Acked-by: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Fabiano Rosas <faro...@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
