assuan interface for gpgme

[Marcus Brinkmann]

Werner Koch wrote:
> On Wed, 18 Feb 2009 12:02, marcus.brinkmann at ruhr-uni-bochum.de said:
> 
>>>     err = sendfnc (sendfnc_value, my_data, my_datalen);
>> I think we should use a gpgme_assuan_send function that does the dispatch to
>> the sendfnc.  We use user provided function pointers in the interface, but
> 
> I considered that but it has the drawback that we need, more or less, to
> expose the assuan interface.  Another option I considered was to either
> pass the gpgme context to the callback or some opaque data to be passed
> to an gpgme_assuan_send_data_from_inq function.

It doesn't matter if the interface is similar to the assuan interface, as long
as it is the correct interface from GPGME's point of view.

>> well, and I don't think that's necessary.  One reason is that this way we can
>> catch all calls into GPGME with a regex breakpoint on gpgme_*.
> 
> Good point.  Given that there may only be one inquire going on per gpgme
> context, what about changing the calllback to:
> 
>   typedef gpgme_error_t (*gpgme_assuan_inquire_cb_t)
>        (void *opaque, const char *name, const char *args,
>         gpgme_ctx_t ctx); 
> 
> and a 
> 
>   gpgme_op_assuan_send_inq_data (...)
> 
> which takes the context and may check that it has been called from an
> inq callback.

In this case, the user can keep the CTX behind the opaque pointer.  No need to
pass redundant information.  However, I still think that providing a buffer
that the user fills is a reasonable idea.  There is one other idea below which
also avoids a send_inq_data function.

>> Instead, the user could indicate if there is more data to be returned by
>> setting a flag in a return value slot, and GPGME can keep calling the inquire
>> callback when sending data is possible until the user is satisfied.
> 
> That would complicate the data flow and make an inquire callback pretty
> unreadable.  We need to allow calling a send function as often as
> needed.

Yes, it complicates the data flow.  Unfortunately, the simple data flow
doesn't work reliably for asynchronous operation.  We can not allow sending
more data in response to an unblocked file descriptor than the system's
PIPE_BUF guaranteed write space.  This is why it makes sense to provide the
buffer to be filled with the inquire callback and drop the send function.  The
available buffer size is a system parameter that GPGME can encapsulate.

Even if you expect the server not to block, there is a problem: The current
interface (but not my proposals) require the user to send all the data at
once.  This can make the application unresponsive for an unacceptable length
of time if a lot of data needs to be sent in an inquiry.  Responsiveness in
usability is measured in miliseconds.

>> If we also provide a buffer for the user to fill, then no send callback is
>> necessary.
> 
> That makes it hard to use.  You don't know beforehand how much data to
> send back.  For example a requested certificate my be just a few hundred
> bytes of 100kb in the case of PGP.

Inquiries are hard to use because of their nature.  They turn around the data
flow semantics between the client and the server in the middle of an operation.

We know beforehand what data we can allow to send back in response to an
unblocked select call on the file descriptor.  It is PIPE_BUF on Unix and
something else on Windows (depending on the I/O backend).

In any way, it is not so hard to implement a pointer into a buffer and provide
the data in chunks, using the opaque pointer.

However, here is another idea:  The user could return a gpgme_data_t object to
the inquire call, and GPGME could handle the flow control transparently.  This
is flexible, and relatively easy for the application.  For simple inquiries,
this is more overhead, but for bulk data it is simpler.  Of course, we could
provide both (a buffer for the user to fill and a gpgme_data_t* if the buffer
is too small).

>>>     err = gpgme_op_assuan_result (ctx);
>>>
>>> which returns 0 for the 'OK' line and an gpg-error code for the 'ERR
>>> <n>' line.
>> For consistency and extensibility, we should return a struct, just as with all
> 
> I don't see a reason for extensibility because the protocol defines that
> there is just a retrun value of OK or ERR which can all be represented
> by an integer (famous last 640k-are-enough words).

Just from the top of my head, we may want to provide a log of the transaction,
some statistics, debug information (audit log), or the server's welcome
message with a version info.

In any case, consistency alone is reason enough to not use a different
semantics for the _result function.

>   if (!err)
>     {
>       s = gpgme_op_assuan_result (ctx);
>       err = s? s->err : gpg_error (GPG_ERR_BUG);
>     }
> 
> is a lot of unneeded code.

The check for S is superfluous if you know that the last operation was an
assuan operation.  So it is possible to write:

if (!err)
  err = (gpgme_op_assuan_result (ctx))->err;

If you feel strongly about it, we can provide an accessor function for the
error field in the result structure:

gpgme_error_t gpgme_op_assuan_error (ctx);

On the other hand, I introduced the result structures exactly to avoid the
need for accessor functions.

My main point is that it shouldn't be called _result() if it doesn't behave
like one of those.

> The question is more whether we need a
> separate error code at all for syncronous operations. Folding it into
> the error code returned by gpgmg_op_assuan_transact would make things
> even easier. If there is need to check whetehr it is an application
> error or a gpgme error, a test with gpg_err_source (err) ==
> GPG_ERR_SORUC$E_GPGME would do the trick.

There is nothing that prevents the server from returning an error with a GPGME
error source.  This can get very confusing.

I think there is some advantage to return all results of the operation in the
result structure, and to return only GPGME internal errors in functions: It is
a simple rule that can be easily understood.  The current mix of return codes
for example for a verify operation is not easy to understand.

> In fact the code I wrote
> until now always did:
> 
>       err = gpgme_op_assuan_transact ();
>       if (!err)
>         err = gpgme_op_assuan_result ();
> 
> and thus not distinguish between the errors.  Usuallay you expect
> success or a few well defined error codes, everyhing else is a severe
> error and most likely you will close or reset the context in response.

True, but it should happen as late as possible, so that a proper diagnostic is
possible.

>> We should add a gpgme_op_assuan_end function, as we have for keylistings.  No
>> need to trash a perfectly reasonably context to disconnect.
> 
> A new gpgme_reset could be used to reuse a context.  Pretty easy to
> implement, given that we already have a _gpgme_op_reset.

A specific end function is more reliable, as it can do consistency checks.  It
can also try to terminate the server connection nicely.  This does not
preclude adding a reset function, but specifying that is more work.

>> need to have this mechanism: Options can be passed as assuan commands using
>> transact the normal way, that's the whole purpose of the protocol.  Using
> 
> I was think not thinking about application option but those required to
> operate a certain socket.

As far as I know, all assuan sockets are equal.  What do you have in mind?  In
any case, the only example we have, GPG_AGENT, is not operating on the socket,
but on the application behind it.

>> convenience behavior as well.  My preference however would be to just require
>> home_dir to be NULL at all times, and to not have any default commands, and
>> instead provide a convenience function
>>
>> gpgme_error_t gpgme_op_assuan_init_gpg_agent (ctx);
> 
> That is very special for the gpg-agent.  Right the option I suggested is
> also special for the gpg-agent (due to PINENTRY_LAUNCHED) but it is not
> codified in the API and on the same semantic level as the name of the
> socket.

I don't understand why you think that this is not part of the API.  It most
definitely is.  OPTION is a regular command and PINENTRY_LAUNCHED is a regular
inquiry that must be handled according to an application-specific policy.

Let's separate the issues. The OPTIONs can be put into the init function I
prototyped above.  The pinentry_launched is actually not particular to
gpg-agent, it could be passed through several other applications (and in fact
it is!).  In fact, it is not even clear that the current process is the right
one to allow the set foreground call.  It may need to be forwarded.  In fact,
it is a bug that it is not possible right now for the application to intercept
PINENTRY_LAUNED inquiries resulting from GpgSM operations.

Thus, I think automatic or external (ie application-level) handling of
PINENTRY_LAUNCHED  should be a special flag for the gpgme context, shared by
assuan and gpgsm operations.  The interface should be a configurable
pinentry_launched callback function that can be set per context and that
defaults to calling the set foreground window function.

>> way there is no inconsistency and no difficulty in understanding and extending
>> the interface.  This also avoids sending commands at connection time, which is
>> good for reliability in the asynchronous variant (in case the server blocks
>> errornously).
> 
> We have quite a couple of hacks in gpgme to cope with special
> requirements of GnuPG.  I don't see the gpgme_op_assuan family as a
> replacement for libassuan. it is merely a convenience API which allows
> us to re-use code we already have in gpgme in case an application
> requires services from GnuPG which are not implemented by gpgme proper.

I am not objecting to the functionality in GPGME, I am only arguing about the
correct interface.  There are at least two reasons against the home_dir
overload: It's a misuse of an existing interface, it is not extensible (can
only pass fixed strings).  Also, parsing strings in C is not very good (source
for bugs, no type safety, no compile time checks, ...)

> We are expecting well behaving servers and frankly a buggy server my
> block at any time not just right after a completed command transaction.
> In fact that is even more likely due to the size of the OS buffers.

That's true, and we actually build on that assumption in the gpgsm engine in
GPGME.  We should specify this in the assuan documentation.  Because this is
true, I did not propose to split up the init function for gpg-agent (that sets
the OPTIONs) into asynchronously operating parts.

I feel pretty strongly about interface consistency.  I do not feel too
strongly about asynchronous operation at this moment, but I think that we
should prepare the interfaces for it to be possible.

Thanks,
Marcus