[libvirt] [PATCH] Add info about access control checks into API reference

From: "Daniel P. Berrange" <berrange@redhat.com> So that app developers / admins know what access control checks are performed for each API, this patch extends the API docs generator to include details of the ACLs for each. The gendispatch.pl script is extended so that it generates a simple XML describing ACL rules, eg. <aclinfo> ... <api name='virConnectNumOfDomains'> <check object='connect' perm='search_domains'/> <filter object='domain' perm='getattr'/> </api> <api name='virDomainAttachDeviceFlags'> <check object='domain' perm='write'/> <check object='domain' perm='save' flags='!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE'/> <check object='domain' perm='save' flags='VIR_DOMAIN_AFFECT_CONFIG'/> </api> ... </aclinfo> The newapi.xsl template loads the XML files containing the ACL rules and generates a short block of HTML for each API describing the parameter checks and return value filters (if any). Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/libvirt.css | 14 +++++++++++ docs/newapi.xsl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 22 ++++++++++++++-- src/rpc/gendispatch.pl | 59 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 157 insertions(+), 6 deletions(-) diff --git a/docs/libvirt.css b/docs/libvirt.css index 8a00d12..ed67b2f 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -477,3 +477,17 @@ dl.variablelist > dt { dl.variablelist > dt:after { content: ": "; } + +table.acl { + margin: 1em; + border-spacing: 0px; + border: 1px solid #ccc; +} + +table.acl tr, table.acl td { + padding: 0.3em; +} + +table.acl thead { + background: #ddd; +} diff --git a/docs/newapi.xsl b/docs/newapi.xsl index d5b210e..58f12eb 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -29,6 +29,69 @@ <xsl:variable name="htmldir">html</xsl:variable> <xsl:variable name="href_base">../</xsl:variable> + <xsl:variable name="acls"> + <xsl:copy-of select="document('../src/libvirt_access.xml')/aclinfo/api"/> + </xsl:variable> + <xsl:variable name="qemuacls"> + <xsl:copy-of select="document('../src/libvirt_access_qemu.xml')/aclinfo/api"/> + </xsl:variable> + <xsl:variable name="lxcacls"> + <xsl:copy-of select="document('../src/libvirt_access_lxc.xml')/aclinfo/api"/> + </xsl:variable> + + <xsl:template name="aclinfo"> + <xsl:param name="api"/> + + <xsl:if test="count(exsl:node-set($acls)/api[@name=$api]/check) > 0"> + <h5>Access control parameter checks</h5> + <table class="acl"> + <thead> + <tr> + <th>Object</th> + <th>Permission</th> + <th>Condition</th> + </tr> + </thead> + <xsl:apply-templates select="exsl:node-set($acls)/api[@name=$api]/check" mode="acl"/> + </table> + </xsl:if> + <xsl:if test="count(exsl:node-set($acls)/api[@name=$api]/filter) > 0"> + <h5>Access control return value filters</h5> + <table class="acl"> + <thead> + <tr> + <th>Object</th> + <th>Permission</th> + </tr> + </thead> + <xsl:apply-templates select="exsl:node-set($acls)/api[@name=$api]/filter" mode="acl"/> + </table> + </xsl:if> + </xsl:template> + + <xsl:template match="check" mode="acl"> + <tr> + <td><xsl:value-of select="@object"/></td> + <td><xsl:value-of select="@perm"/></td> + <xsl:choose> + <xsl:when test="@flags"> + <td><xsl:value-of select="@flags"/></td> + </xsl:when> + <xsl:otherwise> + <td>-</td> + </xsl:otherwise> + </xsl:choose> + </tr> + </xsl:template> + + <xsl:template match="filter" mode="acl"> + <tr> + <td><xsl:value-of select="@object"/></td> + <td><xsl:value-of select="@perm"/></td> + </tr> + </xsl:template> + + <xsl:template name="navbar"> <xsl:variable name="previous" select="preceding-sibling::file[1]"/> <xsl:variable name="next" select="following-sibling::file[1]"/> @@ -553,6 +616,11 @@ </xsl:if> </dl> </xsl:if> + <div class="acl"> + <xsl:call-template name="aclinfo"> + <xsl:with-param name="api" select="$name"/> + </xsl:call-template> + </div> </xsl:template> <xsl:template match="exports" mode="toc"> diff --git a/src/Makefile.am b/src/Makefile.am index ac66ecf..277f749 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -830,6 +830,11 @@ ACCESS_DRIVER_SYM_FILES = \ libvirt_access_qemu.syms \ libvirt_access_lxc.syms +ACCESS_DRIVER_API_FILES = \ + libvirt_access.xml \ + libvirt_access_qemu.xml \ + libvirt_access_lxc.xml + ACCESS_DRIVER_SOURCES = \ access/viraccessperm.h access/viraccessperm.c \ access/viraccessmanager.h access/viraccessmanager.c \ @@ -1496,8 +1501,8 @@ EXTRA_DIST += $(ACCESS_DRIVER_POLKIT_SOURCES) endif -BUILT_SOURCES += $(ACCESS_DRIVER_GENERATED) -CLEANFILES += $(ACCESS_DRIVER_GENERATED) +BUILT_SOURCES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_API_FILES) +CLEANFILES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_API_FILES) libvirt_access.syms: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am @@ -1512,6 +1517,19 @@ libvirt_access_lxc.syms: $(srcdir)/rpc/gendispatch.pl \ $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclsym \ lxc LXC $(LXC_PROTOCOL) > $@ +libvirt_access.xml: $(srcdir)/rpc/gendispatch.pl \ + $(REMOTE_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclapi \ + remote REMOTE $(REMOTE_PROTOCOL) > $@ +libvirt_access_qemu.xml: $(srcdir)/rpc/gendispatch.pl \ + $(QEMU_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclapi \ + qemu QEMU $(QEMU_PROTOCOL) > $@ +libvirt_access_lxc.xml: $(srcdir)/rpc/gendispatch.pl \ + $(LXC_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclapi \ + lxc LXC $(LXC_PROTOCOL) > $@ + $(srcdir)/access/viraccessapicheck.h: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=aclheader \ diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 8f41771..ac0c7ab 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -41,8 +41,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' or 'debug'" - unless $mode =~ /^(client|server|aclheader|aclbody|aclsym|debug)$/; + "'aclheader', 'aclbody', 'aclsym', 'aclapi' or 'debug'" + unless $mode =~ /^(client|server|aclheader|aclbody|aclsym|aclapi|debug)$/; my $structprefix = shift or die "missing struct prefix argument"; my $procprefix = shift or die "missing procedure prefix argument"; @@ -351,6 +351,13 @@ if ($mode eq "aclsym") { # Automatically generated by gendispatch.pl. # Do not edit this file. Any changes you make will be lost. __EOF__ +} elsif ($mode eq "aclapi") { + print <<__EOF__; +<!-- + - Automatically generated by gendispatch.pl. + - Do not edit this file. Any changes you make will be lost. + --> +__EOF__ } else { print <<__EOF__; /* Automatically generated by gendispatch.pl. @@ -1641,7 +1648,8 @@ elsif ($mode eq "client") { } } elsif ($mode eq "aclheader" || $mode eq "aclbody" || - $mode eq "aclsym") { + $mode eq "aclsym" || + $mode eq "aclapi") { my %generate = map { $_ => 1 } @autogen; my @keys = keys %calls; @@ -1667,6 +1675,7 @@ elsif ($mode eq "client") { foreach my $hdr (@headers) { print "#include \"$hdr\"\n"; } + print "\n"; } elsif ($mode eq "aclbody") { my $header = shift; print "#include <config.h>\n"; @@ -1676,8 +1685,12 @@ elsif ($mode eq "client") { print "#include \"virerror.h\"\n"; print "\n"; print "#define VIR_FROM_THIS VIR_FROM_ACCESS\n"; + print "\n"; + } elsif ($mode eq "aclapi") { + print "<aclinfo>\n"; + } else { + print "\n"; } - print "\n"; foreach (@keys) { my $call = $calls{$_}; @@ -1699,6 +1712,8 @@ 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}) { @@ -1835,5 +1850,41 @@ elsif ($mode eq "client") { print "}\n\n"; } } + + sub generate_aclapi { + my $call = shift; + + my $apiname = "vir" . $call->{ProcName}; + if ($structprefix eq "qemu") { + $apiname =~ s/virDomain/virDomainQemu/; + } elsif ($structprefix eq "lxc") { + $apiname =~ s/virDomain/virDomainLxc/; + } + + print " <api name='$apiname'>\n"; + + my $acl = $call->{acl}; + foreach (@{$acl}) { + my @bits = split /:/; + print " <check object='$bits[0]' perm='$bits[1]'"; + if (defined $bits[2]) { + print " flags='$bits[2]'"; + } + print "/>\n"; + } + + my $aclfilter = $call->{aclfilter}; + foreach (@{$aclfilter}) { + my @bits = split /:/; + print " <filter object='$bits[0]' perm='$bits[1]'/>\n"; + } + + print " </api>\n"; + } + + } + + if ($mode eq "aclapi") { + print "</aclinfo>\n"; } } -- 1.8.1.4

On 08/07/2013 06:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
So that app developers / admins know what access control checks are performed for each API, this patch extends the API docs generator to include details of the ACLs for each.
The gendispatch.pl script is extended so that it generates a simple XML describing ACL rules, eg.
<aclinfo> ... <api name='virConnectNumOfDomains'> <check object='connect' perm='search_domains'/> <filter object='domain' perm='getattr'/> </api> <api name='virDomainAttachDeviceFlags'> <check object='domain' perm='write'/> <check object='domain' perm='save' flags='!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE'/> <check object='domain' perm='save' flags='VIR_DOMAIN_AFFECT_CONFIG'/> </api> ... </aclinfo>
The newapi.xsl template loads the XML files containing the ACL rules and generates a short block of HTML for each API describing the parameter checks and return value filters (if any).
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/libvirt.css | 14 +++++++++++ docs/newapi.xsl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 22 ++++++++++++++-- src/rpc/gendispatch.pl | 59 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 157 insertions(+), 6 deletions(-)
I'm no css or xsl expert, and perl is not my strongest language; but I can say that this patch applies and that the output looks like a useful and correct improvement. (See the attached screenshot)
+++ b/src/Makefile.am @@ -830,6 +830,11 @@ ACCESS_DRIVER_SYM_FILES = \ libvirt_access_qemu.syms \ libvirt_access_lxc.syms
+ACCESS_DRIVER_API_FILES = \ + libvirt_access.xml \ + libvirt_access_qemu.xml \ + libvirt_access_lxc.xml +
I also tested 'make distcheck' with this patch applied (which includes VPATH testing and checks that the new files are appropriately generated rather than included in the tarball). I will note that 'make distcheck' currently fails if 'make' was not run first; but that problem existed before your patch, so even if you made it worse, it's not a show-stopper. And since 'make all distcheck' passes, it appears that you got the makefile magic right. ACK.
+} elsif ($mode eq "aclapi") { + print <<__EOF__; +<!-- + - Automatically generated by gendispatch.pl.
This says WHO generated, but not WHICH file to edit if the generated file contains errors. Can we add the source .x file as additional information (probably as a separate patch, since the other generated files likely have the same issue)? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/07/2013 12:06 PM, Eric Blake wrote:
On 08/07/2013 06:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
So that app developers / admins know what access control checks are performed for each API, this patch extends the API docs generator to include details of the ACLs for each.
The newapi.xsl template loads the XML files containing the ACL rules and generates a short block of HTML for each API describing the parameter checks and return value filters (if any).
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/libvirt.css | 14 +++++++++++ docs/newapi.xsl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 22 ++++++++++++++-- src/rpc/gendispatch.pl | 59 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 157 insertions(+), 6 deletions(-)
Can you also touch .gitignore to account for the following files now created by your patch? # src/libvirt_access.xml # src/libvirt_access_lxc.xml # src/libvirt_access_qemu.xml -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Aug 07, 2013 at 12:06:09PM -0600, Eric Blake wrote:
On 08/07/2013 06:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
So that app developers / admins know what access control checks are performed for each API, this patch extends the API docs generator to include details of the ACLs for each.
The gendispatch.pl script is extended so that it generates a simple XML describing ACL rules, eg.
<aclinfo> ... <api name='virConnectNumOfDomains'> <check object='connect' perm='search_domains'/> <filter object='domain' perm='getattr'/> </api> <api name='virDomainAttachDeviceFlags'> <check object='domain' perm='write'/> <check object='domain' perm='save' flags='!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE'/> <check object='domain' perm='save' flags='VIR_DOMAIN_AFFECT_CONFIG'/> </api> ... </aclinfo>
The newapi.xsl template loads the XML files containing the ACL rules and generates a short block of HTML for each API describing the parameter checks and return value filters (if any).
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/libvirt.css | 14 +++++++++++ docs/newapi.xsl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 22 ++++++++++++++-- src/rpc/gendispatch.pl | 59 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 157 insertions(+), 6 deletions(-)
I'm no css or xsl expert, and perl is not my strongest language; but I can say that this patch applies and that the output looks like a useful and correct improvement. (See the attached screenshot)
Hah, I'm sadly too familiar with xsl from previous work writing a content management system where the entire web UI was generated with XSL transforms :-(
+} elsif ($mode eq "aclapi") { + print <<__EOF__; +<!-- + - Automatically generated by gendispatch.pl.
This says WHO generated, but not WHICH file to edit if the generated file contains errors. Can we add the source .x file as additional information (probably as a separate patch, since the other generated files likely have the same issue)?
I guess we could add that. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Eric Blake