An actual error map

[frozencemetery]

    /* placeholder,                                                                                      
     * we will need an actual map but to speed up testing just make a sum with                           
     * a special base and hope no conflicts will happen in the mechglue */

(from gss_plugin.c)

[simo5]

I wonder if we really need to change this at this point, I forgot why I thought an "actual errormap" was needed.

Possibly I thought of fully resolving the error string via gss_display_status() and caching them, as technically it is possible that the underlying mechanism will "lose status" between calls that may end up hitting different threads within gss-proxy.

But then restarting gss-proxy would cause loss of state anyway ... I wonder if we should push this back to uspstream. It has been an age long request from many API users to provide sensible minor error numbers and not arbitrarely mapped ones, esp when we are just relaying unix errno values. There is indeed some code that assumes this is always the case and tries to directly map those minor numbers because they are often directly visible anyway ... (breaks when SPNEGO is used IIRC).

Robbie, WDYT ?

[frozencemetery]

Looking at the code, we add a (seemingly arbitrary) large value to any errors we're encapsulating. (#define MAP_ERROR_BASE 0x04200000). As I read it, the intent is to ensure we can distinguish wrapped and unwrapped errors - that is, errors from the mech we're interposing and errors from the gssproxy logic itself.

But if that's the case, I'm not sure what "an actual map" would even look like. Nothing constrains what mechs can use for minor codes that I'm aware of.

[frozencemetery]

I'm also not sure why we make this distinction at all, as gssi_display_status() unconditionally unmaps the error.

[simo5]

If you look in the krb5 mechanism what happens is that some errors are stored in the krb5 context and returned dynamically when requested. If they are note requested immediately they will be eventually cycled out.

For example in ASC there is an "EINVAL" numerical error code associated with an arbitrary text message, and this pair is stored in a map table and returned if requested.

I think the idea was to create a dynamic map similar to that where we immediately resolve via gss_dispaly_status errors as they are reported and store internally the result, returning the table index as the minor status. Upon request we'd return that specific error back.

Issues I see are related to draining the table to avoid an ever growing "map", and locking etc...

In general it seems to me a much better outcome would be to convince upstream, instead to create a permanent error table in the krb5 mechanism associated to permanent error strings, and stop completely doing any "dynamic" mapping. With the added value that an error can be retrieved even after the process is long gone if the logs happen to report just the minor.

But that is some surgery upstream in the upstream gssapi_krb5 code, so requires some discussion.

If upstream is resistant to that then we really should do some more advanced mapping in gss-proxy because otherwise we may not be able to return back status properly unless gss_dispay_status() is always called by the application immediately after the error is returned.

Ie things like:

 maj1 = gss_fn_1();
 if (maj1) {
    maj2 = gss_fn_2();
    if (maj2) {
       gss_display_status(..., maj1);
       gss_display_status(...., maj2);
    }
  }

will basically fail, as we do not store anywhere the custom minor error code message returned by the first function.