[PATCH 0/9] tools: rewrite virt-pki-validate in C

Daniel P. Berrangé posted 9 patches 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240607142616.749339-1-berrange@redhat.com
docs/manpages/virt-pki-validate.rst |   9 +-
libvirt.spec.in                     |   2 -
po/POTFILES                         |   3 +
src/rpc/meson.build                 |   7 +-
src/rpc/virnettlscert.c             | 553 ++++++++++++++++++++++++++
src/rpc/virnettlscert.h             |  42 ++
src/rpc/virnettlsconfig.c           | 202 ++++++++++
src/rpc/virnettlsconfig.h           |  68 ++++
src/rpc/virnettlscontext.c          | 586 +---------------------------
tools/meson.build                   |  31 +-
tools/virt-host-validate-ch.c       |  12 +-
tools/virt-host-validate-common.c   | 308 ++++++---------
tools/virt-host-validate-common.h   |  48 +--
tools/virt-host-validate-lxc.c      |  18 +-
tools/virt-host-validate-qemu.c     |  30 +-
tools/virt-host-validate.c          |   2 +-
tools/virt-login-shell-helper.c     |   2 +-
tools/virt-pki-query-dn.c           |   2 +-
tools/virt-pki-validate.c           | 424 ++++++++++++++++++++
tools/virt-pki-validate.in          | 323 ---------------
tools/virt-validate-common.c        | 110 ++++++
tools/virt-validate-common.h        |  57 +++
22 files changed, 1670 insertions(+), 1169 deletions(-)
create mode 100644 src/rpc/virnettlscert.c
create mode 100644 src/rpc/virnettlscert.h
create mode 100644 src/rpc/virnettlsconfig.c
create mode 100644 src/rpc/virnettlsconfig.h
create mode 100644 tools/virt-pki-validate.c
delete mode 100644 tools/virt-pki-validate.in
create mode 100644 tools/virt-validate-common.c
create mode 100644 tools/virt-validate-common.h
[PATCH 0/9] tools: rewrite virt-pki-validate in C
Posted by Daniel P. Berrangé 3 months ago
This was driven by the complaint that libvirt pulls in gnutls-utils

  https://src.fedoraproject.org/rpms/virt-viewer/pull-request/4

but also it lets us remove more usage of Shell code from libvirt,
as well as improving the consistency of certificate checks vs the
runtime checks we do.

Daniel P. Berrangé (9):
  rpc: split out helpers for TLS cert path location
  rpc: refactor method for checking session certificates
  rpc: split TLS cert validation into separate file
  docs: fix author credit for virt-pki-validate tool
  tools: split off common helpers for host validate tool
  tools: drop unused --version argument
  tools: stop checking init scripts & iptables config
  tools: reimplement virt-pki-validate in C
  tools: support validating user/custom PKI certs

 docs/manpages/virt-pki-validate.rst |   9 +-
 libvirt.spec.in                     |   2 -
 po/POTFILES                         |   3 +
 src/rpc/meson.build                 |   7 +-
 src/rpc/virnettlscert.c             | 553 ++++++++++++++++++++++++++
 src/rpc/virnettlscert.h             |  42 ++
 src/rpc/virnettlsconfig.c           | 202 ++++++++++
 src/rpc/virnettlsconfig.h           |  68 ++++
 src/rpc/virnettlscontext.c          | 586 +---------------------------
 tools/meson.build                   |  31 +-
 tools/virt-host-validate-ch.c       |  12 +-
 tools/virt-host-validate-common.c   | 308 ++++++---------
 tools/virt-host-validate-common.h   |  48 +--
 tools/virt-host-validate-lxc.c      |  18 +-
 tools/virt-host-validate-qemu.c     |  30 +-
 tools/virt-host-validate.c          |   2 +-
 tools/virt-login-shell-helper.c     |   2 +-
 tools/virt-pki-query-dn.c           |   2 +-
 tools/virt-pki-validate.c           | 424 ++++++++++++++++++++
 tools/virt-pki-validate.in          | 323 ---------------
 tools/virt-validate-common.c        | 110 ++++++
 tools/virt-validate-common.h        |  57 +++
 22 files changed, 1670 insertions(+), 1169 deletions(-)
 create mode 100644 src/rpc/virnettlscert.c
 create mode 100644 src/rpc/virnettlscert.h
 create mode 100644 src/rpc/virnettlsconfig.c
 create mode 100644 src/rpc/virnettlsconfig.h
 create mode 100644 tools/virt-pki-validate.c
 delete mode 100644 tools/virt-pki-validate.in
 create mode 100644 tools/virt-validate-common.c
 create mode 100644 tools/virt-validate-common.h

-- 
2.43.0
Re: [PATCH 0/9] tools: rewrite virt-pki-validate in C
Posted by Michal Prívozník 2 months, 4 weeks ago
On 6/7/24 16:26, Daniel P. Berrangé wrote:
> This was driven by the complaint that libvirt pulls in gnutls-utils
> 
>   https://src.fedoraproject.org/rpms/virt-viewer/pull-request/4
> 
> but also it lets us remove more usage of Shell code from libvirt,
> as well as improving the consistency of certificate checks vs the
> runtime checks we do.
> 
> Daniel P. Berrangé (9):
>   rpc: split out helpers for TLS cert path location
>   rpc: refactor method for checking session certificates
>   rpc: split TLS cert validation into separate file
>   docs: fix author credit for virt-pki-validate tool
>   tools: split off common helpers for host validate tool
>   tools: drop unused --version argument
>   tools: stop checking init scripts & iptables config
>   tools: reimplement virt-pki-validate in C
>   tools: support validating user/custom PKI certs
> 
>  docs/manpages/virt-pki-validate.rst |   9 +-
>  libvirt.spec.in                     |   2 -
>  po/POTFILES                         |   3 +
>  src/rpc/meson.build                 |   7 +-
>  src/rpc/virnettlscert.c             | 553 ++++++++++++++++++++++++++
>  src/rpc/virnettlscert.h             |  42 ++
>  src/rpc/virnettlsconfig.c           | 202 ++++++++++
>  src/rpc/virnettlsconfig.h           |  68 ++++
>  src/rpc/virnettlscontext.c          | 586 +---------------------------
>  tools/meson.build                   |  31 +-
>  tools/virt-host-validate-ch.c       |  12 +-
>  tools/virt-host-validate-common.c   | 308 ++++++---------
>  tools/virt-host-validate-common.h   |  48 +--
>  tools/virt-host-validate-lxc.c      |  18 +-
>  tools/virt-host-validate-qemu.c     |  30 +-
>  tools/virt-host-validate.c          |   2 +-
>  tools/virt-login-shell-helper.c     |   2 +-
>  tools/virt-pki-query-dn.c           |   2 +-
>  tools/virt-pki-validate.c           | 424 ++++++++++++++++++++
>  tools/virt-pki-validate.in          | 323 ---------------
>  tools/virt-validate-common.c        | 110 ++++++
>  tools/virt-validate-common.h        |  57 +++
>  22 files changed, 1670 insertions(+), 1169 deletions(-)
>  create mode 100644 src/rpc/virnettlscert.c
>  create mode 100644 src/rpc/virnettlscert.h
>  create mode 100644 src/rpc/virnettlsconfig.c
>  create mode 100644 src/rpc/virnettlsconfig.h
>  create mode 100644 tools/virt-pki-validate.c
>  delete mode 100644 tools/virt-pki-validate.in
>  create mode 100644 tools/virt-validate-common.c
>  create mode 100644 tools/virt-validate-common.h
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
Re: [PATCH 0/9] tools: rewrite virt-pki-validate in C
Posted by Daniel P. Berrangé 2 months, 4 weeks ago
On Mon, Jun 10, 2024 at 01:57:17PM +0200, Michal Prívozník wrote:
> On 6/7/24 16:26, Daniel P. Berrangé wrote:
> > This was driven by the complaint that libvirt pulls in gnutls-utils
> > 
> >   https://src.fedoraproject.org/rpms/virt-viewer/pull-request/4
> > 
> > but also it lets us remove more usage of Shell code from libvirt,
> > as well as improving the consistency of certificate checks vs the
> > runtime checks we do.
> > 
> > Daniel P. Berrangé (9):
> >   rpc: split out helpers for TLS cert path location
> >   rpc: refactor method for checking session certificates
> >   rpc: split TLS cert validation into separate file
> >   docs: fix author credit for virt-pki-validate tool
> >   tools: split off common helpers for host validate tool
> >   tools: drop unused --version argument
> >   tools: stop checking init scripts & iptables config
> >   tools: reimplement virt-pki-validate in C
> >   tools: support validating user/custom PKI certs
> > 
> >  docs/manpages/virt-pki-validate.rst |   9 +-
> >  libvirt.spec.in                     |   2 -
> >  po/POTFILES                         |   3 +
> >  src/rpc/meson.build                 |   7 +-
> >  src/rpc/virnettlscert.c             | 553 ++++++++++++++++++++++++++
> >  src/rpc/virnettlscert.h             |  42 ++
> >  src/rpc/virnettlsconfig.c           | 202 ++++++++++
> >  src/rpc/virnettlsconfig.h           |  68 ++++
> >  src/rpc/virnettlscontext.c          | 586 +---------------------------
> >  tools/meson.build                   |  31 +-
> >  tools/virt-host-validate-ch.c       |  12 +-
> >  tools/virt-host-validate-common.c   | 308 ++++++---------
> >  tools/virt-host-validate-common.h   |  48 +--
> >  tools/virt-host-validate-lxc.c      |  18 +-
> >  tools/virt-host-validate-qemu.c     |  30 +-
> >  tools/virt-host-validate.c          |   2 +-
> >  tools/virt-login-shell-helper.c     |   2 +-
> >  tools/virt-pki-query-dn.c           |   2 +-
> >  tools/virt-pki-validate.c           | 424 ++++++++++++++++++++
> >  tools/virt-pki-validate.in          | 323 ---------------
> >  tools/virt-validate-common.c        | 110 ++++++
> >  tools/virt-validate-common.h        |  57 +++
> >  22 files changed, 1670 insertions(+), 1169 deletions(-)
> >  create mode 100644 src/rpc/virnettlscert.c
> >  create mode 100644 src/rpc/virnettlscert.h
> >  create mode 100644 src/rpc/virnettlsconfig.c
> >  create mode 100644 src/rpc/virnettlsconfig.h
> >  create mode 100644 tools/virt-pki-validate.c
> >  delete mode 100644 tools/virt-pki-validate.in
> >  create mode 100644 tools/virt-validate-common.c
> >  create mode 100644 tools/virt-validate-common.h
> > 
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Shoving this through CI highlighted that I forgot to test non-Linux
portability. Here are the resulting fixes, including your feedback,
that I'll be including before pushing, to get a clean build in CI:

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2570c2458a..9bff6ef6db 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2511,7 +2511,7 @@ exit 0
 %{mingw32_bindir}/virt-admin.exe
 %{mingw32_bindir}/virt-xml-validate
 %{mingw32_bindir}/virt-pki-query-dn.exe
-%{mingw32_bindir}/virt-pki-validate
+%{mingw32_bindir}/virt-pki-validate.exe
 %{mingw32_bindir}/libvirt-lxc-0.dll
 %{mingw32_bindir}/libvirt-qemu-0.dll
 %{mingw32_bindir}/libvirt-admin-0.dll
@@ -2570,7 +2570,7 @@ exit 0
 %{mingw64_bindir}/virt-admin.exe
 %{mingw64_bindir}/virt-xml-validate
 %{mingw64_bindir}/virt-pki-query-dn.exe
-%{mingw64_bindir}/virt-pki-validate
+%{mingw64_bindir}/virt-pki-validate.exe
 %{mingw64_bindir}/libvirt-lxc-0.dll
 %{mingw64_bindir}/libvirt-qemu-0.dll
 %{mingw64_bindir}/libvirt-admin-0.dll
diff --git a/src/rpc/meson.build b/src/rpc/meson.build
index 8bdbf5c88f..68aaf24b2a 100644
--- a/src/rpc/meson.build
+++ b/src/rpc/meson.build
@@ -1,9 +1,9 @@
 gendispatch_prog = find_program('gendispatch.pl')
 
-tlsconfig_sources = [
-    files('virnettlsconfig.c'),
-    files('virnettlscert.c'),
-]
+tlsconfig_sources = files(
+  'virnettlsconfig.c',
+  'virnettlscert.c',
+)
 
 socket_sources = tlsconfig_sources + [
   'virnettlscontext.c',
diff --git a/src/rpc/virnettlscert.c b/src/rpc/virnettlscert.c
index 2e1e4c56d5..1befbe06bc 100644
--- a/src/rpc/virnettlscert.c
+++ b/src/rpc/virnettlscert.c
@@ -20,6 +20,8 @@
 
 #include <config.h>
 
+#include <unistd.h>
+
 #include "virnettlscert.h"
 
 #include "viralloc.h"
diff --git a/tools/virt-host-validate-bhyve.c b/tools/virt-host-validate-bhyve.c
index adb5ae1717..d7a409db9d 100644
--- a/tools/virt-host-validate-bhyve.c
+++ b/tools/virt-host-validate-bhyve.c
@@ -28,21 +28,21 @@
 #include "virt-host-validate-common.h"
 
 #define MODULE_STATUS(mod, err_msg, err_code) \
-    virHostMsgCheck("BHYVE", _("Checking for %1$s module"), #mod); \
+    virValidateCheck("BHYVE", _("Checking for %1$s module"), #mod); \
     if (mod ## _loaded) { \
-        virHostMsgPass(); \
+        virValidatePass(); \
     } else { \
-        virHostMsgFail(err_code, \
-                       _("%1$s module is not loaded, " err_msg), \
+        virValidateFail(err_code, \
+                        _("%1$s module is not loaded, " err_msg), \
                         #mod); \
         ret = -1; \
     }
 
 #define MODULE_STATUS_FAIL(mod, err_msg) \
-    MODULE_STATUS(mod, err_msg, VIR_HOST_VALIDATE_FAIL)
+    MODULE_STATUS(mod, err_msg, VIR_VALIDATE_FAIL)
 
 #define MODULE_STATUS_WARN(mod, err_msg) \
-    MODULE_STATUS(mod, err_msg, VIR_HOST_VALIDATE_WARN)
+    MODULE_STATUS(mod, err_msg, VIR_VALIDATE_WARN)
 
 
 int virHostValidateBhyve(void)
diff --git a/tools/virt-validate-common.c b/tools/virt-validate-common.c
index 88c10e846f..9768fd9208 100644
--- a/tools/virt-validate-common.c
+++ b/tools/virt-validate-common.c
@@ -21,6 +21,8 @@
 
 #include <config.h>
 
+#include <unistd.h>
+
 #include "virt-validate-common.h"
 
 static bool quiet;


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