[GPGME PATCH] core: Enable extraction of session keys.

[Daniel Kahn Gillmor]

Hi Werner--

Thanks for the review and the followup.  This is great!

On Tue 2016-11-15 18:42:34 +0900, Werner Koch wrote:

> Thanks for the patch.  I applied it with this comment:
>
>     On the concern of adding a new field to a structure: It may not be
>     clearly documented but we don't expect that a user ever allocates such
>     a structure - those result structure may only be created bu gpgme and
>     are read-only for the user.  Adding a new member constitutes a
>     compatible ABI change and thus an older SO may not be used by code
>     compiled with a header for the newer API.  Unless someone tinkers with
>     the build system, this should never happen.  We have added new fields
>     to result structure may times and I can't remember any problems.

I'm not sure what this means exactly -- when you say "an older SO may
not be used by code compiled with a header for the newer API", are you
implying that there's mechanism to enforce that?  I don't think "tinkers
with the build system" is the issue -- there are many people who run
code on systems different than the build system.  The build system can
remain intact, but the version mismatch can happen during deployment.

the only mechanism i'm aware of to enforce the correct linkage is the
dynamic linker's requirement of mapping symbols (function names).  But
with the choice of using gpgme_set_ctx_flag instead of
gpgme_set_export_session_keys, we don't have any exported symbol to
catch errors when the dynamic linker runs.

In debian, we would be able to distinguish compiled objects that link
against the new symbol and automatically indicate the right dependency
versioning in the associated package as a result (we track which version
a new symbol was added).  I don't think we have any way to do that
otherwise, though.  I suppose i can just mark the symbols depdendencies
as a hard lower limit with the new revision, though.  that will be a
tighter dependency match than necessary for most code (which doesn't
need the new functionality), but whatever.

> I also went forward and implemented the override feature.  However I
> changed the API for both:
>
>     To keep the number of context manipulation functions at bay, this
>     patches removes the just added gpgme_set_export_session_keys and
>     gpgme_get_export_session_keys by flags for the generic context
>     function.
>
> This is because I consider the session-key exporting a rarely used
> feature and thus the generic flag setting function is more appropriate.
> This also instantly provides the Qt and cpp bindings.  Usage is pretty
> simple:
>
>   if (export_session_key)
>     gpgme_set_ctx_flag (ctx, "export-session-key", "1");
>   if (override_session_key)
>     gpgme_set_ctx_flag (ctx, "override-session-key", override_session_key);
>
> see tests/run-decrypt.c

If this is intended to be an example for how others should use this
feature, it's a bit dangerous to not check the error return value from
gpgme_set_ctx_flag, no?

It would be good to have a bit of documentation that ensures that the
user at least tests that the flags have been properly set.

How about this change:

-----------------------
diff --git a/doc/gpgme.texi b/doc/gpgme.texi
index fd396e0..8820312 100644
--- a/doc/gpgme.texi
+++ b/doc/gpgme.texi
@@ -4810,14 +4810,19 @@ known, otherwise this is a null pointer.
 @item char *session_key
 A textual representation (nul-terminated string) of the session key
 used in symmetric encryption of the message, if the context has been
 set to export session keys (see @code{gpgme_set_ctx_flag,
 "export-session-key"}), and a session key was available for the most
 recent decryption operation.  Otherwise, this is a null pointer.
 
+You must not try to access this member of the struct unless
+ at code{gpgme_set_ctx_flag (ctx, "export-session-key")} returns
+ at code{GPG_ERR_NO_ERROR} or @code{gpgme_get_ctx_flag (ctx,
+"export-session-key")} returns @code{"1"}.
+
 @end table
 @end deftp
 
 @deftypefun gpgme_decrypt_result_t gpgme_op_decrypt_result (@w{gpgme_ctx_t @var{ctx}})
 The function @code{gpgme_op_decrypt_result} returns a
 @code{gpgme_decrypt_result_t} pointer to a structure holding the
 result of a @code{gpgme_op_decrypt} operation.  The pointer is only
-----------------------

another (hacky) advantage of this is that compliant reliant code might
make use of the newly-introduced symbols gpgme_get_ctx_flag(), which
would give us the exposed symbol indicators that debian (and similar
systems) could use for dependency tracking :)

Thanks again for sorting this out,

       --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 962 bytes
Desc: not available
URL: </pipermail/attachments/20161115/660bcf17/attachment.sig>