On Sun, Feb 09, 2020 at 10:03:14PM +0800, Zhang Bo wrote:
virAdmServerUpdateTlsFiles:
@flags specifies how to update server cert/key in tls service.
Two modes are currently supported: append mode and clear mode, means
whether to clear the original cert then add the new one, or just append
to the original one.
---
include/libvirt/libvirt-admin.h | 14 ++++++++++++++
src/admin/admin_server.c | 7 +------
src/admin/libvirt-admin.c | 7 ++++++-
src/rpc/virnetserver.c | 17 +++++++++++++----
src/rpc/virnetserver.h | 3 ++-
src/rpc/virnettlscontext.c | 7 +++++--
src/rpc/virnettlscontext.h | 3 ++-
7 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 6e38261129..dfdd81ae83 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,20 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int flags);
# define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"
+typedef enum {
+ /* free old credentials and then set new tls context.
+ */
+ VIR_TLS_UPDATE_CLEAR = 0,
+
+ /* do not clear original certificates and keys.
+ */
+ VIR_TLS_UPDATE_APPEND = 1,
I don't think we should we supporting this operation. In order to achieve
reliability of the TLS reload, we need to re-create the credentials object
and swap out the original credentails on success. This precludes updating
the original credentials....
@@ -1165,7 +1166,9 @@ int virNetTLSContextReload(virNetTLSContextPtr
ctxt,
}
if (filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT) {
- gnutls_certificate_free_keys(ctxt->x509cred);
+ if (flags == VIR_TLS_UPDATE_CLEAR)
+ gnutls_certificate_free_keys(ctxt->x509cred);
+
if (virNetTLSContextSetCertAndKey(ctxt, cert, key, false))
goto cleanup;
...because this code still has the dangerous behaviour that it leaves the
server without any valid cert/key on failure.
The second bad thing is that the state in-memory does not match the
state on disk. This puts the old & new cert+key in memory, but only
the new cert+key is on disk. So if libvirtd is restarted for any
reason you'll get a change in certs again. The reload operation
should just be updating libvirtd's in-memory state to exactly match
the on-disk state, in the same way that a restart does.
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 :|