Revisiting krb5_gic_opt_ext

[ghudson@MIT.EDU]

Warning: grotty stuff ahead.

The background: the krb5_get_init_creds system was originally
specified using a caller-allocated options structure
(krb5_get_init_creds_opt), which meant options fields couldn't be
added without breaking the ABI.

To address this, krb5_gic_opt_ext was created.  This is a
library-allocated structure which is ABI-compatible with
krb5_get_init_creds_opt (sort of; see below).  If you're an API which
takes an options argument, you call krb5int_gic_opt_to_opte on it,
with the "force" flag set.

opt_to_opte has an "interesting" contract: if the input is already an
extended structure, it creates an alias; otherwise, it creates a copy
and sets a "shadowed" flag.  The caller is supposed to check the
shadowed flag and free the structure when it is done.

The problem: this contract is not nestable.  If the input of
opt_to_opte is a copy created by a previous call to opt_to_opte, then
the output will be an alias which looks like a copy, and that will
result in a double-free.

We haven't run into this problem yet because we've never had a reason
to pass an opt_to_opte'd options structure to an external API.  But
I'm preparing to integrate some IAKERB support code which does;
specifically, the synchronous wrapper function krb5_get_init_creds
needs to supply options to krb5_init_creds_init.

Possible solutions:

1. Make opt_to_opte copy the structure if the input is shadowed and
the force flag is set.  This solves the immediate problem and creates
no extra work if the original caller used the new API.

2. Make opt_to_opte copy the structure all the time.  This is extra
work in the common case, but makes the code easier to analyze.
("Maybe this is allocated and maybe this is an alias" is not a very
safe construction.)

3. Eliminate the copies.  Instead, when peering at new options fields,
use accessors which can deal with either an extended or non-extended
options structure.  This would touch the most code, but would have the
same benefits of (2) without the extra allocation-and-copy work.  This
would probably entail changing internal interfaces like
krb5_get_init_creds to use pointers to krb5_get_init_creds_opt instead
of krb5_gic_opt_ext.

My current preference is for (3).

Some ancillary issues while I am in the neighborhood:

1. In RT #6034, Tom notes that krb5_gic_opt_ext is not guaranteed to
be ABI-compatible with krb5_get_init_creds_opt simply because it has
the same initial fields, and instead suggests making krb5_gic_opt_ext
have a krb5_get_init_creds_opt as its first field.  It would be
easy to make this change in concert with solution (3) above.

2. All of the new fields in krb5_gic_opt_ext are indirected through a
field opt_private, which is a pointer to a krb5_gic_opt_private
structure.  This feels needless; does anyone know why it was done this
way, instead of just adding fields directly to krb5_gic_opt_ext?