Page MenuHome GnuPG

libassuan, w32: assuan_sock_check_nonce fails with master
Closed, ResolvedPublic

Description

We noticed during testing for the last Gpg4win release that with current master of libassuan Kleopatra fails to create the uiserver socket on Windows. On Linux Kleopatra does not use the UiServer so it is not tested there.

I bisected the failure to: c93eb901e58d5b31294c2d452659b5150d95ec59

The code failing in Kleopatra is:

qCDebug(KLEOPATRA_LOG) << "UiServer: client connect on fd " << fd;
if (assuan_sock_check_nonce((assuan_fd_t)fd, &nonce)) {
    qCDebug(KLEOPATRA_LOG) << "UiServer: nonce check failed";
    assuan_sock_close((assuan_fd_t)fd);
    return;
}

assuan_sock_check_nonce returns true since c93eb901e58d5b31294c2d452659b5150d95ec59 with an fd value of something like 1272.

@gniibe I am assigning this to you because the commit was created by you.

Revisions and Commits

Event Timeline

aheinecke triaged this task as Normal priority.Mon, Jan 2, 3:32 PM
aheinecke created this task.

Btw. This is how Kleopatra creates the socket: https://dev.gnupg.org/source/kleo/browse/master/src/uiserver/uiserver_win.cpp$34 which does not use a function that would set is_socket=1. My naive fix would be:

diff --git a/src/assuan-socket.c b/src/assuan-socket.c
index 3fefc39..3749d8f 100644
--- a/src/assuan-socket.c
+++ b/src/assuan-socket.c
@@ -527,6 +527,7 @@ eval_redirection (const char *fname, int *r_redirect)
 assuan_fd_t
 _assuan_sock_new (assuan_context_t ctx, int domain, int type, int proto)
 {
+  ctx->flags.is_socket = 1;
 #ifdef HAVE_W32_SYSTEM
   assuan_fd_t res;
   if (domain == AF_UNIX || domain == AF_LOCAL)

But I am unsure if this is correct or if this is used for something where is_socket should not be true.

The question is why Kleopatra does not use assuan_sock_set_sockaddr_un as we do in GnuPG. See for example
https://dev.gnupg.org/source/gnupg/browse/master/kbx/keyboxd.c$1124 - was this a workaround back when we had no support for Unicode? assuan_sock_set_sockaddr_un and assuan_sock_get_nonce work together and their internal workings should be opaque to the caller.

OTOH, a patch similar to your suggestion should not harm. But needs to be double checked.

From the NEWS assuan_sock_set_sockaddr_un was only added in 2014, years after the UIServer code was really last modified.

But I don't see the problem here. The only difference I can see between what Kleo does and assuan_sock_set_sockaddr_un is that sa_familiy is set to AF_LOCAL in set_sockaddr and to AF_UNIX in kleo. which results in the same codepath in sock_get_nonce.

As far as I can see that would not change that "is_socket" is not set which in turn causes assuan_write and assuan_read in system-w32 not to use the socket functions but instead ReadFile and WriteFile. This fails then where it previously would have made a different check with the is_socket function. Without having tested it I think the keyboxd code would have the same problem.

What I mean is that our socket emulation is encapsulated in libgcrypt and details should not be visible to the caller. Further libassuan and kleopatra might be build against different libc versions and thus the used structures might also differ.

I found an issue in the assuan code of client side. This might be the cause of the server failure for nonce.

Thanks! I don't understand why your fix helps as Kleopatra never calls assuan_socket_connect on the server side, but it helps. :) So this is probably called indirectly and I don't see it.

I am changing this to resolved as I have tested it multiple times.

My understanding is that: selftest in Kleo does call assuan_socket_connect (possibly in kleopatra/src/libkleopatraclient/core/command.cpp), and it didn't send nonce correctly.

gniibe mentioned this in Unknown Object (Event).Mon, Jan 9, 7:34 AM