[libvirt] fix for recent gnutls behavior change
TL;DR: please think if more needs to be changed to follow new gnutls behavior Hi, I found that recently our builds broke with these errors: TEST: virnettlssessiontest [...] 10) TLS Session servercertreq.filename + clientcertreq.filename ... FAILED 11) TLS Session servercertreq.filename + clientcertreq.filename ... FAILED 12) TLS Session servercertreq.filename + clientcertreq.filename ... OK 13) TLS Session servercertreq.filename + clientcertreq.filename ... FAILED 14) TLS Session servercertreq.filename + clientcertreq.filename ... FAILED 15) TLS Session servercertlevel3areq.filename + clientcertlevel2breq.filename ... OK FAIL virnettlssessiontest (exit status: 1) I was able to track that down to a change in gnutls which can be read at https://gitlab.com/gnutls/gnutls/issues/111 which changed the behavior of dname en- and decoding to follow RFC4514. More details on the debugging - if anyone want to reproduce - can be found on https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1641615 But the primary purpose of the cover letter is a call to everybody to think if that change could imply the need for more changes in libvirt than just to make the tests work again. Christian Ehrhardt (1): tests: adapt to gnutls change in dname en-/decoding tests/virnettlssessiontest.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) -- 2.7.4
A recent change in gnutls that was released with 3.5.6 changed the behavior of dname en- and decoding to follow RFC4514. That breaks the related tests which failed validation in virNetTLSContextCheckCertDNWhitelist due to the strings no more matching in the fnmatch check. The fix is a gnutls version dependent definition of the wildcard strings used by the tests (older gnutls versions require the old order). Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- tests/virnettlssessiontest.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 0d2e106..c0ef5f6 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -374,6 +374,33 @@ mymain(void) DO_SESS_TEST(cacertreq.filename, servercertalt2req.filename, clientcertreq.filename, false, false, "wiki.libvirt.org", NULL); +#if GNUTLS_VERSION_NUMBER >= 0x030506 + const char *const wildcards1[] = { + "CN=dogfood,C=UK", + NULL, + }; + const char *const wildcards2[] = { + "CN=libvirt,C=UK", + NULL, + }; + const char *const wildcards3[] = { + "CN=dogfood,C=UK", + "CN=libvirt,C=UK", + NULL, + }; + const char *const wildcards4[] = { + "CN=libvirtstuff,C=UK", + NULL, + }; + const char *const wildcards5[] = { + "CN=libvirt*,C=UK", + NULL, + }; + const char *const wildcards6[] = { + "CN=*virt*,C=UK", + NULL, + }; +#else const char *const wildcards1[] = { "C=UK,CN=dogfood", NULL, @@ -399,6 +426,7 @@ mymain(void) "C=UK,CN=*virt*", NULL, }; +#endif /* GNUTLS_VERSION_NUMBER >= 0x030506 */ DO_SESS_TEST(cacertreq.filename, servercertreq.filename, clientcertreq.filename, true, false, "libvirt.org", wildcards1); -- 2.7.4
On Wed, Nov 16, 2016 at 04:23:41PM +0100, Christian Ehrhardt wrote:
A recent change in gnutls that was released with 3.5.6 changed the behavior of dname en- and decoding to follow RFC4514.
That breaks the related tests which failed validation in virNetTLSContextCheckCertDNWhitelist due to the strings no more matching in the fnmatch check.
The fix is a gnutls version dependent definition of the wildcard strings used by the tests (older gnutls versions require the old order).
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- tests/virnettlssessiontest.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
NACK, the gnutls changes are being reverted by upstream and IMHO if any distro is shipping 3.5.6 they should revert them too, as the change was a semantic break in gnutls API that will in turn break any libvirt deployments using this feature when upgraded Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On Wed, Nov 16, 2016 at 4:44 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
NACK, the gnutls changes are being reverted by upstream and IMHO if any distro is shipping 3.5.6 they should revert them too, as the change was a semantic break in gnutls API that will in turn break any libvirt deployments using this feature when upgraded
Thats kind of what I thought when seeing the effect of the change, but I didn't find that upstream reverted that yesterday. Thanks for pointing this out as it makes more sense this way. Explicitly looking for it I found the change in gnutls which is not yet released in any version: commit 70bf8475bb0ab178fe36ee4c601a6cfec8e70a3f Author: Nikos Mavrogiannopoulos <nmav@redhat.com> Date: Fri Nov 11 16:20:01 2016 +0100 Introduced new functions to allow multiple DN parsing modes The old DN parsing functions are changed to return the original non-fully compliant with RFC4514 string format, while the new ones return the compliant string by default. This allows applications which relied on the previous format to continue functioning without changes. -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
participants (2)
-
Christian Ehrhardt -
Daniel P. Berrange