[libvirt] [PATCH 01/20] Secret manipulation step 1: Public API

Wed Aug 19 09:36:27 UTC 2009

----- "Daniel Veillard" <veillard at redhat.com> wrote:
> On Sun, Aug 16, 2009 at 10:47:54PM +0200, Miloslav Trmač wrote:
> > Rather than add explicit accessors for attributes of secrets, and
> > hard-code the "secrets are related to storage volumes" association
> in
> > the API, the API uses XML to manipulate the association as well as
> > other attributes, similarly to other areas of libvirt.
> > 
> > The user can set attributes of the secret using XML, e.g.
> >   <secret ephemeral='no' private='yes'>
> >     <uuid>b8eecf55-798e-4db7-b2dd-025b0cf08a36</uuid>
> >     <volume>/var/lib/libvirt/images/mail.img</volume>
> >     <description>LUKS passphrase for our mail server</description>
> >   </secret>
> > If <uuid/> is not specified, it is chosen automatically.
> 
>   Should secret always be tied to volumes. The API is generic enough
> that we should make sure we can use this later to get priviledged
> access to other resources, though right now I don't have a good
> example in mind.
The secret is not "tied" to a volume - the volume is an optional attribute.  A libvirt user can ignore the volume field completely, and manage the secrets separately - but other clients, e.g. virt-manager, can use the volume attribute to locate a secret for a given volume without storing any data outside of libvirt.

> > - s/secret_id/uuid/g
> 
>   Not sure I catch that, 
An earlier patch was calling the ID of a secret object "secret_id" everywhere, leading to constructs like <secret secret_id=.../>  In an earlier patch review I was asked to change that.

> > - use "unsigned char *" for secret value
> 
>   fine, assuming 0 terninated. though this may not work with everything
> but since we need to import/export to XML, unless uuencoding the value
> we won't be able to embed something which is not part of XML character
> range (#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] |
> [#x10000-#x10FFFF]) unicode code points.
No, the value is an arbitrary set of bytes that is never passed through XML.  Originally it was (void *value, size_t value_size), the change to (unsigned char *value, size_t value_size) was requested in an earlier review.

> > diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
> > new file mode 100644
> > index 0000000..7471bf7
> > --- /dev/null
> > +++ b/docs/formatsecret.html.in
> > @@ -0,0 +1,52 @@
> > +<html>
> > +  <body>
> > +    <h1>Secret XML format</h1>
> > +
> > +    <ul id="toc"></ul>
> 
>   it's good to have a document from the start :-) but this lacks a bit
>   of the intended use for the API, i.e. where this may be required to
>   use it.
Those parts are documented in later patches that add references to secrets to the <volume> and <domain> XML formats.

> > +virSecretPtr            virSecretDefineXML      (virConnectPtr conn,
> > +                                                 const char *xml);
> 
>  Let's add an "unsigned int flags" to virSecretDefineXML() especially
> as we don't know yet the scope of future usages.
>
> > +char *                  virSecretGetXMLDesc     (virSecretPtr secret);
> 
>   And also add an "unsigned int flags" to virSecretGetXMLDesc
> 
Will do.

> > +int                     virSecretSetValue       (virSecretPtr secret,
> > +                                                 const unsigned char *value,
> > +                                                 size_t value_size);
> > +unsigned char *         virSecretGetValue       (virSecretPtr secret,
> > +                                                 size_t *value_size);
> 
>   Ah, so we aren't relying on 0 termination, but in that case the value
> need to be uuencoded in the XML, and that should be made clear in the
> API description. Actually not having the secret completely in the clear
> in the XML dump sounds like a good idea.
The values are never transferred in the XML, only using the virSecret[GS]etValue functions.  (Storing secrets in XML was a major objection against the first version of my patch e.g. because XML tends to be revealed in various debugging dumps.)
    Mirek