sts-sponsors team mailing list archive

[Dan Streetman <ddstreet@xxxxxxxxxxxxx>]

Review: Needs Fixing

one more minor comment inline below

Diff comments:

> diff --git a/debian/patches/dirmngr-handle-EAFNOSUPPORT-at-connect_server.patch b/debian/patches/dirmngr-handle-EAFNOSUPPORT-at-connect_server.patch
> new file mode 100644
> index 0000000..d926add
> --- /dev/null
> +++ b/debian/patches/dirmngr-handle-EAFNOSUPPORT-at-connect_server.patch
> @@ -0,0 +1,61 @@
> +From ca937cf390662b830d4fc5d295e69b24b1778050 Mon Sep 17 00:00:00 2001
> +From: NIIBE Yutaka <gniibe@xxxxxxxx>
> +Date: Mon, 13 Jul 2020 10:00:58 +0900
> +Subject: [PATCH] dirmngr: Handle EAFNOSUPPORT at connect_server.
> +
> +* dirmngr/http.c (connect_server): Skip server with EAFNOSUPPORT.
> +
> +--
> +
> +GnuPG-bug-id: 4977
> +Signed-off-by: NIIBE Yutaka <gniibe@xxxxxxxx>
> +
> +Origin: backport, https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=commit;h=109d16e8f644
> +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1910432
> +---
> + dirmngr/http.c | 9 +++++++++
> + 1 file changed, 9 insertions(+)
> +
> +Index: gnupg2/dirmngr/http.c
> +===================================================================
> +--- gnupg2.orig/dirmngr/http.c
> ++++ gnupg2/dirmngr/http.c
> +@@ -2843,7 +2843,7 @@ connect_server (const char *server, unsi
> +   unsigned int srvcount = 0;
> +   int hostfound = 0;
> +   int anyhostaddr = 0;
> +-  int srv, connected;
> ++  int srv, connected, v4_valid, v6_valid;
> +   gpg_error_t last_err = 0;
> +   struct srventry *serverlist = NULL;
> +
> +@@ -2930,9 +2930,11 @@ connect_server (const char *server, unsi
> +
> +       for (ai = aibuf; ai && !connected; ai = ai->next)
> +         {
> +-          if (ai->family == AF_INET && (flags & HTTP_FLAG_IGNORE_IPv4))
> ++          if (ai->family == AF_INET
> ++              && ((flags & HTTP_FLAG_IGNORE_IPv4) || !v4_valid))

I think this is checking v4_valid before it's initialized; it seems it was added with the upstream commit 12def3a84e, but that looks way bigger than is needed to backport. Maybe you could just initialize v4_valid = (flags & HTTP_FLAG_IGNORE_IPv4) and then you only need to check v4_valid in this if statement? And do the same for v6_valid of course

> +             continue;
> +-          if (ai->family == AF_INET6 && (flags & HTTP_FLAG_IGNORE_IPv6))
> ++          if (ai->family == AF_INET6
> ++              && ((flags & HTTP_FLAG_IGNORE_IPv6) || !v6_valid))
> +             continue;
> +
> +           if (sock != ASSUAN_INVALID_FD)
> +@@ -2940,6 +2942,15 @@ connect_server (const char *server, unsi
> +           sock = my_sock_new_for_addr (ai->addr, ai->socktype, ai->protocol);
> +           if (sock == ASSUAN_INVALID_FD)
> +             {
> ++              if (errno == EAFNOSUPPORT)
> ++                {
> ++                  if (ai->family == AF_INET)
> ++                    v4_valid = 0;
> ++                  if (ai->family == AF_INET6)
> ++                    v6_valid = 0;
> ++                  continue;
> ++                }
> ++
> +               err = gpg_err_make (default_errsource,
> +                                   gpg_err_code_from_syserror ());
> +               log_error ("error creating socket: %s\n", gpg_strerror (err));


-- 
https://code.launchpad.net/~halves/ubuntu/+source/gnupg2/+git/gnupg2/+merge/396408
Your team STS Sponsors is requested to review the proposed merge of ~halves/ubuntu/+source/gnupg2:lp1910432-bionic into ubuntu/+source/gnupg2:ubuntu/bionic-devel.