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 :|