[PATCH 0/6] Generate ACL permision data in apibuild

Peter Krempa (6): remote_protocol: Fix list of supported ACL object names apibuild: Add infrastructure for generating ACL flag info into function docs scripts/apibuild: Extract and format API ACLs docs/newapi.xsl: Take API flag data from libvirt-api.xml rather than access/libvirt-access.xml docs|access: Don't build the ACL flags into a separate XML gendispatch: Drop 'aclapi' mode docs/html/meson.build | 32 +------ docs/meson.build | 3 + docs/newapi.xsl | 20 ++--- scripts/apibuild.py | 163 ++++++++++++++++++++++++++++++++++- src/access/meson.build | 16 ---- src/remote/remote_protocol.x | 5 +- src/rpc/gendispatch.pl | 64 +------------- 7 files changed, 176 insertions(+), 127 deletions(-) -- 2.39.2

Add missing and fix spelling of existing ones. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote/remote_protocol.x | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c34d6f189d..5d86a51116 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3974,8 +3974,9 @@ enum remote_procedure { * Declare the access control requirements for the API. May be repeated * multiple times, if multiple rules are required. * - * <object> is one of 'connect', 'domain', 'network', 'storagepool', - * 'interface', 'nodedev', 'secret'. + * <object> is one of 'connect', 'domain', 'interface', 'network', + * 'network_port', 'node_device', 'nwfilter', + * 'nwfilter_binding', 'secret', 'storage_pool', 'storage_vol' * <permission> is one of the permissions in access/viraccessperm.h * <flagname> indicates the rule only applies if the named flag * is set in the API call -- 2.39.2

If the user of the 'docBuilder' class provides a dict (key is API name, value is a tuple of arrays (acls, aclfilters), use the dict to generate ACL definitions into the function definition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/apibuild.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 9614709e6c..cced9a5551 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -2071,10 +2071,11 @@ class CParser: class docBuilder: """A documentation builder""" - def __init__(self, name, syms, path='.', directories=['.'], includes=[]): + def __init__(self, name, syms, path='.', directories=['.'], includes=[], acls=None): self.name = name self.syms = syms self.path = path + self.acls = acls self.directories = directories if name == "libvirt": self.includes = includes + list(included_files.keys()) @@ -2477,6 +2478,32 @@ class docBuilder: except Exception: print("Exception:", sys.exc_info()[1], file=sys.stderr) self.warning("Failed to save function %s info: %s" % (name, repr(id.info))) + + if self.acls and name in self.acls: + acls = self.acls[name][0] + aclfilters = self.acls[name][1] + + if len(acls) > 0 or len(aclfilters) > 0: + output.write(" <acls>\n") + for acl in acls: + comp = acl.split(':', 3) + objname = comp[0].replace('_', '-') + perm = comp[1].replace('_', '-') + output.write(" <check object='%s' perm='%s'" % (objname, perm)) + if len(comp) > 2: + output.write(" flags='%s'" % comp[2]) + + output.write("/>\n") + + for aclfilter in aclfilters: + comp = aclfilter.split(':', 2) + objname = comp[0].replace('_', '-') + perm = comp[1].replace('_', '-') + + output.write(" <filter object='%s' perm='%s'/>\n" % (objname, perm)) + + output.write(" </acls>\n") + output.write(" </%s>\n" % (id.type)) def serialize_exports(self, output, file): -- 2.39.2

As an additional step before processing the API parse the protocol file and extract all ACL definitions. This way we can distribute them for any user of the libvirt API XML files. We will be also able to avoid another call to gendispatch, which generates all this data into a standalone XML. The remote procedure to API name is inspired by what rpcgen does. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/meson.build | 3 + scripts/apibuild.py | 134 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/docs/meson.build b/docs/meson.build index 89ac93a958..864abf0ba5 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -160,6 +160,9 @@ docs_api_generated = custom_target( libvirt_lxc_sources, admin_sources, util_public_sources, + meson.project_source_root() / 'src' / 'remote' / 'remote_protocol.x', + meson.project_source_root() / 'src' / 'remote' / 'qemu_protocol.x', + meson.project_source_root() / 'src' / 'remote' / 'lxc_protocol.x', ], ) diff --git a/scripts/apibuild.py b/scripts/apibuild.py index cced9a5551..f532dbe834 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -2588,6 +2588,125 @@ class docBuilder: sys.exit(3) +def remoteProcToAPI(remotename: str) -> (str): + components = remotename.split('_') + fixednames = [] + + if components[1] != "PROC": + raise Exception("Malformed remote function name '%s'" % remotename) + + if components[0] == 'REMOTE': + driver = '' + elif components[0] == 'QEMU': + driver = 'Qemu' + elif components[0] == 'LXC': + driver = 'Lxc' + else: + raise Exception("Unknown remote protocol '%s'" % components[0]) + + for comp in components[2:]: + if comp == '': + raise Exception("Invalid empty component in remote procedure name '%s'" % remotename) + + fixedname = comp[0].upper() + comp[1:].lower() + + fixedname = re.sub('Nwfilter', 'NWFilter', fixedname) + fixedname = re.sub('Xml$', 'XML', fixedname) + fixedname = re.sub('Xml2$', 'XML2', fixedname) + fixedname = re.sub('Uri$', 'URI', fixedname) + fixedname = re.sub('Uuid$', 'UUID', fixedname) + fixedname = re.sub('Id$', 'ID', fixedname) + fixedname = re.sub('Mac$', 'MAC', fixedname) + fixedname = re.sub('Cpu$', 'CPU', fixedname) + fixedname = re.sub('Os$', 'OS', fixedname) + fixedname = re.sub('Nmi$', 'NMI', fixedname) + fixedname = re.sub('Pm', 'PM', fixedname) + fixedname = re.sub('Fstrim$', 'FSTrim', fixedname) + fixedname = re.sub('Fsfreeze$', 'FSFreeze', fixedname) + fixedname = re.sub('Fsthaw$', 'FSThaw', fixedname) + fixedname = re.sub('Fsinfo$', 'FSInfo', fixedname) + fixedname = re.sub('Iothread$', 'IOThread', fixedname) + fixedname = re.sub('Scsi', 'SCSI', fixedname) + fixedname = re.sub('Wwn$', 'WWN', fixedname) + fixedname = re.sub('Dhcp$', 'DHCP', fixedname) + + fixednames.append(fixedname) + + apiname = "vir" + fixednames[0] + + # In case of remote procedures for qemu/lxc private APIs we need to add + # the name of the driver in the middle of the string after the object name. + # For a special case of event callbacks the 'object' name is actually two + # words: virConenctDomainQemuEvent ... + if fixednames[1] == 'Domain': + apiname += 'Domain' + fixednames.pop(1) + + apiname += driver + + for name in fixednames[1:]: + apiname = apiname + name + + return apiname + + +def remoteProtocolGetAcls(protocolfilename: str) -> {}: + apiacls = {} + + with open(protocolfilename) as proto: + in_procedures = False + acls = [] + aclfilters = [] + + while True: + line = proto.readline() + if not line: + break + + if not in_procedures: + if re.match('^enum [a-z]+_procedure {$', line): + in_procedures = True + + continue + + if line == '};\n': + break + + acl_match = re.search(r"\* @acl: ([^\s]+)", line) + + if acl_match: + acls.append(acl_match.group(1)) + continue + + aclfilter_match = re.search(r"\* @aclfilter: ([^\s]+)", line) + + if aclfilter_match: + aclfilters.append(aclfilter_match.group(1)) + continue + + remote_proc_match = re.search(r"^\s+([A-Z_0-9]+) ", line) + + if remote_proc_match: + proc = remote_proc_match.group(1) + apiname = remoteProcToAPI(proc) + + if len(acls) == 0: + raise Exception("No ACLs for procedure %s(%s)" % proc, apiname) + + if 'none' in acls: + if len(acls) > 1: + raise Exception("Procedure %s(%s) has 'none' ACL followed by other ACLs" % proc, apiname) + + acls = [] + + apiacls[apiname] = (acls, aclfilters) + acls = [] + aclfilters = [] + continue + + return apiacls + + class app: def warning(self, msg): global warnings @@ -2595,16 +2714,27 @@ class app: print(msg) def rebuild(self, name, srcdir, builddir): + apiacl = None + syms = { "libvirt": srcdir + "/../src/libvirt_public.syms", "libvirt-qemu": srcdir + "/../src/libvirt_qemu.syms", "libvirt-lxc": srcdir + "/../src/libvirt_lxc.syms", "libvirt-admin": srcdir + "/../src/admin/libvirt_admin_public.syms", } - if name not in syms: + protocols = { + "libvirt": srcdir + "/../src/remote/remote_protocol.x", + "libvirt-qemu": srcdir + "/../src/remote/qemu_protocol.x", + "libvirt-lxc": srcdir + "/../src/remote/lxc_protocol.x", + "libvirt-admin": None, + } + if name not in syms or name not in protocols: self.warning("rebuild() failed, unknown module %s" % name) return None + if protocols[name]: + apiacl = remoteProtocolGetAcls(protocols[name]) + builder = None if glob.glob(srcdir + "/../src/libvirt.c") != []: if not quiet: @@ -2614,7 +2744,7 @@ class app: srcdir + "/../src/util", srcdir + "/../include/libvirt", builddir + "/../include/libvirt"] - builder = docBuilder(name, syms[name], builddir, dirs, []) + builder = docBuilder(name, syms[name], builddir, dirs, [], apiacl) else: self.warning("rebuild() failed, unable to guess the module") return None -- 2.39.2

Since now we embed the data in the libvirt API we don't need to source it from the extra document. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/newapi.xsl | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/docs/newapi.xsl b/docs/newapi.xsl index cc3ec681b7..cc08b8460a 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -24,22 +24,14 @@ <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:if test="$aclxmlpath != ''"> - <xsl:copy-of select="document($aclxmlpath)/aclinfo/api"/> - </xsl:if> - </xsl:variable> - <xsl:template name="aclinfo"> - <xsl:param name="api"/> + <xsl:param name="acl"/> - <xsl:if test="count(exsl:node-set($acls)/api[@name=$api]/check) > 0"> + <xsl:if test="count($acl/check) > 0"> <h5>Access control parameter checks</h5> <table> <thead> @@ -49,10 +41,10 @@ <th>Condition</th> </tr> </thead> - <xsl:apply-templates select="exsl:node-set($acls)/api[@name=$api]/check" mode="acl"/> + <xsl:apply-templates select="$acl/check" mode="acl"/> </table> </xsl:if> - <xsl:if test="count(exsl:node-set($acls)/api[@name=$api]/filter) > 0"> + <xsl:if test="count($acl/filter) > 0"> <h5>Access control return value filters</h5> <table> <thead> @@ -61,7 +53,7 @@ <th>Permission</th> </tr> </thead> - <xsl:apply-templates select="exsl:node-set($acls)/api[@name=$api]/filter" mode="acl"/> + <xsl:apply-templates select="$acl/filter" mode="acl"/> </table> </xsl:if> </xsl:template> @@ -692,7 +684,7 @@ </xsl:if> <div class="acl"> <xsl:call-template name="aclinfo"> - <xsl:with-param name="api" select="$name"/> + <xsl:with-param name="acl" select="acls"/> </xsl:call-template> </div> </xsl:template> -- 2.39.2

Since we now build it into the libvirt-api.xml or equivalents we don't need the extra XML files. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/html/meson.build | 32 +------------------------------- src/access/meson.build | 16 ---------------- 2 files changed, 1 insertion(+), 47 deletions(-) diff --git a/docs/html/meson.build b/docs/html/meson.build index 4e818f9142..b18a8ccb5f 100644 --- a/docs/html/meson.build +++ b/docs/html/meson.build @@ -27,14 +27,12 @@ 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, ], @@ -43,7 +41,7 @@ index_api_gen = custom_target( docs_html_gen += index_api_gen.to_list() docs_html_dep += index_api_gen -foreach name : [ 'lxc', 'qemu' ] +foreach name : [ 'admin', 'lxc', 'qemu' ] index_api_gen = custom_target( 'index-@0@-api'.format(name), input: [ @@ -56,13 +54,11 @@ foreach name : [ '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, ], @@ -72,32 +68,6 @@ foreach name : [ '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/src/access/meson.build b/src/access/meson.build index 0b12581dc1..07c703e8b5 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -23,12 +23,10 @@ foreach name : [ 'remote', 'qemu', 'lxc' ] 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 +60,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

The separate API perms XML is no longer used. Remove the support for generating it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/rpc/gendispatch.pl | 64 ++---------------------------------------- 1 file changed, 3 insertions(+), 61 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 4a50ac27e0..b186849606 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -42,8 +42,8 @@ my $res = GetOptions("mode=s" => \$mode); die "cannot parse command line options" unless $res; die "unknown mode '$mode', expecting 'client', 'server', " . - "'aclheader', 'aclbody', 'aclsym', 'aclapi' or 'debug'" - unless $mode =~ /^(client|server|aclheader|aclbody|aclsym|aclapi|debug)$/; + "'aclheader', 'aclbody', 'aclsym', or 'debug'" + unless $mode =~ /^(client|server|aclheader|aclbody|aclsym|debug)$/; my $structprefix = shift or die "missing struct prefix argument"; my $procprefix = shift or die "missing procedure prefix argument"; @@ -452,14 +452,6 @@ if ($mode eq "aclsym") { # Automatically generated from $protocol by gendispatch.pl. # Do not edit this file. Any changes you make will be lost. __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. - --> -__EOF__ } else { print <<__EOF__; /* Automatically generated from $protocol by gendispatch.pl. @@ -2020,8 +2012,7 @@ elsif ($mode eq "client") { } } elsif ($mode eq "aclheader" || $mode eq "aclbody" || - $mode eq "aclsym" || - $mode eq "aclapi") { + $mode eq "aclsym") { my %generate = map { $_ => 1 } @autogen; my @keys = keys %calls; @@ -2059,8 +2050,6 @@ elsif ($mode eq "client") { print "\n"; print "#define VIR_FROM_THIS VIR_FROM_ACCESS\n"; print "\n"; - } elsif ($mode eq "aclapi") { - print "<aclinfo>\n"; } else { print "\n"; } @@ -2085,8 +2074,6 @@ elsif ($mode eq "client") { print $apiname . "CheckACL;\n"; } print $apiname . "EnsureACL;\n"; - } elsif ($mode eq "aclapi") { - &generate_aclapi($call); } else { &generate_acl($call, $call->{acl}, "Ensure"); if (defined $call->{aclfilter}) { @@ -2247,50 +2234,5 @@ elsif ($mode eq "client") { print "}\n\n"; } } - - sub generate_aclapi { - my $call = shift; - - my $apiname = $prefix . $call->{ProcName}; - if ($structprefix eq "qemu") { - $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; - } elsif ($structprefix eq "lxc") { - $apiname =~ s/virDomain/virDomainLxc/; - } - - print " <api name='$apiname'>\n"; - - my $acl = $call->{acl}; - foreach (@{$acl}) { - my @bits = split /:/; - 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]'"; - } - print "/>\n"; - } - - my $aclfilter = $call->{aclfilter}; - foreach (@{$aclfilter}) { - my @bits = split /:/; - my $objname = $bits[0]; - $objname =~ s/_/-/g; - my $perm = $bits[1]; - $perm =~ s/_/-/g; - - print " <filter object='$objname' perm='$perm'/>\n"; - } - - print " </api>\n"; - } - - } - - if ($mode eq "aclapi") { - print "</aclinfo>\n"; } } -- 2.39.2

On a Tuesday in 2023, Peter Krempa wrote:
Peter Krempa (6): remote_protocol: Fix list of supported ACL object names apibuild: Add infrastructure for generating ACL flag info into function docs scripts/apibuild: Extract and format API ACLs docs/newapi.xsl: Take API flag data from libvirt-api.xml rather than access/libvirt-access.xml docs|access: Don't build the ACL flags into a separate XML gendispatch: Drop 'aclapi' mode
docs/html/meson.build | 32 +------ docs/meson.build | 3 + docs/newapi.xsl | 20 ++--- scripts/apibuild.py | 163 ++++++++++++++++++++++++++++++++++- src/access/meson.build | 16 ---- src/remote/remote_protocol.x | 5 +- src/rpc/gendispatch.pl | 64 +------------- 7 files changed, 176 insertions(+), 127 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Peter Krempa