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

[Daniel Kahn Gillmor]

On Tue 2016-11-15 23:46:30 +0900, Werner Koch wrote:
> If you build software using a new header we can assume that the it is
> also linked to the matching version of the library.  The SO minor number
> will be bumped up for the release (but not in the git version).  By
> tinkering with the build system I mean to force linking against an older
> SO; that is installing just the application but not the library and
> hoping that everything works.

In practice, at runtime, the linker normally looks for the SO major
number, not the SO minor number.  that is, it looks for libfoo.so.X, not
libfoo.so.X.Y.

Most people who install software today did not build it themselves, and
they rely on library dependency declarations, as stated by their package
managers.  So its the dependency declaratoins done by the package
managers that need to be well-formed.

>> 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.
>
> We have
>
>  gpgme_set_sender                NEW.
>  gpgme_get_sender                NEW.
>  gpgme_op_query_swdb             NEW.
>  gpgme_op_query_swdb_result      NEW.
>  gpgme_query_swdb_result_t       NEW.
>  gpgme_get_ctx_flag              NEW.
>
> however they are most likely not used.  That is the same as with
> gpgme_set_export_session_keys.

sure, but if we say that gpgme-dependent softwre that accesses
->session_key must have invoked gpgme_set_export_session_keys(), then
that software will use the symbols (and get mapped to the correct, tight
dependencies).  And other gpgme-dependent software that *doesn't* ever
access ->session_key won't use the symbol, and it will get mapped to
(also correct) looser depenedencies, making it easier to install on
different systems.

> Well, we have new exported symbols.  And the SO minor will be bumbed.
> So both mechanismn are in place; the Debian thing and the proper thing
> all libraries should do.

If there's no guidance that requires users to test whether the library
actually supports the new field, and there's no explicit symbol use,
then the safest thing for me to do as a distributor will be to implement
much more strict versioning than i'd otherwise need to, which will make
binary packages harder for people to install.  *shrug* so it goes.

>> 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?
>
> Okay, I thought I could leave that out because we don't have a way to
> check whether the override works in gpg.  But, okay for education I add
> an error check.

I'll send a patch shortly, now that i hear you're OK with this.  thanks!

>> 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.
>
> We are extending the result structures for more than 10 years.  I am not
> aware of any problems related to this.  Granted, some of these changes
> are merely new bits taken from a reserved bit field variable but often
> enough we added new pointer fields.

I'm not sure what to tell you about why you haven't heard about these
problems.  They're easy to trigger, unfortunately.

>> +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,
>
> We would need to add such requirments to a lot of other fucntions as
> well.  For example the TOFU stuff also adds new fields to the result
> structure which _may_ only be set when GPGME_KEYLIST_MODE_WITH_TOFU is
> used.

it's not a question of whether the fields are *set* correctly -- even
checking to see whether they're NULL or not when run against older
versions of the library will cause a segmentation fault.  So it's
critically important to not try to access them *at all* unless you know
for certain that you're using a version of the library that supports it.

> For backward compatible changes on glibc systems we bump the minor SO
> number - what's wrong with this approach?  Just because some projects
> don't do proper ABI tracking we shall fallback to such hacks as tracking
> new symbol names?

it's not "hacks", it's a commonly-accepted practice [0], and there are
good reasons for it.

The better package managers try to ensure that their dependencies are as
*loose* as possible, so that binary packages are as widely-deployable as
possible.

The simplest way to do that effectively given the binary packages is to
inspect the linked symbols.  If the API changes in such a way that it's
not detectable by symbol changes, then the package managers are forced
to create tighter dependencies to avoid runtime failures.  This makes it
more difficult to install software on different systems.

This isn't the end of the world, but it's a consequence of the API
design that ought to be made explicitly.

> If you really really want it, I guess we could add extra code on glibc
> systems to detect broken installations.  But is it really worse the
> trouble and extra bugs just due to this code?  We used such magic in
> GNOME more than 15 years ago.  Can you point me to any bug reported
> related to GPGME caused by such improper use?  I only recall problems
> with off_t and off64_t for which we added detection code to gpgme.

No, please do not add extra code to do a custom check for this at
runtime.  The ideal situation would be a library with symbol-based
versioning (or some other standard, simple mechanism).  that would let
us check things at build time and map them to explicit
dependencies. Barring that, we can just make the external dependency
declarations tighter and carry on.

Thanks for the discussion!

       --dkg

[0] e.g. "Do not expose any complex structures in your API; Use get()
    and set() instead." from libabc's README, which also encourages the
    use of symbol versioning:
     https://git.kernel.org/cgit/linux/kernel/git/kay/libabc.git/plain/README
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 962 bytes
Desc: not available
URL: </pipermail/attachments/20161116/1a1d6b2c/attachment.sig>