[PATCH 0/6] qemu: Enable client TLS certificate validation by default for chardev, migration, and backup servers
See patches 6 for a explanation. Peter Krempa (6): qemu: conf: Allow individual control of default value for *_tls_x509_verify qemu: conf: Clarify default of "vnc_tls_x509_verify" qemu: conf: Enable 'chardev_tls_x509_verify' by default qemu: conf: Enable 'migrate_tls_x509_verify' by default qemu: conf: Enable 'backup_tls_x509_verify' by default NEWS: Mention change of default for TLS certificate verification NEWS.rst | 11 +++++++++++ src/qemu/qemu.conf | 18 ++++++++++++++---- src/qemu/qemu_conf.c | 22 ++++++++++++++-------- src/qemu/qemu_conf.h | 1 + 4 files changed, 40 insertions(+), 12 deletions(-) -- 2.28.0
Store whether "default_tls_x509_verify" was provided and enhance the SET_TLS_VERIFY_DEFAULT macro so that indiviual users can provide their own default if "default_tls_x509_verify" config option was not provided. For now we keep setting it to 'false'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 22 ++++++++++++++-------- src/qemu/qemu_conf.h | 1 + 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f7d2b14f7..6f9d940477 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -40,6 +40,12 @@ # client-cert.pem - the client certificate signed with the ca-cert.pem # client-key.pem - the client private key # +# If this option is supplied it provides the default for the "_verify" option +# of specific TLS users such as vnc, backups, migration, etc. The specific +# users of TLS may override this by setting the specific "_verify" option. +# +# When not supplied the specific TLS users provide their own defaults. +# #default_tls_x509_verify = 1 # diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2fb2f021c2..c3a61816a4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -406,8 +406,10 @@ virQEMUDriverConfigLoadDefaultTLSEntry(virQEMUDriverConfigPtr cfg, if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir)) < 0) return -1; cfg->defaultTLSx509certdirPresent = (rv == 1); - if (virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify) < 0) + if ((rv = virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify)) < 0) return -1; + if (rv == 1) + cfg->defaultTLSx509verifyPresent = true; if (virConfGetValueString(conf, "default_tls_x509_secret_uuid", &cfg->defaultTLSx509secretUUID) < 0) return -1; @@ -1240,16 +1242,20 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) #undef SET_TLS_X509_CERT_DEFAULT -#define SET_TLS_VERIFY_DEFAULT(val) \ +#define SET_TLS_VERIFY_DEFAULT(val, defaultverify) \ do { \ - if (!cfg->val## TLSx509verifyPresent) \ - cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \ + if (!cfg->val## TLSx509verifyPresent) {\ + if (cfg->defaultTLSx509verifyPresent) \ + cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \ + else \ + cfg->val## TLSx509verify = defaultverify;\ + }\ } while (0) - SET_TLS_VERIFY_DEFAULT(vnc); - SET_TLS_VERIFY_DEFAULT(chardev); - SET_TLS_VERIFY_DEFAULT(migrate); - SET_TLS_VERIFY_DEFAULT(backup); + SET_TLS_VERIFY_DEFAULT(vnc, false); + SET_TLS_VERIFY_DEFAULT(chardev, false); + SET_TLS_VERIFY_DEFAULT(migrate, false); + SET_TLS_VERIFY_DEFAULT(backup, false); #undef SET_TLS_VERIFY_DEFAULT diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da03a184c1..8748212a82 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -108,6 +108,7 @@ struct _virQEMUDriverConfig { char *defaultTLSx509certdir; bool defaultTLSx509certdirPresent; bool defaultTLSx509verify; + bool defaultTLSx509verifyPresent; char *defaultTLSx509secretUUID; bool vncAutoUnixSocket; -- 2.28.0
On 11/13/20 4:01 PM, Peter Krempa wrote:
Store whether "default_tls_x509_verify" was provided and enhance the SET_TLS_VERIFY_DEFAULT macro so that indiviual users can provide their own default if "default_tls_x509_verify" config option was not provided.
For now we keep setting it to 'false'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 22 ++++++++++++++-------- src/qemu/qemu_conf.h | 1 + 3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2fb2f021c2..c3a61816a4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -406,8 +406,10 @@ virQEMUDriverConfigLoadDefaultTLSEntry(virQEMUDriverConfigPtr cfg, if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir)) < 0) return -1; cfg->defaultTLSx509certdirPresent = (rv == 1); - if (virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify) < 0) + if ((rv = virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify)) < 0) return -1; + if (rv == 1) + cfg->defaultTLSx509verifyPresent = true; if (virConfGetValueString(conf, "default_tls_x509_secret_uuid", &cfg->defaultTLSx509secretUUID) < 0) return -1; @@ -1240,16 +1242,20 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
#undef SET_TLS_X509_CERT_DEFAULT
-#define SET_TLS_VERIFY_DEFAULT(val) \ +#define SET_TLS_VERIFY_DEFAULT(val, defaultverify) \ do { \ - if (!cfg->val## TLSx509verifyPresent) \ - cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \ + if (!cfg->val## TLSx509verifyPresent) {\ + if (cfg->defaultTLSx509verifyPresent) \ + cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \ + else \ + cfg->val## TLSx509verify = defaultverify;\
Alignment.
+ }\ } while (0)
Michal
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing from the config file the client certificate validation is disabled. VNC provides a layer of authentication so client certificate validation is not strictly required. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f9d940477..f40963ce48 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -119,7 +119,8 @@ # CA in the vnc_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either +# the default is "0". # #vnc_tls_x509_verify = 1 -- 2.28.0
On 11/13/20 4:01 PM, Peter Krempa wrote:
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing from the config file the client certificate validation is disabled. VNC provides a layer of authentication so client certificate validation is not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f9d940477..f40963ce48 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -119,7 +119,8 @@ # CA in the vnc_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either
No native speaker, but perhaps s/either/neither/? Applies for next patches too.
+# the default is "0". # #vnc_tls_x509_verify = 1
Michal
On a Friday in 2020, Michal Privoznik wrote:
On 11/13/20 4:01 PM, Peter Krempa wrote:
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing from the config file the client certificate validation is disabled. VNC provides a layer of authentication so client certificate validation is not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f9d940477..f40963ce48 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -119,7 +119,8 @@ # CA in the vnc_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either
No native speaker, but perhaps s/either/neither/? Applies for next patches too.
Not a native speaker either, but your suggested substitution sounds weirder. Jano
+# the default is "0". # #vnc_tls_x509_verify = 1
Michal
On Fri, Nov 13, 2020 at 04:38:06PM +0100, Michal Privoznik wrote:
On 11/13/20 4:01 PM, Peter Krempa wrote:
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing from the config file the client certificate validation is disabled. VNC provides a layer of authentication so client certificate validation is not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f9d940477..f40963ce48 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -119,7 +119,8 @@ # CA in the vnc_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either
No native speaker, but perhaps s/either/neither/? Applies for next patches too.
That wouldn't be right - what's there now is fine. 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 :|
On 11/13/20 4:45 PM, Daniel P. Berrangé wrote:
On Fri, Nov 13, 2020 at 04:38:06PM +0100, Michal Privoznik wrote:
On 11/13/20 4:01 PM, Peter Krempa wrote:
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing from the config file the client certificate validation is disabled. VNC provides a layer of authentication so client certificate validation is not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6f9d940477..f40963ce48 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -119,7 +119,8 @@ # CA in the vnc_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either
No native speaker, but perhaps s/either/neither/? Applies for next patches too.
That wouldn't be right - what's there now is fine.
Okay, coming from a language that has double negatives so I'm never sure :-) Thanks for clarification. Michal
Chardevs don't have any other form of client authentication on top of the TLS transport, so the only way to authenticate clients is to verify their certificate. Enable this option by defauilt when both 'chardev_tls_x509_verify' and 'default_tls_x509_verify' were not configured. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- src/qemu/qemu_conf.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f40963ce48..8a1a50d664 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -258,7 +258,8 @@ # CA in the chardev_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either +# the default is "1". # #chardev_tls_x509_verify = 1 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c3a61816a4..e8bad33a40 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1253,7 +1253,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) } while (0) SET_TLS_VERIFY_DEFAULT(vnc, false); - SET_TLS_VERIFY_DEFAULT(chardev, false); + SET_TLS_VERIFY_DEFAULT(chardev, true); SET_TLS_VERIFY_DEFAULT(migrate, false); SET_TLS_VERIFY_DEFAULT(backup, false); -- 2.28.0
The migration stream connection and also the NBD server for non-shared storage migration don't have any other form of client authentication on top of the TLS transport, so the only way to authenticate clients is to verify their certificate. Enable this option by defauilt when both 'migrate_tls_x509_verify' and 'default_tls_x509_verify' were not configured. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- src/qemu/qemu_conf.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 8a1a50d664..d621dad53b 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -385,7 +385,8 @@ # CA in the migrate_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied +# either the default is "1". # #migrate_tls_x509_verify = 1 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e8bad33a40..6f74766607 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1254,7 +1254,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_VERIFY_DEFAULT(vnc, false); SET_TLS_VERIFY_DEFAULT(chardev, true); - SET_TLS_VERIFY_DEFAULT(migrate, false); + SET_TLS_VERIFY_DEFAULT(migrate, true); SET_TLS_VERIFY_DEFAULT(backup, false); #undef SET_TLS_VERIFY_DEFAULT -- 2.28.0
On 11/13/20 9:01 AM, Peter Krempa wrote:
The migration stream connection and also the NBD server for non-shared storage migration don't have any other form of client authentication on top of the TLS transport, so the only way to authenticate clients is to verify their certificate.
Enable this option by defauilt when both 'migrate_tls_x509_verify' and 'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- src/qemu/qemu_conf.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 8a1a50d664..d621dad53b 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -385,7 +385,8 @@ # CA in the migrate_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied +# either the default is "1".
s/either/either,/ Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The NBD server used to export pull-mode backups doesn't have any other form of client authentication on top of the TLS transport, so the only way to authenticate clients is to verify their certificate. Enable this option by defauilt when both 'backup_tls_x509_verify' and 'default_tls_x509_verify' were not configured. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- src/qemu/qemu_conf.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index d621dad53b..cc46a34ae2 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -422,7 +422,8 @@ # CA in the backup_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either +# the default is "1". # #backup_tls_x509_verify = 1 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6f74766607..8ae7c682cb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1255,7 +1255,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_VERIFY_DEFAULT(vnc, false); SET_TLS_VERIFY_DEFAULT(chardev, true); SET_TLS_VERIFY_DEFAULT(migrate, true); - SET_TLS_VERIFY_DEFAULT(backup, false); + SET_TLS_VERIFY_DEFAULT(backup, true); #undef SET_TLS_VERIFY_DEFAULT -- 2.28.0
On 11/13/20 9:01 AM, Peter Krempa wrote:
The NBD server used to export pull-mode backups doesn't have any other form of client authentication on top of the TLS transport, so the only way to authenticate clients is to verify their certificate.
Enable this option by defauilt when both 'backup_tls_x509_verify' and 'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 3 ++- src/qemu/qemu_conf.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index d621dad53b..cc46a34ae2 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -422,7 +422,8 @@ # CA in the backup_tls_x509_cert_dir (or default_tls_x509_cert_dir). # # If this option is not supplied, it will be set to the value of -# "default_tls_x509_verify". +# "default_tls_x509_verify". If "default_tls_x509_verify" is not supplied either +# the default is "1".
s/either/either,/ Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 3fd3ce4cb9..6fcfd4e26b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -11,6 +11,17 @@ For a more fine-grained view, use the `git log`_. v6.10.0 (unreleased) ==================== +* **Security** + + * qemu: Enable client TLS certificate validation by default for ``chardev``, + ``migration``, and ``backup`` servers. + + The default value if qemu.conf options ``chardev_tls_x509_verify``, + ``migrate_tls_x509_verify``, or ``backup_tls_x509_verify`` are not specified + explicitly in the config file and also the ``default_tls_x509_verify`` config + option is missing are now '1'. This ensures that only legitimate clients + access servers, which don't have any additional form of authentication. + * **New features** * hyperv: implement new APIs -- 2.28.0
On 11/13/20 4:01 PM, Peter Krempa wrote:
See patches 6 for a explanation.
Peter Krempa (6): qemu: conf: Allow individual control of default value for *_tls_x509_verify qemu: conf: Clarify default of "vnc_tls_x509_verify" qemu: conf: Enable 'chardev_tls_x509_verify' by default qemu: conf: Enable 'migrate_tls_x509_verify' by default qemu: conf: Enable 'backup_tls_x509_verify' by default NEWS: Mention change of default for TLS certificate verification
NEWS.rst | 11 +++++++++++ src/qemu/qemu.conf | 18 ++++++++++++++---- src/qemu/qemu_conf.c | 22 ++++++++++++++-------- src/qemu/qemu_conf.h | 1 + 4 files changed, 40 insertions(+), 12 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (5)
-
Daniel P. Berrangé -
Eric Blake -
Ján Tomko -
Michal Privoznik -
Peter Krempa