[PATCH 0/6] ACL documentation fixes
This series fixes multiple problems with our ACL documentation. Results of the changes should be browsable once pipeline finishes: https://gitlab.com/pipo.sk/libvirt/-/jobs/3799680208 Peter Krempa (6): docs: Fix generated names for ACL objects docs: ACL: Mention the ACL object name along with the corresponding libvirt object name gendispatch: Add proper XML header to ACL permissions XML file docs/html: Properly generate ACL permissions into API reference docs: Distribute the XMLs with ACL permission flags for APIs examples: polkit: Grant 'domain.read-secure' for the example cases docs/html/meson.build | 32 ++++++++++++++++++++++++++++++- docs/meson.build | 27 ++++++++++++++++++++++++++ docs/newapi.xsl | 12 +++++------- examples/polkit/libvirt-acl.rules | 1 + libvirt.spec.in | 3 +++ scripts/genaclperms.py | 8 ++++---- src/access/meson.build | 13 ------------- src/rpc/gendispatch.pl | 14 ++++++++++++-- 8 files changed, 83 insertions(+), 27 deletions(-) -- 2.39.2
Both the object name and permission name in ACL use '-' instead of '_' separator when refering to them in the docs or even when used inside of polkig. Unfortunately the generators used for generating our docs don't honour this in certain cases which would result in broken names in the API docs (once they will be generated). Rename both object and permission name to use dash and reflect that in the anchor names in the documentation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/genaclperms.py | 6 +++--- src/rpc/gendispatch.pl | 13 +++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/genaclperms.py b/scripts/genaclperms.py index 43616dad04..eaf4a3d17d 100755 --- a/scripts/genaclperms.py +++ b/scripts/genaclperms.py @@ -88,7 +88,8 @@ print(' <body>') for obj in sorted(perms.keys()): klass = classes[obj] - olink = "object_" + obj.lower() + objname = obj.lower().replace("_", "-") + olink = "object_" + objname print(' <h3><a id="%s">%s</a></h3>' % (olink, klass)) print(' <table>') @@ -112,8 +113,7 @@ for obj in sorted(perms.keys()): if description is None: raise Exception("missing description for %s.%s" % (obj, perm)) - plink = "perm_" + obj.lower() + "_" + perm.lower() - plink = plink.replace("-", "_") + plink = "perm_" + objname + "_" + perm.lower() print(' <tr>') print(' <td><a id="%s">%s</a></td>' % (plink, perm)) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 085e2a29d8..c5f5c85811 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -2262,7 +2262,11 @@ elsif ($mode eq "client") { my $acl = $call->{acl}; foreach (@{$acl}) { my @bits = split /:/; - print " <check object='$bits[0]' perm='$bits[1]'"; + my $objname = $bits[0]; + $objname =~ s/_/-/g; + my $perm = $bits[1]; + $perm =~ s/_/-/g; + print " <check object='$objname' perm='$perm'"; if (defined $bits[2]) { print " flags='$bits[2]'"; } @@ -2272,7 +2276,12 @@ elsif ($mode eq "client") { my $aclfilter = $call->{aclfilter}; foreach (@{$aclfilter}) { my @bits = split /:/; - print " <filter object='$bits[0]' perm='$bits[1]'/>\n"; + my $objname = $bits[0]; + $objname =~ s/_/-/g; + my $perm = $bits[1]; + $perm =~ s/_/-/g; + + print " <filter object='$objname' perm='$perm'/>\n"; } print " </api>\n"; -- 2.39.2
On a Monday in 2023, Peter Krempa wrote:
Both the object name and permission name in ACL use '-' instead of '_' separator when refering to them in the docs or even when used inside of
referring
polkig. Unfortunately the generators used for generating our docs don't
polkit
honour this in certain cases which would result in broken names in the API docs (once they will be generated).
Rename both object and permission name to use dash and reflect that in the anchor names in the documentation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/genaclperms.py | 6 +++--- src/rpc/gendispatch.pl | 13 +++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
It's not trivial to figure out the ACL object name from our documentation. Add it above the table outlining existing permissions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/genaclperms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/genaclperms.py b/scripts/genaclperms.py index eaf4a3d17d..527005dd98 100755 --- a/scripts/genaclperms.py +++ b/scripts/genaclperms.py @@ -91,7 +91,7 @@ for obj in sorted(perms.keys()): objname = obj.lower().replace("_", "-") olink = "object_" + objname - print(' <h3><a id="%s">%s</a></h3>' % (olink, klass)) + print(' <h3><a id="%s"><code>%s</code> - %s</a></h3>' % (olink, objname, klass)) print(' <table>') print(' <thead>') print(' <tr>') -- 2.39.2
On Mon, Feb 20, 2023 at 11:47:05AM +0100, Peter Krempa wrote:
It's not trivial to figure out the ACL object name from our documentation. Add it above the table outlining existing permissions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/genaclperms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/rpc/gendispatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index c5f5c85811..4a50ac27e0 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -454,6 +454,7 @@ if ($mode eq "aclsym") { __EOF__ } elsif ($mode eq "aclapi") { print <<__EOF__; +<?xml version="1.0" encoding="UTF-8"?> <!-- - Automatically generated from $protocol by gendispatch.pl. - Do not edit this file. Any changes you make will be lost. -- 2.39.2
On Mon, Feb 20, 2023 at 11:47:06AM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/rpc/gendispatch.pl | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
The 'newapi.xsl' stylesheet was referencing non-existing paths to the XML files holding ACL permission flags for individual APIs. Additionally the 'document()' XSL function doesn't even allow concatenation of the path as it was done via '{$builddir}/src..', but requires either direct argument or use of the 'concat()' function. This meant that the 'acls' variable was always empty and thus none of our API documentation was actually generated with the 'acl' section. Fix it by passing the path to the XML via an argument to the stylesheet as the files differ based on which document is being generated. Since the 'admin' API does not have ACL we need to handle it separately now in the build system. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/html/meson.build | 32 +++++++++++++++++++++++++++++++- docs/newapi.xsl | 12 +++++------- src/access/meson.build | 4 ++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/docs/html/meson.build b/docs/html/meson.build index b18a8ccb5f..4e818f9142 100644 --- a/docs/html/meson.build +++ b/docs/html/meson.build @@ -27,12 +27,14 @@ index_api_gen = custom_target( command: [ xsltproc_prog, '--nonet', '-o', docs_builddir, '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', docs_acl_xml.full_path(), '--stringparam', 'timestamp', docs_timestamp, '--stringparam', 'indexfile', 'index.html', '@INPUT@', ], install: true, install_dir: docs_html_dir / 'html', + depends: docs_acl_xml, depend_files: [ page_xsl, ], @@ -41,7 +43,7 @@ index_api_gen = custom_target( docs_html_gen += index_api_gen.to_list() docs_html_dep += index_api_gen -foreach name : [ 'admin', 'lxc', 'qemu' ] +foreach name : [ 'lxc', 'qemu' ] index_api_gen = custom_target( 'index-@0@-api'.format(name), input: [ @@ -54,11 +56,13 @@ foreach name : [ 'admin', 'lxc', 'qemu' ] command: [ xsltproc_prog, '--nonet', '-o', docs_builddir, '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', get_variable('docs_acl_@0@_xml'.format(name)).full_path(), '--stringparam', 'timestamp', docs_timestamp, '@INPUT@', ], install: true, install_dir: docs_html_dir / 'html', + depends: get_variable('docs_acl_@0@_xml'.format(name)), depend_files: [ page_xsl, ], @@ -68,6 +72,32 @@ foreach name : [ 'admin', 'lxc', 'qemu' ] docs_html_dep += index_api_gen endforeach +index_api_gen = custom_target( + 'index-admin-api'.format(name), + input: [ + newapi_xsl, + docs_admin_api_xml, + ], + output: [ + 'libvirt-libvirt-admin.html' + ], + command: [ + xsltproc_prog, '--nonet', '-o', docs_builddir, + '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', '', + '--stringparam', 'timestamp', docs_timestamp, + '@INPUT@', + ], + install: true, + install_dir: docs_html_dir / 'html', + depend_files: [ + page_xsl, + ], + ) + +docs_html_gen += index_api_gen.to_list() +docs_html_dep += index_api_gen + docs_html_paths = [] install_web_deps += docs_html_dep diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 3de603bb00..cc3ec681b7 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -24,18 +24,16 @@ <xsl:param name="indexfile" select="''"/> + <xsl:param name="aclxmlpath" select="''"/> + <!-- the target directory for the HTML output --> <xsl:variable name="htmldir">html</xsl:variable> <xsl:variable name="href_base">../</xsl:variable> <xsl:variable name="acls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access.xml')/aclinfo/api"/> - </xsl:variable> - <xsl:variable name="qemuacls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access_qemu.xml')/aclinfo/api"/> - </xsl:variable> - <xsl:variable name="lxcacls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access_lxc.xml')/aclinfo/api"/> + <xsl:if test="$aclxmlpath != ''"> + <xsl:copy-of select="document($aclxmlpath)/aclinfo/api"/> + </xsl:if> </xsl:variable> <xsl:template name="aclinfo"> diff --git a/src/access/meson.build b/src/access/meson.build index 07fd7d372e..0b12581dc1 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -74,6 +74,10 @@ foreach name : [ 'remote', 'qemu', 'lxc' ] ) endforeach +docs_acl_xml = access_gen_xml[0] +docs_acl_qemu_xml = access_gen_xml[1] +docs_acl_lxc_xml = access_gen_xml[2] + if conf.has('WITH_POLKIT') access_sources += access_polkit_sources -- 2.39.2
On Mon, Feb 20, 2023 at 11:47:07AM +0100, Peter Krempa wrote:
The 'newapi.xsl' stylesheet was referencing non-existing paths to the XML files holding ACL permission flags for individual APIs. Additionally the 'document()' XSL function doesn't even allow concatenation of the path as it was done via '{$builddir}/src..', but requires either direct argument or use of the 'concat()' function.
This meant that the 'acls' variable was always empty and thus none of our API documentation was actually generated with the 'acl' section.
Fix it by passing the path to the XML via an argument to the stylesheet as the files differ based on which document is being generated.
Since the 'admin' API does not have ACL we need to handle it separately now in the build system.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/html/meson.build | 32 +++++++++++++++++++++++++++++++- docs/newapi.xsl | 12 +++++------- src/access/meson.build | 4 ++++ 3 files changed, 40 insertions(+), 8 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/docs/html/meson.build b/docs/html/meson.build index b18a8ccb5f..4e818f9142 100644 --- a/docs/html/meson.build +++ b/docs/html/meson.build @@ -27,12 +27,14 @@ index_api_gen = custom_target( command: [ xsltproc_prog, '--nonet', '-o', docs_builddir, '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', docs_acl_xml.full_path(), '--stringparam', 'timestamp', docs_timestamp, '--stringparam', 'indexfile', 'index.html', '@INPUT@', ], install: true, install_dir: docs_html_dir / 'html', + depends: docs_acl_xml, depend_files: [ page_xsl, ], @@ -41,7 +43,7 @@ index_api_gen = custom_target( docs_html_gen += index_api_gen.to_list() docs_html_dep += index_api_gen
-foreach name : [ 'admin', 'lxc', 'qemu' ] +foreach name : [ 'lxc', 'qemu' ] index_api_gen = custom_target( 'index-@0@-api'.format(name), input: [ @@ -54,11 +56,13 @@ foreach name : [ 'admin', 'lxc', 'qemu' ] command: [ xsltproc_prog, '--nonet', '-o', docs_builddir, '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', get_variable('docs_acl_@0@_xml'.format(name)).full_path(), '--stringparam', 'timestamp', docs_timestamp, '@INPUT@', ], install: true, install_dir: docs_html_dir / 'html', + depends: get_variable('docs_acl_@0@_xml'.format(name)), depend_files: [ page_xsl, ], @@ -68,6 +72,32 @@ foreach name : [ 'admin', 'lxc', 'qemu' ] docs_html_dep += index_api_gen endforeach
+index_api_gen = custom_target( + 'index-admin-api'.format(name), + input: [ + newapi_xsl, + docs_admin_api_xml, + ], + output: [ + 'libvirt-libvirt-admin.html' + ], + command: [ + xsltproc_prog, '--nonet', '-o', docs_builddir, + '--stringparam', 'builddir', meson.project_build_root(), + '--stringparam', 'aclxmlpath', '', + '--stringparam', 'timestamp', docs_timestamp, + '@INPUT@', + ], + install: true, + install_dir: docs_html_dir / 'html', + depend_files: [ + page_xsl, + ], + ) + +docs_html_gen += index_api_gen.to_list() +docs_html_dep += index_api_gen + docs_html_paths = []
install_web_deps += docs_html_dep diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 3de603bb00..cc3ec681b7 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -24,18 +24,16 @@
<xsl:param name="indexfile" select="''"/>
+ <xsl:param name="aclxmlpath" select="''"/> + <!-- the target directory for the HTML output --> <xsl:variable name="htmldir">html</xsl:variable> <xsl:variable name="href_base">../</xsl:variable>
<xsl:variable name="acls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access.xml')/aclinfo/api"/> - </xsl:variable> - <xsl:variable name="qemuacls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access_qemu.xml')/aclinfo/api"/> - </xsl:variable> - <xsl:variable name="lxcacls"> - <xsl:copy-of select="document('{$builddir}/src/libvirt_access_lxc.xml')/aclinfo/api"/> + <xsl:if test="$aclxmlpath != ''"> + <xsl:copy-of select="document($aclxmlpath)/aclinfo/api"/> + </xsl:if> </xsl:variable>
<xsl:template name="aclinfo"> diff --git a/src/access/meson.build b/src/access/meson.build index 07fd7d372e..0b12581dc1 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -74,6 +74,10 @@ foreach name : [ 'remote', 'qemu', 'lxc' ] ) endforeach
+docs_acl_xml = access_gen_xml[0] +docs_acl_qemu_xml = access_gen_xml[1] +docs_acl_lxc_xml = access_gen_xml[2] + if conf.has('WITH_POLKIT') access_sources += access_polkit_sources
-- 2.39.2
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 :|
Similarly to the API XML we can distribute the ACL permissions for the APIs so that users who are potentially interested into the data don't have to scrape our web. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/meson.build | 27 +++++++++++++++++++++++++++ libvirt.spec.in | 3 +++ src/access/meson.build | 17 ----------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/docs/meson.build b/docs/meson.build index a90c59866a..08bf75e329 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -168,6 +168,33 @@ docs_lxc_api_xml = docs_api_generated[1] docs_qemu_api_xml = docs_api_generated[2] docs_admin_api_xml = docs_api_generated[3] +access_gen_xml = [] + +foreach name : [ 'remote', 'qemu', 'lxc' ] + if name == 'remote' + xml_file = 'libvirt_access.xml' + else + xml_file = 'libvirt_access_@0@.xml'.format(name) + endif + protocol_file = remote_path / '@0@_protocol.x'.format(name) + + access_gen_xml += custom_target( + xml_file, + input: protocol_file, + output: xml_file, + capture: true, + command: [ + gendispatch_prog, '--mode=aclapi', name, name.to_upper(), '@INPUT@', + ], + install: true, + install_dir: pkgdatadir / 'api', + ) +endforeach + +docs_acl_xml = access_gen_xml[0] +docs_acl_qemu_xml = access_gen_xml[1] +docs_acl_lxc_xml = access_gen_xml[2] + docs_programs_groups = [ { 'name': 'rst2html5', 'prog': [ 'rst2html5', 'rst2html5.py', 'rst2html5-3' ] }, { 'name': 'rst2man', 'prog': [ 'rst2man', 'rst2man.py', 'rst2man-3' ] }, diff --git a/libvirt.spec.in b/libvirt.spec.in index e795b98d48..df625a0db4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2333,6 +2333,9 @@ exit 0 %{_datadir}/libvirt/api/libvirt-admin-api.xml %{_datadir}/libvirt/api/libvirt-qemu-api.xml %{_datadir}/libvirt/api/libvirt-lxc-api.xml +%{_datadir}/libvirt/api/libvirt_access.xml +%{_datadir}/libvirt/api/libvirt_access_qemu.xml +%{_datadir}/libvirt/api/libvirt_access_lxc.xml %if %{with_mingw} %files -n mingw32-libvirt -f mingw32-libvirt.lang diff --git a/src/access/meson.build b/src/access/meson.build index 0b12581dc1..842f37a4b6 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -16,19 +16,16 @@ remote_path = meson.project_source_root() / 'src' / 'remote' access_gen_headers = [] access_gen_sources = [] access_gen_sym = [] -access_gen_xml = [] foreach name : [ 'remote', 'qemu', 'lxc' ] if name == 'remote' header_file = 'viraccessapicheck.h' source_file = 'viraccessapicheck.c' syms_file = 'libvirt_access.syms' - xml_file = 'libvirt_access.xml' else header_file = 'viraccessapicheck@0@.h'.format(name) source_file = 'viraccessapicheck@0@.c'.format(name) syms_file = 'libvirt_access_@0@.syms'.format(name) - xml_file = 'libvirt_access_@0@.xml'.format(name) endif protocol_file = remote_path / '@0@_protocol.x'.format(name) @@ -62,22 +59,8 @@ foreach name : [ 'remote', 'qemu', 'lxc' ] gendispatch_prog, '--mode=aclsym', name, name.to_upper(), '@INPUT@', ] ) - - access_gen_xml += custom_target( - xml_file, - input: protocol_file, - output: xml_file, - capture: true, - command: [ - gendispatch_prog, '--mode=aclapi', name, name.to_upper(), '@INPUT@', - ], - ) endforeach -docs_acl_xml = access_gen_xml[0] -docs_acl_qemu_xml = access_gen_xml[1] -docs_acl_lxc_xml = access_gen_xml[2] - if conf.has('WITH_POLKIT') access_sources += access_polkit_sources -- 2.39.2
On Mon, Feb 20, 2023 at 11:47:08AM +0100, Peter Krempa wrote:
Similarly to the API XML we can distribute the ACL permissions for the APIs so that users who are potentially interested into the data don't have to scrape our web.
IMHO if we want to expose this to apps, we should be including the info directly in the API files we already ship, rather than exporting something new.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/meson.build | 27 +++++++++++++++++++++++++++ libvirt.spec.in | 3 +++ src/access/meson.build | 17 ----------------- 3 files changed, 30 insertions(+), 17 deletions(-)
Note the current API XMLs are reported by 'pkg-config' but these new XML files are not reported, so this change is not complete in that regard
diff --git a/docs/meson.build b/docs/meson.build index a90c59866a..08bf75e329 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -168,6 +168,33 @@ docs_lxc_api_xml = docs_api_generated[1] docs_qemu_api_xml = docs_api_generated[2] docs_admin_api_xml = docs_api_generated[3]
+access_gen_xml = [] + +foreach name : [ 'remote', 'qemu', 'lxc' ] + if name == 'remote' + xml_file = 'libvirt_access.xml' + else + xml_file = 'libvirt_access_@0@.xml'.format(name) + endif + protocol_file = remote_path / '@0@_protocol.x'.format(name) + + access_gen_xml += custom_target( + xml_file, + input: protocol_file, + output: xml_file, + capture: true, + command: [ + gendispatch_prog, '--mode=aclapi', name, name.to_upper(), '@INPUT@', + ], + install: true, + install_dir: pkgdatadir / 'api', + ) +endforeach + +docs_acl_xml = access_gen_xml[0] +docs_acl_qemu_xml = access_gen_xml[1] +docs_acl_lxc_xml = access_gen_xml[2] + docs_programs_groups = [ { 'name': 'rst2html5', 'prog': [ 'rst2html5', 'rst2html5.py', 'rst2html5-3' ] }, { 'name': 'rst2man', 'prog': [ 'rst2man', 'rst2man.py', 'rst2man-3' ] }, diff --git a/libvirt.spec.in b/libvirt.spec.in index e795b98d48..df625a0db4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2333,6 +2333,9 @@ exit 0 %{_datadir}/libvirt/api/libvirt-admin-api.xml %{_datadir}/libvirt/api/libvirt-qemu-api.xml %{_datadir}/libvirt/api/libvirt-lxc-api.xml +%{_datadir}/libvirt/api/libvirt_access.xml +%{_datadir}/libvirt/api/libvirt_access_qemu.xml +%{_datadir}/libvirt/api/libvirt_access_lxc.xml
%if %{with_mingw} %files -n mingw32-libvirt -f mingw32-libvirt.lang diff --git a/src/access/meson.build b/src/access/meson.build index 0b12581dc1..842f37a4b6 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -16,19 +16,16 @@ remote_path = meson.project_source_root() / 'src' / 'remote' access_gen_headers = [] access_gen_sources = [] access_gen_sym = [] -access_gen_xml = []
foreach name : [ 'remote', 'qemu', 'lxc' ] if name == 'remote' header_file = 'viraccessapicheck.h' source_file = 'viraccessapicheck.c' syms_file = 'libvirt_access.syms' - xml_file = 'libvirt_access.xml' else header_file = 'viraccessapicheck@0@.h'.format(name) source_file = 'viraccessapicheck@0@.c'.format(name) syms_file = 'libvirt_access_@0@.syms'.format(name) - xml_file = 'libvirt_access_@0@.xml'.format(name) endif protocol_file = remote_path / '@0@_protocol.x'.format(name)
@@ -62,22 +59,8 @@ foreach name : [ 'remote', 'qemu', 'lxc' ] gendispatch_prog, '--mode=aclsym', name, name.to_upper(), '@INPUT@', ] ) - - access_gen_xml += custom_target( - xml_file, - input: protocol_file, - output: xml_file, - capture: true, - command: [ - gendispatch_prog, '--mode=aclapi', name, name.to_upper(), '@INPUT@', - ], - ) endforeach
-docs_acl_xml = access_gen_xml[0] -docs_acl_qemu_xml = access_gen_xml[1] -docs_acl_lxc_xml = access_gen_xml[2] - if conf.has('WITH_POLKIT') access_sources += access_polkit_sources
-- 2.39.2
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 :|
On Mon, Feb 20, 2023 at 17:05:31 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 11:47:08AM +0100, Peter Krempa wrote:
Similarly to the API XML we can distribute the ACL permissions for the APIs so that users who are potentially interested into the data don't have to scrape our web.
IMHO if we want to expose this to apps, we should be including the info directly in the API files we already ship, rather than exporting something new.
Yeah, I thought the same but didn't really fancy changing the generator as they are partially generated by gendispatch rather than the ACL generator as the information is in the protocol headers rather than with the function headers. I can drop this patch for now as the series works well without.
On Mon, Feb 20, 2023 at 06:11:41PM +0100, Peter Krempa wrote:
On Mon, Feb 20, 2023 at 17:05:31 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 11:47:08AM +0100, Peter Krempa wrote:
Similarly to the API XML we can distribute the ACL permissions for the APIs so that users who are potentially interested into the data don't have to scrape our web.
IMHO if we want to expose this to apps, we should be including the info directly in the API files we already ship, rather than exporting something new.
Yeah, I thought the same but didn't really fancy changing the generator as they are partially generated by gendispatch rather than the ACL generator as the information is in the protocol headers rather than with the function headers.
Rather than change the python generator, it would be possible to use an XSL transform to merge the ACL XML info into the main API info doc. Whether that's better or not of course depends how much you enjoy working with XSL :-) 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 :|
On Mon, Feb 20, 2023 at 17:13:24 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 06:11:41PM +0100, Peter Krempa wrote:
On Mon, Feb 20, 2023 at 17:05:31 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 11:47:08AM +0100, Peter Krempa wrote:
Similarly to the API XML we can distribute the ACL permissions for the APIs so that users who are potentially interested into the data don't have to scrape our web.
IMHO if we want to expose this to apps, we should be including the info directly in the API files we already ship, rather than exporting something new.
Yeah, I thought the same but didn't really fancy changing the generator as they are partially generated by gendispatch rather than the ACL generator as the information is in the protocol headers rather than with the function headers.
Rather than change the python generator, it would be possible to use an XSL transform to merge the ACL XML info into the main API info doc. Whether that's better or not of course depends how much you enjoy working with XSL :-)
Hmm yeah, I thought about that one too. But I wanted to avoid having an extra step. I do prefer XSL compared to trying to understand what gendispatch does ;)
The example gives the user authorized to work with the domain permission to open the graphics socket. Since the graphics socket may be protected with a password it makes sense to grant the user the 'domain.read-secure' permission to fetch the password for the graphics object. This also goes along with e.g. 'domain.send-input' and 'domain.screenshot' as they'll allow the user to interact with the domain even if they didn't have the password. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/polkit/libvirt-acl.rules | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/polkit/libvirt-acl.rules b/examples/polkit/libvirt-acl.rules index dd6836599a..2edd9c5b8e 100644 --- a/examples/polkit/libvirt-acl.rules +++ b/examples/polkit/libvirt-acl.rules @@ -93,6 +93,7 @@ restrictedActions = [ "domain.inject-nmi", "domain.open-device", "domain.open-graphics", + "domain.read-secure", "domain.pm-control", "domain.read", "domain.reset", -- 2.39.2
On Mon, Feb 20, 2023 at 11:47:09AM +0100, Peter Krempa wrote:
The example gives the user authorized to work with the domain permission to open the graphics socket. Since the graphics socket may be protected with a password it makes sense to grant the user the 'domain.read-secure' permission to fetch the password for the graphics object.
This also goes along with e.g. 'domain.send-input' and 'domain.screenshot' as they'll allow the user to interact with the domain even if they didn't have the password.
The password isn't required, as you can use virDomainOpenGraphics to connect when its a local display, and that's allowed via the domain.open-graphics permission. virt-viewer at least will use this API, but can't remember in virt-manager will. This also bypasses any need to configure TLS certificates for VNC, or do Kerberos auth if that's enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/polkit/libvirt-acl.rules | 1 + 1 file changed, 1 insertion(+)
diff --git a/examples/polkit/libvirt-acl.rules b/examples/polkit/libvirt-acl.rules index dd6836599a..2edd9c5b8e 100644 --- a/examples/polkit/libvirt-acl.rules +++ b/examples/polkit/libvirt-acl.rules @@ -93,6 +93,7 @@ restrictedActions = [ "domain.inject-nmi", "domain.open-device", "domain.open-graphics", + "domain.read-secure",
We don't allow the secret.read-secure parameter, and I don't think we should allow this either.
"domain.pm-control", "domain.read", "domain.reset", -- 2.39.2
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 :|
On Mon, Feb 20, 2023 at 17:09:18 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 11:47:09AM +0100, Peter Krempa wrote:
The example gives the user authorized to work with the domain permission to open the graphics socket. Since the graphics socket may be protected with a password it makes sense to grant the user the 'domain.read-secure' permission to fetch the password for the graphics object.
This also goes along with e.g. 'domain.send-input' and 'domain.screenshot' as they'll allow the user to interact with the domain even if they didn't have the password.
The password isn't required, as you can use virDomainOpenGraphics to connect when its a local display, and that's allowed via the domain.open-graphics permission. virt-viewer at least will use
So in such case authentication is not needed? e.g. if you setup a password regardles of that?
On Mon, Feb 20, 2023 at 06:12:53PM +0100, Peter Krempa wrote:
On Mon, Feb 20, 2023 at 17:09:18 +0000, Daniel P. Berrangé wrote:
On Mon, Feb 20, 2023 at 11:47:09AM +0100, Peter Krempa wrote:
The example gives the user authorized to work with the domain permission to open the graphics socket. Since the graphics socket may be protected with a password it makes sense to grant the user the 'domain.read-secure' permission to fetch the password for the graphics object.
This also goes along with e.g. 'domain.send-input' and 'domain.screenshot' as they'll allow the user to interact with the domain even if they didn't have the password.
The password isn't required, as you can use virDomainOpenGraphics to connect when its a local display, and that's allowed via the domain.open-graphics permission. virt-viewer at least will use
So in such case authentication is not needed? e.g. if you setup a password regardles of that?
Yes, if VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH is set as a flag. 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 :|
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa