[libvirt] [PATCH] Generate RFC4122 compliant UUIDs
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Neither does virUUIDIsValid() check for RFC4122 compliance. Following patch modifies virUUIDGenerate() to generate valid UUIDs and adds check to virUUIDIsValid() to validate UUIDs. Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz> --- src/util/viruuid.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 7250543..2dc9a56 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -114,6 +114,25 @@ virUUIDGenerate(unsigned char *uuid) err = virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); } + /* + * Make UUID RFC 4122 compliant. Following form will be used: + * + * xxxxxxxx-xxxx-Axxx-Bxxx-xxxxxxxxxxxx + * + * where + * A is version defined in 4.1.3 of RFC + * Msb0 Msb1 Msb2 Msb3 Version Description + * 0 1 0 0 4 The randomly or pseudo- + * randomly generated version + * specified in this document. + * + * B is variant defined in 4.1.1 of RFC + * Msb0 Msb1 Msb2 Description + * 1 0 x The variant specified in this document. + */ + uuid[6] = (uuid[6] & 0x0F) | (4 << 4); + uuid[8] = (uuid[8] & 0x3F) | (2 << 6); + return err; } @@ -209,6 +228,7 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr) * Do some basic tests to check whether the given UUID is * valid as a host UUID. * Basic tests: + * - Validate RFC4122 compliance * - Not all of the digits may be equal */ int @@ -216,10 +236,21 @@ virUUIDIsValid(unsigned char *uuid) { unsigned int i, ctr = 1; unsigned char c; + unsigned char version; + unsigned char variant; if (!uuid) return 0; + /* + * RFC4122 defines version 1 to 5 (section 4.1.3) + * RFC4122 defined variant is desribed in section 4.1.1 + */ + version = (uuid[6] >> 4); + variant = (uuid[8] >> 6); + if (!(version > 0 && version <= 5) || variant != 2) + return 0; + c = uuid[0]; for (i = 1; i < VIR_UUID_BUFLEN; i++) -- 1.7.1
On Mon, Apr 08, 2013 at 12:35:40PM -0400, Milos Vyletel wrote:
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Neither does virUUIDIsValid() check for RFC4122 compliance. Following patch modifies virUUIDGenerate() to generate valid UUIDs and adds check to virUUIDIsValid() to validate UUIDs.
Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz> --- src/util/viruuid.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 7250543..2dc9a56 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -114,6 +114,25 @@ virUUIDGenerate(unsigned char *uuid) err = virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); }
+ /* + * Make UUID RFC 4122 compliant. Following form will be used: + * + * xxxxxxxx-xxxx-Axxx-Bxxx-xxxxxxxxxxxx + * + * where + * A is version defined in 4.1.3 of RFC + * Msb0 Msb1 Msb2 Msb3 Version Description + * 0 1 0 0 4 The randomly or pseudo- + * randomly generated version + * specified in this document. + * + * B is variant defined in 4.1.1 of RFC + * Msb0 Msb1 Msb2 Description + * 1 0 x The variant specified in this document. + */ + uuid[6] = (uuid[6] & 0x0F) | (4 << 4); + uuid[8] = (uuid[8] & 0x3F) | (2 << 6); + return err; }
ACK to this bit of the patch
@@ -209,6 +228,7 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr) * Do some basic tests to check whether the given UUID is * valid as a host UUID. * Basic tests: + * - Validate RFC4122 compliance * - Not all of the digits may be equal */ int @@ -216,10 +236,21 @@ virUUIDIsValid(unsigned char *uuid) { unsigned int i, ctr = 1; unsigned char c; + unsigned char version; + unsigned char variant;
if (!uuid) return 0;
+ /* + * RFC4122 defines version 1 to 5 (section 4.1.3) + * RFC4122 defined variant is desribed in section 4.1.1 + */ + version = (uuid[6] >> 4); + variant = (uuid[8] >> 6); + if (!(version > 0 && version <= 5) || variant != 2) + return 0; + c = uuid[0];
for (i = 1; i < VIR_UUID_BUFLEN; i++)
but NACk to this part What you're checking here is just one possible valid scheme for UUIDs. We shouldn't reject UUIDs just because they use a different scheme than the one we do. Regards, 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 :|
----- Original Message -----
On Mon, Apr 08, 2013 at 12:35:40PM -0400, Milos Vyletel wrote:
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Neither does virUUIDIsValid() check for RFC4122 compliance. Following patch modifies virUUIDGenerate() to generate valid UUIDs and adds check to virUUIDIsValid() to validate UUIDs.
Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz> --- src/util/viruuid.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 7250543..2dc9a56 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -114,6 +114,25 @@ virUUIDGenerate(unsigned char *uuid) err = virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); }
+ /* + * Make UUID RFC 4122 compliant. Following form will be used: + * + * xxxxxxxx-xxxx-Axxx-Bxxx-xxxxxxxxxxxx + * + * where + * A is version defined in 4.1.3 of RFC + * Msb0 Msb1 Msb2 Msb3 Version Description + * 0 1 0 0 4 The randomly or pseudo- + * randomly generated version + * specified in this document. + * + * B is variant defined in 4.1.1 of RFC + * Msb0 Msb1 Msb2 Description + * 1 0 x The variant specified in this document. + */ + uuid[6] = (uuid[6] & 0x0F) | (4 << 4); + uuid[8] = (uuid[8] & 0x3F) | (2 << 6); + return err; }
ACK to this bit of the patch
Thanks.
@@ -209,6 +228,7 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr) * Do some basic tests to check whether the given UUID is * valid as a host UUID. * Basic tests: + * - Validate RFC4122 compliance * - Not all of the digits may be equal */ int @@ -216,10 +236,21 @@ virUUIDIsValid(unsigned char *uuid) { unsigned int i, ctr = 1; unsigned char c; + unsigned char version; + unsigned char variant;
if (!uuid) return 0;
+ /* + * RFC4122 defines version 1 to 5 (section 4.1.3) + * RFC4122 defined variant is desribed in section 4.1.1 + */ + version = (uuid[6] >> 4); + variant = (uuid[8] >> 6); + if (!(version > 0 && version <= 5) || variant != 2) + return 0; + c = uuid[0];
for (i = 1; i < VIR_UUID_BUFLEN; i++)
but NACk to this part
What you're checking here is just one possible valid scheme for UUIDs. We shouldn't reject UUIDs just because they use a different scheme than the one we do.
Fair enough. I've noticed shortly after I've sent the mail that it's only used to verify hosts UUID and does not validate user defined UUIDs for guests. Let me remove these changed and resend patch with only first part. Thanks, Milos
On 04/08/2013 10:45 AM, Daniel P. Berrange wrote:
On Mon, Apr 08, 2013 at 12:35:40PM -0400, Milos Vyletel wrote:
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Neither does virUUIDIsValid() check for RFC4122 compliance. Following patch modifies virUUIDGenerate() to generate valid UUIDs and adds check to virUUIDIsValid() to validate UUIDs.
+ /* + * RFC4122 defines version 1 to 5 (section 4.1.3) + * RFC4122 defined variant is desribed in section 4.1.1 + */ + version = (uuid[6] >> 4); + variant = (uuid[8] >> 6); + if (!(version > 0 && version <= 5) || variant != 2) + return 0; + c = uuid[0];
for (i = 1; i < VIR_UUID_BUFLEN; i++)
but NACk to this part
What you're checking here is just one possible valid scheme for UUIDs. We shouldn't reject UUIDs just because they use a different scheme than the one we do.
Furthermore, if we took this hunk, but a user is running a guest created by a previous version of libvirt that happened to generate an invalid UUID, we would lose the ability to manage that older guest. It _might_ be appropriate to warn the user when a UUID is not valid according to the schemes we recognize, but it must be a warning and not a fatal error; furthermore, if we do add such a warning, we'd need to recognize ALL of the schemes that are valid in the RFCs, not just the particular scheme we use when generating a uuid ourselves, so as to minimize false negative printouts of the warning. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Following patch modifies generated UUIDs to conform to rules described in RFC. Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz> --- src/util/viruuid.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 7250543..8f82187 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -114,6 +114,25 @@ virUUIDGenerate(unsigned char *uuid) err = virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); } + /* + * Make UUID RFC 4122 compliant. Following form will be used: + * + * xxxxxxxx-xxxx-Axxx-Bxxx-xxxxxxxxxxxx + * + * where + * A is version defined in 4.1.3 of RFC + * Msb0 Msb1 Msb2 Msb3 Version Description + * 0 1 0 0 4 The randomly or pseudo- + * randomly generated version + * specified in this document. + * + * B is variant defined in 4.1.1 of RFC + * Msb0 Msb1 Msb2 Description + * 1 0 x The variant specified in this document. + */ + uuid[6] = (uuid[6] & 0x0F) | (4 << 4); + uuid[8] = (uuid[8] & 0x3F) | (2 << 6); + return err; } -- 1.7.1
On 04/08/2013 12:10 PM, Milos Vyletel wrote:
Even though http://libvirt.org/formatdomain.html#elementsMetadata states that it requires RFC4122 compliance UUIDs that are generated by virUUIDGenerate() are not. Following patch modifies generated UUIDs to conform to rules described in RFC.
Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz> --- src/util/viruuid.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
ACK and pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Milos Vyletel