Page MenuHome GnuPG
Create Task
Maniphest T4994

Windows: assuan_sock_init or WSAStartup by main/_init_common_subsystem
Closed, ResolvedPublic
Actions

Description

WSAStartup is called from assuan_sock_init.

And I believe that an application should call assuan_sock_init (when it uses assuan sock API afterwards), instead of calling WSAStartup by itself.

But IIUC, I found discrepancies in:

  • dirmngr-client:
    • dirmngr/dirmngr-client.c: Calling WSAStartup
  • dirmngr
    • dirmngr/http.c: Calling WSAStartup
  • gpg, gpgsm, tools/*
    • No call of assuan_sock_init

I think that calling WSAStartup from _init_common_subsystem should be removed and an application should call assuan_sock_init.

I think that correct executable does:

  • gpg-agent
    • agent/gpg-agent.c: Call init_common_subsystems and assuan_sock_init
  • keyboxd
    • kbx/keyboxd.c: Call init_common_subsystems and assuan_sock_init
  • scdaemon:
    • scd/scdaemon.c: Call init_common_subsystems and assuan_sock_init

Revisions and Commits

rG GnuPG
rG8d9ce32c30db w32: More adding NETLIBS.
rGf95d923090e1 w32: Add link to $(NETLIB) for -lws2_32.
rG5fa4427419c8 w32: More adding NETLIBS.
rGd69f5570ee5e w32: Add link to $(NETLIB) for -lws2_32.

Related Objects

Event Timeline

• gniibe created this task.Jul 16 2020, 3:03 AM
• gniibe updated the task description. (Show Details)Jul 16 2020, 3:06 AM
• gniibe mentioned this in T4624: libassuan-config and libassuan.pc both put -lws2_32 before -lgpg-error, which fails during static linking.Jul 16 2020, 3:11 AM
• gniibe updated the task description. (Show Details)Jul 16 2020, 6:18 AM
• gniibe added a comment.Jul 16 2020, 6:22 AM

Call of WSAStartup in dirmngr/http.c is no problem, as we define HTTP_NO_WSASTARTUP.

• gniibe added a comment.EditedJul 16 2020, 6:43 AM

Here are the fixes:

diff --git a/common/init.c b/common/init.c
index 073c5cd8a..dbdf40527 100644
--- a/common/init.c
+++ b/common/init.c
@@ -161,17 +161,6 @@ _init_common_subsystems (gpg_err_source_t errsource, int *argcp, char ***argvp)
   /* Try to auto set the character set.  */
   set_native_charset (NULL);
 
-#ifdef HAVE_W32_SYSTEM
-  /* For W32 we need to initialize the socket layer.  This is because
-     we use recv and send in libassuan as well as at some other
-     places.  */
-  {
-    WSADATA wsadat;
-
-    WSAStartup (0x202, &wsadat);
-  }
-#endif
-
 #ifdef HAVE_W32CE_SYSTEM
   /* Register the sleep exit function before the estream init so that
      the sleep will be called after the estream registered atexit
diff --git a/dirmngr/dirmngr-client.c b/dirmngr/dirmngr-client.c
index 1ea10a8ad..eaedecf1d 100644
--- a/dirmngr/dirmngr-client.c
+++ b/dirmngr/dirmngr-client.c
@@ -208,19 +208,10 @@ main (int argc, char **argv )
    * init_common_subsystems, but we don't need that here.  */
   gpgrt_set_fixed_string_mapper (map_static_macro_string);
 
-  /* For W32 we need to initialize the socket subsystem.  Because we
-     don't use Pth we need to do this explicit. */
-#ifdef HAVE_W32_SYSTEM
- {
-   WSADATA wsadat;
-
-   WSAStartup (0x202, &wsadat);
- }
-#endif /*HAVE_W32_SYSTEM*/
-
   /* Init Assuan.  */
   assuan_set_assuan_log_prefix (log_get_prefix (NULL));
   assuan_set_gpg_err_source (GPG_ERR_SOURCE_DEFAULT);
+  assuan_sock_init ();
 
   /* Setup I18N. */
   i18n_init();
diff --git a/g10/gpg.c b/g10/gpg.c
index ce9197574..a0b63d570 100644
--- a/g10/gpg.c
+++ b/g10/gpg.c
@@ -2497,6 +2497,7 @@ main (int argc, char **argv)
     assuan_set_malloc_hooks (&malloc_hooks);
     assuan_set_gpg_err_source (GPG_ERR_SOURCE_DEFAULT);
     setup_libassuan_logging (&opt.debug, NULL);
+    assuan_sock_init ();
 
     /* Set default options which require that malloc stuff is ready.  */
     additional_weak_digest ("MD5");
diff --git a/sm/gpgsm.c b/sm/gpgsm.c
index 377cb1191..59b9adff1 100644
--- a/sm/gpgsm.c
+++ b/sm/gpgsm.c
@@ -1088,6 +1088,7 @@ main ( int argc, char **argv)
   assuan_set_malloc_hooks (&malloc_hooks);
   assuan_set_gpg_err_source (GPG_ERR_SOURCE_DEFAULT);
   setup_libassuan_logging (&opt.debug, NULL);
+  assuan_sock_init ();
 
   /* Setup a default control structure for command line mode */
   memset (&ctrl, 0, sizeof ctrl);
diff --git a/tools/gpg-card.c b/tools/gpg-card.c
index 7910a48fe..6325d5a1c 100644
--- a/tools/gpg-card.c
+++ b/tools/gpg-card.c
@@ -263,6 +263,7 @@ main (int argc, char **argv)
 
   assuan_set_gpg_err_source (GPG_ERR_SOURCE_DEFAULT);
   setup_libassuan_logging (&opt.debug, NULL);
+  assuan_sock_init ();
 
   /* Setup default options.  */
   opt.autostart = 1;
diff --git a/tools/gpg-connect-agent.c b/tools/gpg-connect-agent.c
index cde086770..2305f8f4e 100644
--- a/tools/gpg-connect-agent.c
+++ b/tools/gpg-connect-agent.c
@@ -1197,6 +1197,7 @@ main (int argc, char **argv)
   init_common_subsystems (&argc, &argv);
 
   assuan_set_gpg_err_source (0);
+  assuan_sock_init ();
 
 
   opt.autostart = 1;
diff --git a/tools/gpg-wks-client.c b/tools/gpg-wks-client.c
index 5f765d3c1..9093af732 100644
--- a/tools/gpg-wks-client.c
+++ b/tools/gpg-wks-client.c
@@ -276,6 +276,7 @@ main (int argc, char **argv)
 
   assuan_set_gpg_err_source (GPG_ERR_SOURCE_DEFAULT);
   setup_libassuan_logging (&opt.debug, NULL);
+  assuan_sock_init ();
 
   /* Parse the command line. */
   pargs.argc  = &argc;
• gniibe added a comment.Jul 17 2020, 2:35 AM

Sorry, I was confused by assuan_socket_ API and assuan_sock_ API.

gpg, gpgsm, dirmngr/dirmngr-client, and tools/* don't use assuan_sock_ API (use of assuan_sock_close only is OK in g10/server.c, with no assuan_sock_init).
So, no calling assuan_sock_init would be no problem, but then, another issues are:

  • Where should be call of WSAStartup for those applications? _init_common_subsystems might not be good because...
  • For gpg-agent, keyboxd, and scdaemon, WSAStartup is called twice.
• gniibe added a comment.EditedJul 17 2020, 4:32 AM

Given the situation we have call of WSAStartup in assuan_sock_init (for Windows), the solution would be:

  • Removal of call of WSAStartup in _init_common_subsystems
  • Even though it is not needed for POSIX system and it is only needed to call WAStartup on Windows, calling assuan_sock_init from each application (including gpg, gpgsm, dirmngr/dirmngr-client, and tools/* which uses libassuan), would be the solution (not perfect one, though, because it allocates sock_ctx)

Or else, to be better and correct:

  • Removal of call of WSAStartup in _init_common_subsystems
  • Introduce new function, say, socket_init in common/socket-init.c
    • which does nothing for POSIX
    • call WSAStartup for Windows
  • Add socket_init for applications which doesn't call assuan_sock_init but using assuan
• werner added a subscriber: • werner.Jul 17 2020, 10:43 AM

Thanks for looking into this. However, I do not understand the problem behind it. Is it the need to link against the socket lib? 10 or 15 years ago things were more complicated because two TCP stacks were in use and you could use the modern one only if a certain service pack or Explorer version was installed. That might be the reasons for some of the peculiarities we have in the code.

WSAStartup can be called several times, so that should not be a functional problem.

• gniibe triaged this task as Normal priority.Jul 17 2020, 11:33 AM

I just learned that WSAStartup can be called multiple times. So, it doesn't cause any erroneous behavior which I had been afraid of.

Currently, after libassuan change (requiring -lws2_32 only when static linking), I am trying to minimize libraries to be linked for executables in GnuPG, so that the change of libassuan won't break the build of GnuPG.

_init_common_subsystems is currently used even by an application which doesn't use assuan or any socket. But because of WSAStartup in _init_common_subsystems, it requires -lws2_32.

Well, it's totally minor issue. We can always revert the change of libassuan.

• gniibe added a comment.Jul 29 2020, 10:28 AM

Linking $(NETLIB) is required when the executable uses WSAStartup.

diff --git a/common/Makefile.am b/common/Makefile.am
index 31924317e..4ae4bf82d 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -189,34 +189,34 @@ t_common_ldadd = libcommon.a \
 
 # Common tests
 t_stringhelp_SOURCES = t-stringhelp.c $(t_extra_src)
-t_stringhelp_LDADD = $(t_common_ldadd)
+t_stringhelp_LDADD = $(t_common_ldadd) $(NETLIBS)
 
 t_timestuff_SOURCES = t-timestuff.c $(t_extra_src)
 t_timestuff_LDADD = $(t_common_ldadd)
 
 t_convert_LDADD = $(t_common_ldadd)
 t_percent_LDADD = $(t_common_ldadd)
-t_gettime_LDADD = $(t_common_ldadd)
-t_sysutils_LDADD = $(t_common_ldadd)
-t_helpfile_LDADD = $(t_common_ldadd)
-t_sexputil_LDADD = $(t_common_ldadd)
+t_gettime_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_sysutils_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_helpfile_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_sexputil_LDADD = $(t_common_ldadd) $(NETLIBS)
 t_b64_LDADD = $(t_common_ldadd)
-t_exechelp_LDADD = $(t_common_ldadd)
-t_exectool_LDADD = $(t_common_ldadd)
+t_exechelp_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_exectool_LDADD = $(t_common_ldadd) $(NETLIBS)
 t_session_env_LDADD = $(t_common_ldadd)
 t_openpgp_oid_LDADD = $(t_common_ldadd)
-t_ssh_utils_LDADD = $(t_common_ldadd)
-t_mapstrings_LDADD = $(t_common_ldadd)
+t_ssh_utils_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_mapstrings_LDADD = $(t_common_ldadd) $(NETLIBS)
 
 t_zb32_SOURCES = t-zb32.c $(t_extra_src)
 t_zb32_LDADD = $(t_common_ldadd)
 
-t_mbox_util_LDADD = $(t_common_ldadd)
-t_iobuf_LDADD = $(t_common_ldadd)
-t_strlist_LDADD = $(t_common_ldadd)
-t_name_value_LDADD = $(t_common_ldadd)
+t_mbox_util_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_iobuf_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_strlist_LDADD = $(t_common_ldadd) $(NETLIBS)
+t_name_value_LDADD = $(t_common_ldadd) $(NETLIBS)
 t_ccparray_LDADD = $(t_common_ldadd)
-t_recsel_LDADD = $(t_common_ldadd)
+t_recsel_LDADD = $(t_common_ldadd) $(NETLIBS)
 
 # System specific test
 if HAVE_W32_SYSTEM
diff --git a/dirmngr/Makefile.am b/dirmngr/Makefile.am
index e812329dc..506a3f65d 100644
--- a/dirmngr/Makefile.am
+++ b/dirmngr/Makefile.am
@@ -82,7 +82,7 @@ endif
 dirmngr_LDADD = $(libcommonpth) \
         $(DNSLIBS) $(LIBASSUAN_LIBS) \
 	$(LIBGCRYPT_LIBS) $(KSBA_LIBS) $(NPTH_LIBS) \
-	$(NTBTLS_LIBS) $(LIBGNUTLS_LIBS) $(LIBINTL) $(LIBICONV)
+	$(NTBTLS_LIBS) $(LIBGNUTLS_LIBS) $(LIBINTL) $(LIBICONV) $(NETLIBS)
 if USE_LDAP
 dirmngr_LDADD += $(ldaplibs)
 endif
diff --git a/sm/Makefile.am b/sm/Makefile.am
index 7314341e3..4931dabba 100644
--- a/sm/Makefile.am
+++ b/sm/Makefile.am
@@ -65,7 +65,7 @@ common_libs = ../kbx/libkeybox509.a $(libcommon)
 gpgsm_LDADD = $(common_libs) ../common/libgpgrl.a \
               $(LIBGCRYPT_LIBS) $(KSBA_LIBS) $(LIBASSUAN_LIBS) \
               $(GPG_ERROR_LIBS) $(LIBREADLINE) $(LIBINTL) \
-	      $(LIBICONV) $(resource_objs) $(extra_sys_libs)
+	      $(LIBICONV) $(resource_objs) $(extra_sys_libs) $(NETLIBS)
 gpgsm_LDFLAGS = $(extra_bin_ldflags)
 
 
@@ -80,7 +80,7 @@ t_common_ldadd = $(libcommon) $(LIBGCRYPT_LIBS) $(KSBA_LIBS) \
 t_minip12_CFLAGS = -DWITHOUT_NPTH=1 \
 	           $(LIBGCRYPT_CFLAGS) $(GPG_ERROR_CFLAGS)
 t_minip12_SOURCES = $(t_common_src) t-minip12.c minip12.c
-t_minip12_LDADD   = $(t_common_ldadd)
+t_minip12_LDADD   = $(t_common_ldadd) $(NETLIBS)
 
 
 
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 878c249bc..eed6a44eb 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -185,7 +185,7 @@ gpg_wks_client_SOURCES = \
 gpg_wks_client_CFLAGS = $(LIBASSUAN_CFLAGS) $(GPG_ERROR_CFLAGS) $(INCICONV)
 gpg_wks_client_LDADD = $(libcommon) \
 		       $(LIBASSUAN_LIBS) $(LIBGCRYPT_LIBS) $(GPG_ERROR_LIBS) \
-		       $(LIBINTL) $(LIBICONV)
+		       $(LIBINTL) $(LIBICONV) $(NETLIBS)
 
 if HAVE_NEWER_LIBGCRYPT
 libexec_PROGRAMS += gpg-pair-tool
• aheinecke mentioned this in T4623: pkg-config for mingw needs to emit -lws2_32.Jul 29 2020, 1:17 PM
• aheinecke added a subscriber: • aheinecke.Jul 29 2020, 1:20 PM

That patch fixes the build problem I got into today when trying to build 2.3 for windows. So ? from me and please commit the patch as it is already required when assuan and gpgrt config no longer emit ws2_32 in their pgk-config --libs line.

• gniibe added a commit: rGd69f5570ee5e: w32: Add link to $(NETLIB) for -lws2_32..Jul 30 2020, 4:41 AM
• gniibe added a commit: rG5fa4427419c8: w32: More adding NETLIBS..Jul 30 2020, 8:09 AM
• gniibe added a commit: rGf95d923090e1: w32: Add link to $(NETLIB) for -lws2_32..Jul 30 2020, 8:17 AM
• gniibe added a commit: rG8d9ce32c30db: w32: More adding NETLIBS..
• gniibe changed the task status from Open to Testing.Jul 30 2020, 8:27 AM
• gniibe added a project: Restricted Project.

Pushed modified patch to master and 2.2.

I learned that even if an application (like t-name-value) does no network access, it requires to link NETLIBS, because (in deep dependency) some function like log_error might use socket output.

When/if more accurate library specification is needed (for other OS), it would be good to introduce new variable (say, WS2_32_LIB).
For now, we use NETLIB for this purpose.

• gniibe closed this task as Resolved.Nov 16 2020, 7:29 AM
• gniibe claimed this task.