Page MenuHome GnuPG
Create Task
Maniphest T7399

assuan: GPG_ERR_ASS_NO_DATA_CB situation
Open, NormalPublic
Actions

Description

When a client wrongly implements the transaction and doesn't supply data callback (data_cb), it results GPG_ERR_ASS_NO_DATA_CB error in assuan_transact function.

In this situation, in the buffer, the response (D-lines) from server remains unread.

And it results more errors for next transactions, because the client now sees wrong responses.

I observed this in gpgsm master (connecting gpg-agent 2.4, which doesn't support --status option for LISTTRUSTED command). Server's responses are D-lines, but client expects status lines.

Related Objects

Event Timeline

• gniibe triaged this task as High priority.Nov 15 2024, 3:51 AM
• gniibe created this task.
• gniibe added a project: libassuan.
• gniibe renamed this task from assuan: GPG_ERR_ASS_NO_INQUIRE_CB situation to assuan: GPG_ERR_ASS_NO_DATA_CB situation.Nov 15 2024, 5:18 AM
• gniibe updated the task description. (Show Details)
• gniibe mentioned this in T7323: scdaemon hangs up (when output from scdaemon is not consumed by gpg-agent).
• gniibe added a comment.Nov 29 2024, 8:29 AM

Here is my proposal to avoid unsynched state for data.

diff --git a/src/client.c b/src/client.c
index 410f940..0989984 100644
--- a/src/client.c
+++ b/src/client.c
@@ -250,6 +250,7 @@ assuan_transact (assuan_context_t ctx,
   int off;
   char *line;
   int linelen;
+  gpg_error_t last_err = 0;
 
   rc = assuan_write_line (ctx, command);
   if (rc)
@@ -272,15 +273,18 @@ assuan_transact (assuan_context_t ctx,
   else if (response == ASSUAN_RESPONSE_DATA)
     {
       if (!data_cb)
-        rc = _assuan_error (ctx, GPG_ERR_ASS_NO_DATA_CB);
+        rc = GPG_ERR_ASS_NO_DATA_CB;
       else
         {
           rc = data_cb (data_cb_arg, line, linelen);
           if (ctx->flags.confidential)
             wipememory (ctx->inbound.line, LINELENGTH);
-          if (!rc)
-            goto again;
         }
+      /* We need to consume the whole data from server to avoid
+         unsynched state.  */
+      if (!rc)
+        last_err = rc;
+      goto again;
     }
   else if (response == ASSUAN_RESPONSE_INQUIRE)
     {
@@ -336,14 +340,18 @@ assuan_transact (assuan_context_t ctx,
   else if (response == ASSUAN_RESPONSE_END)
     {
       if (!data_cb)
-        rc = _assuan_error (ctx, GPG_ERR_ASS_NO_DATA_CB);
+        rc = GPG_ERR_ASS_NO_DATA_CB;
       else
-        {
-          rc = data_cb (data_cb_arg, NULL, 0);
-          if (!rc)
-            goto again;
-        }
+        rc = data_cb (data_cb_arg, NULL, 0);
+      /* We need to finish the response from server to avoid
+         unsynched state.  */
+      if (!rc)
+        last_err = rc;
+      goto again;
     }
 
+  if (!rc)
+    return last_err;
+
   return rc;
 }

similar change for status would be also needed.

• gniibe lowered the priority of this task from High to Normal.Dec 2 2024, 6:00 AM

Put it under lower priority, as it's basically programming error.