[PATCH 0/3] More work on deprecation/removal of clear text passwords

[PATCH 0/3] More work on deprecation/removal of clear text passwords

[Daniel P. Berrangé]

This deprecates the -iscsi clear text 'password' option
and deletes the previously deprecated -spice 'password'
option.

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 +++
 docs/about/deprecated.rst       | 19 +++++++++++--------
 docs/about/removed-features.rst |  7 +++++++
 qemu-options.hx                 | 11 ++---------
 ui/spice-core.c                 | 15 ---------------
 5 files changed, 23 insertions(+), 32 deletions(-)

-- 
2.38.1

[PATCH 1/3] 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

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 7f99d15b23..055df73306 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1889,7 +1889,7 @@ SRST
 ERST
 
 DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
-    "-iscsi [user=user][,password=password]\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"
-- 
2.38.1

Re: [PATCH 1/3] block: mention 'password-secret' option for -iscsi

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> 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
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qemu-options.hx | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7f99d15b23..055df73306 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1889,7 +1889,7 @@ SRST
>  ERST
>  
>  DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
> -    "-iscsi [user=user][,password=password]\n"
> +    "-iscsi [user=user][,password=password],password-secret=secret-id]\n"

Loos like you're missing a bracket before the comma.

The line below also lacks one at the end.

>      "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
>      "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
>      "       [,timeout=timeout]\n"

Re: [PATCH 1/3] block: mention 'password-secret' option for -iscsi

[Daniel P. Berrangé]

On Thu, Dec 01, 2022 at 09:58:12AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  qemu-options.hx | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 7f99d15b23..055df73306 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1889,7 +1889,7 @@ SRST
> >  ERST
> >  
> >  DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
> > -    "-iscsi [user=user][,password=password]\n"
> > +    "-iscsi [user=user][,password=password],password-secret=secret-id]\n"
> 
> Loos like you're missing a bracket before the comma.
> 
> The line below also lacks one at the end.

Thanks, will fix both.

> 
> >      "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
> >      "       [,initiator-name=initiator-iqn][,id=target-iqn]\n"
> >      "       [,timeout=timeout]\n"
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 2/3] 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.

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

diff --git a/block/iscsi.c b/block/iscsi.c
index a316d46d96..58c0623052 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1352,6 +1352,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 93affe3669..2cc8924fe9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -267,6 +267,17 @@ Options are:
     - move backing file to NVDIMM storage and keep ``pmem=on``
       (to have NVDIMM with persistence guaranties).
 
+Block driver options
+--------------------
+
+``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.
+
 Device options
 --------------
 
-- 
2.38.1

Re: [PATCH 2/3] block: deprecate iSCSI 'password' in favour of 'password-secret'

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> 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.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/iscsi.c             |  3 +++
>  docs/about/deprecated.rst | 11 +++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a316d46d96..58c0623052 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1352,6 +1352,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 93affe3669..2cc8924fe9 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -267,6 +267,17 @@ Options are:
>      - move backing file to NVDIMM storage and keep ``pmem=on``
>        (to have NVDIMM with persistence guaranties).
>  
> +Block driver options
> +--------------------

I'm not sure about this headline.  For what it's worth, -help shows
-iscsi under "Block device options".

> +
> +``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.
> +
>  Device options
>  --------------

[PATCH 3/3] 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.

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 2cc8924fe9..ee4301f96d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -73,14 +73,6 @@ Input parameters that take a size value should only use a size suffix
 the value is hexadecimal.  That is, '0x20M' is deprecated, and should
 be written either as '32M' or as '0x2000000'.
 
-``-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 63df9848fd..e04e095320 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -408,6 +408,13 @@ pcspk-audiodev=<name>``.
 
 Use ``-device`` instead.
 
+``-spice password=string`` (removed in 8.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.
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/qemu-options.hx b/qemu-options.hx
index 055df73306..8a326f4dbb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2132,7 +2132,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"
@@ -2158,13 +2158,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 c3ac20ad43..15fba68e31 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -413,9 +413,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,
@@ -671,20 +668,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.38.1

Re: [PATCH 3/3] ui: remove deprecated 'password' option for SPICE

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> This has been replaced by the 'password-secret' option,
> which references a 'secret' object instance.
>
> 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 2cc8924fe9..ee4301f96d 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -73,14 +73,6 @@ Input parameters that take a size value should only use a size suffix
>  the value is hexadecimal.  That is, '0x20M' is deprecated, and should
>  be written either as '32M' or as '0x2000000'.
>  
> -``-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 63df9848fd..e04e095320 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -408,6 +408,13 @@ pcspk-audiodev=<name>``.
>  
>  Use ``-device`` instead.
>  
> +``-spice password=string`` (removed in 8.0)
> +'''''''''''''''''''''''''''''''''''''''''''
> +
> +This option is insecure because the SPICE password remains visible in

Nitpicking...  since the option doesn't exist anymore, it can't *be*
insecure.  It sure *was* insecure.

> +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.
>  
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------

[...]

Reviewed-by: Markus Armbruster <armbru@redhat.com>