[PULL 0/5] Misc next patches

[PULL 0/5] Misc next patches

[Daniel P. Berrangé]

The following changes since commit 6a50f64ca01d0a7b97f14f069762bfd88160f31e:

  Merge tag 'pull-request-2023-02-14' of https://gitlab.com/thuth/qemu into staging (2023-02-14 14:46:10 +0000)

are available in the Git repository at:

  https://gitlab.com/berrange/qemu tags/misc-next-pull-request

for you to fetch changes up to 36debafddd788066be10b33c5f11b984a08e5c85:

  ui: remove deprecated 'password' option for SPICE (2023-02-15 11:14:58 -0500)

----------------------------------------------------------------
 * Document 'password-secret' option for -iscsi
 * Deprecate iSCSI 'password' in favour of 'password-secret'
 * Remove deprecated 'password' option for SPICE
 * Fix handling of cached read buffers with TLS

----------------------------------------------------------------

Antoine Damhet (2):
  crypto: TLS: introduce `check_pending`
  io/channel-tls: fix handling of bigger read buffers

Daniel P. Berrangé (3):
  block: mention 'password-secret' option for -iscsi
  block: deprecate iSCSI 'password' in favour of 'password-secret'
  ui: remove deprecated 'password' option for SPICE

 block/iscsi.c                   |  3 ++
 crypto/tlssession.c             | 14 +++++++
 docs/about/deprecated.rst       | 16 ++++----
 docs/about/removed-features.rst |  7 ++++
 include/crypto/tlssession.h     | 11 ++++++
 io/channel-tls.c                | 66 ++++++++++++++++++++++++++++++++-
 qemu-options.hx                 | 13 ++-----
 ui/spice-core.c                 | 15 --------
 8 files changed, 111 insertions(+), 34 deletions(-)

-- 
2.39.1

[PULL 1/5] crypto: TLS: introduce `check_pending`

[Daniel P. Berrangé]

From: Antoine Damhet <antoine.damhet@shadow.tech>

The new `qcrypto_tls_session_check_pending` function allows the caller
to know if data have already been consumed from the backend and is
already available.

Signed-off-by: Antoine Damhet <antoine.damhet@shadow.tech>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlssession.c         | 14 ++++++++++++++
 include/crypto/tlssession.h | 11 +++++++++++
 2 files changed, 25 insertions(+)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index b302d835d2..1e98f44e0d 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -493,6 +493,13 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
 }
 
 
+size_t
+qcrypto_tls_session_check_pending(QCryptoTLSSession *session)
+{
+    return gnutls_record_check_pending(session->handle);
+}
+
+
 int
 qcrypto_tls_session_handshake(QCryptoTLSSession *session,
                               Error **errp)
@@ -615,6 +622,13 @@ qcrypto_tls_session_read(QCryptoTLSSession *sess,
 }
 
 
+size_t
+qcrypto_tls_session_check_pending(QCryptoTLSSession *session)
+{
+    return 0;
+}
+
+
 int
 qcrypto_tls_session_handshake(QCryptoTLSSession *sess,
                               Error **errp)
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index 15b9cef086..571049bd0e 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -248,6 +248,17 @@ ssize_t qcrypto_tls_session_read(QCryptoTLSSession *sess,
                                  char *buf,
                                  size_t len);
 
+/**
+ * qcrypto_tls_session_check_pending:
+ * @sess: the TLS session object
+ *
+ * Check if there are unread data in the TLS buffers that have
+ * already been read from the underlying data source.
+ *
+ * Returns: the number of bytes available or zero
+ */
+size_t qcrypto_tls_session_check_pending(QCryptoTLSSession *sess);
+
 /**
  * qcrypto_tls_session_handshake:
  * @sess: the TLS session object
-- 
2.39.1

[PULL 2/5] io/channel-tls: fix handling of bigger read buffers

[Daniel P. Berrangé]

From: Antoine Damhet <antoine.damhet@shadow.tech>

Since the TLS backend can read more data from the underlying QIOChannel
we introduce a minimal child GSource to notify if we still have more
data available to be read.

Signed-off-by: Antoine Damhet <antoine.damhet@shadow.tech>
Signed-off-by: Charles Frey <charles.frey@shadow.tech>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 io/channel-tls.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/io/channel-tls.c b/io/channel-tls.c
index c730cb8ec5..8052945ba0 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -389,12 +389,76 @@ static void qio_channel_tls_set_aio_fd_handler(QIOChannel *ioc,
     qio_channel_set_aio_fd_handler(tioc->master, ctx, io_read, io_write, opaque);
 }
 
+typedef struct QIOChannelTLSSource QIOChannelTLSSource;
+struct QIOChannelTLSSource {
+    GSource parent;
+    QIOChannelTLS *tioc;
+};
+
+static gboolean
+qio_channel_tls_source_check(GSource *source)
+{
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+
+    return qcrypto_tls_session_check_pending(tsource->tioc->session) > 0;
+}
+
+static gboolean
+qio_channel_tls_source_prepare(GSource *source, gint *timeout)
+{
+    *timeout = -1;
+    return qio_channel_tls_source_check(source);
+}
+
+static gboolean
+qio_channel_tls_source_dispatch(GSource *source, GSourceFunc callback,
+                                gpointer user_data)
+{
+    return G_SOURCE_CONTINUE;
+}
+
+static void
+qio_channel_tls_source_finalize(GSource *source)
+{
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+
+    object_unref(OBJECT(tsource->tioc));
+}
+
+static GSourceFuncs qio_channel_tls_source_funcs = {
+    qio_channel_tls_source_prepare,
+    qio_channel_tls_source_check,
+    qio_channel_tls_source_dispatch,
+    qio_channel_tls_source_finalize
+};
+
+static void
+qio_channel_tls_read_watch(QIOChannelTLS *tioc, GSource *source)
+{
+    GSource *child;
+    QIOChannelTLSSource *tlssource;
+
+    child = g_source_new(&qio_channel_tls_source_funcs,
+                          sizeof(QIOChannelTLSSource));
+    tlssource = (QIOChannelTLSSource *)child;
+
+    tlssource->tioc = tioc;
+    object_ref(OBJECT(tioc));
+
+    g_source_add_child_source(source, child);
+}
+
 static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
                                              GIOCondition condition)
 {
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
+    GSource *source = qio_channel_create_watch(tioc->master, condition);
+
+    if (condition & G_IO_IN) {
+        qio_channel_tls_read_watch(tioc, source);
+    }
 
-    return qio_channel_create_watch(tioc->master, condition);
+    return source;
 }
 
 QCryptoTLSSession *
-- 
2.39.1

[PULL 3/5] block: mention 'password-secret' option for -iscsi

[Daniel P. Berrangé]

The 'password-secret' option was added

  commit b189346eb1784df95ed6fed610411dbf23d19e1f
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Jan 21 14:19:21 2016 +0000

    iscsi: add support for getting CHAP password via QCryptoSecret API

but was not mentioned in the command line docs

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 88e93c6103..e79ff4d8fb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1892,8 +1892,8 @@ SRST
 ERST
 
 DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
-    "-iscsi [user=user][,password=password]\n"
-    "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
+    "-iscsi [user=user][,password=password][,password-secret=secret-id]\n"
+    "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE]\n"
     "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
     "       [,timeout=timeout]\n"
     "                iSCSI session parameters\n", QEMU_ARCH_ALL)
-- 
2.39.1

[PULL 4/5] block: deprecate iSCSI 'password' in favour of 'password-secret'

[Daniel P. Berrangé]

Support for referencing secret objects was added in

  commit b189346eb1784df95ed6fed610411dbf23d19e1f
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Jan 21 14:19:21 2016 +0000

    iscsi: add support for getting CHAP password via QCryptoSecret API

The existing 'password' option is overdue for deprecation and
subsequent removal.

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/iscsi.c             | 3 +++
 docs/about/deprecated.rst | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index b3e10f40b6..ed3e87a548 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1353,6 +1353,9 @@ static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
     } else if (!password) {
         error_setg(errp, "CHAP username specified but no password was given");
         return;
+    } else {
+        warn_report("iSCSI block driver 'password' option is deprecated, "
+                    "use 'password-secret' instead");
     }
 
     if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index cb1ec72347..d31ffa86d4 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -301,6 +301,14 @@ The above, converted to the current supported format::
 
   json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
 
+``iscsi,password=xxx`` (since 8.0)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Specifying the iSCSI password in plain text on the command line using the
+``password`` option is insecure. The ``password-secret`` option should be
+used instead, to refer to a ``--object secret...`` instance that provides
+a password via a file, or encrypted.
+
 Backwards compatibility
 -----------------------
 
-- 
2.39.1

[PULL 5/5] ui: remove deprecated 'password' option for SPICE

[Daniel P. Berrangé]

This has been replaced by the 'password-secret' option,
which references a 'secret' object instance.

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/about/deprecated.rst       |  8 --------
 docs/about/removed-features.rst |  7 +++++++
 qemu-options.hx                 |  9 +--------
 ui/spice-core.c                 | 15 ---------------
 4 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d31ffa86d4..2827b0c0be 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -66,14 +66,6 @@ and will cause a warning.
 The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
 rather than ``delay=off``.
 
-``-spice password=string`` (since 6.0)
-''''''''''''''''''''''''''''''''''''''
-
-This option is insecure because the SPICE password remains visible in
-the process listing. This is replaced by the new ``password-secret``
-option which lets the password be securely provided on the command
-line using a ``secret`` object instance.
-
 ``-smp`` ("parameter=0" SMP configurations) (since 6.2)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4a84e6174f..e901637ce5 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -428,6 +428,13 @@ respectively. The actual backend names should be used instead.
 Use ``-drive if=pflash`` to configure the OTP device of the sifive_u
 RISC-V machine instead.
 
+``-spice password=string`` (removed in 8.0)
+'''''''''''''''''''''''''''''''''''''''''''
+
+This option was insecure because the SPICE password remained visible in
+the process listing. This was replaced by the new ``password-secret``
+option which lets the password be securely provided on the command
+line using a ``secret`` object instance.
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/qemu-options.hx b/qemu-options.hx
index e79ff4d8fb..cafd8be8ed 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2135,7 +2135,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "       [,tls-channel=[main|display|cursor|inputs|record|playback]]\n"
     "       [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n"
     "       [,sasl=on|off][,disable-ticketing=on|off]\n"
-    "       [,password=<string>][,password-secret=<secret-id>]\n"
+    "       [,password-secret=<secret-id>]\n"
     "       [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n"
     "       [,jpeg-wan-compression=[auto|never|always]]\n"
     "       [,zlib-glz-wan-compression=[auto|never|always]]\n"
@@ -2161,13 +2161,6 @@ SRST
     ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off``
         Force using the specified IP version.
 
-    ``password=<string>``
-        Set the password you need to authenticate.
-
-        This option is deprecated and insecure because it leaves the
-        password visible in the process listing. Use ``password-secret``
-        instead.
-
     ``password-secret=<secret-id>``
         Set the ID of the ``secret`` object containing the password
         you need to authenticate.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 72f8f1681c..76f7c2bc3d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -412,9 +412,6 @@ static QemuOptsList qemu_spice_opts = {
             .name = "unix",
             .type = QEMU_OPT_BOOL,
 #endif
-        },{
-            .name = "password",
-            .type = QEMU_OPT_STRING,
         },{
             .name = "password-secret",
             .type = QEMU_OPT_STRING,
@@ -666,20 +663,8 @@ static void qemu_spice_init(void)
     }
     passwordSecret = qemu_opt_get(opts, "password-secret");
     if (passwordSecret) {
-        if (qemu_opt_get(opts, "password")) {
-            error_report("'password' option is mutually exclusive with "
-                         "'password-secret'");
-            exit(1);
-        }
         password = qcrypto_secret_lookup_as_utf8(passwordSecret,
                                                  &error_fatal);
-    } else {
-        str = qemu_opt_get(opts, "password");
-        if (str) {
-            warn_report("'password' option is deprecated and insecure, "
-                        "use 'password-secret' instead");
-            password = g_strdup(str);
-        }
     }
 
     if (tls_port) {
-- 
2.39.1

Re: [PULL 0/5] Misc next patches

[Peter Maydell]

On Wed, 15 Feb 2023 at 17:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit 6a50f64ca01d0a7b97f14f069762bfd88160f31e:
>
>   Merge tag 'pull-request-2023-02-14' of https://gitlab.com/thuth/qemu into staging (2023-02-14 14:46:10 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/berrange/qemu tags/misc-next-pull-request
>
> for you to fetch changes up to 36debafddd788066be10b33c5f11b984a08e5c85:
>
>   ui: remove deprecated 'password' option for SPICE (2023-02-15 11:14:58 -0500)
>
> ----------------------------------------------------------------
>  * Document 'password-secret' option for -iscsi
>  * Deprecate iSCSI 'password' in favour of 'password-secret'
>  * Remove deprecated 'password' option for SPICE
>  * Fix handling of cached read buffers with TLS
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.

-- PMM