Re: [gmime-devel] extracting session keys from encrypted parts (or: how to augment the interface of GMimeDecryptResult?)



On Thu 2016-07-14 04:56:00 +0200, Jeffrey Stedfast wrote:
Honestly, I'm not sure if it matters if you just add a session key to 
the existing GMimeDecryptResult struct.

I'd probably prefer that over a new subclass.

I know it technically breaks ABI, but meh. What are the scenarios that 
would break for people if we did this? Is it that big of an issue?

Where it breaks is if someone declares a GMimeDecryptResult object (not
a GMimeDecryptResult*) directly on the stack (or on the heap via
e.g. malloc(sizeof(GMimeDecryptResult)) and then somehow passes that
object through to the library (or any other code) in a way that the
library can operate on it.

If the library believes that the struct is larger than the actual
allocated RAM, the library could modify the adjacent RAM (whatever is
next to it on the stack or the heap).

So in this example, anything in that modifies or accesses the new member
of a GMimeDecryptResult that was created by direct allocation (not by
g_mime_decrypt_result_new()) which built against the old ABI could
potentially cause a segfault (if the neighboring bytes are unallocated)
or could read or write to RAM being used for other purposes.

Likewise, if any code tried to access the a new session_key directly
as a struct member (not through an accessor function) because it built
against the newer version, but is then deployed against the older
version, the runtime linker won't catch this mistake, and the code might
be investigating unallocated memory.

That said, in the entire debian archive, there are only 4 packages
(including gmime) that use GMimeDecryptResult and it looks to me like
none of them allocate it directly:

 https://codesearch.debian.net/search?q=GMimeDecryptResult&perpkg=1

Both pan and maildirutils access the signatures member of the
struct directly, though :/

Not everything is in debian, of course, but it's a nice way to get a bit
of an overview.

Most of the G* libs have moved to making the actual structs private I 
think, which has its pluses for sure (in that you can expand the structs 
w/o breaking ABI), but I always liked the idea of being able to just 
quickly dereference members to get at the internals rather than having 
to use a g_mime_object_type_get_property () API which I always found 
annoying. Then again, I'm not necessary opposed to it either (just more 
explaining why I never bothered).

I suspect most people use the functions to access property values, so 
I'm likely in the minority.

Since sealing all the structs would certainly break API, that's not a 
realistic fix for the short-term, so we're back to either adding the 
member to the struct or subclassing and I'd prefer just adding the 
member assuming it won't break most people using decryption.

I think I'm ok with that if you are -- though gmime does break API in
the future, it would probably be good to make the structs opaque for
this reason.

In the meantime, it'd be nice to indicate that direct access to the
struct members is deprecated where possible.  GTK apparently uses a
preprocessor symbol GSEAL_ENABLE:

  https://developer.gnome.org/gtk3/stable/gtk-migrating-2-to-3.html#id-1.6.3.3.5

i don't know whether that's available directly to other GLib users, or
if it's a gtk-only thing.

   --dkg

Attachment: signature.asc
Description: PGP signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]