----- "Daniel Veillard" <veillard@xxxxxxxxxx> 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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list