gssapi / gss-ntlmssp

A complete implementation of the MS-NLMP documents as a GSSAPI mechanism
ISC License
30 stars 26 forks source link

Debug-log for gssntlmssp cannot be collected for process with nonempty permitted capability set #53

Closed kvv81 closed 3 years ago

kvv81 commented 3 years ago

We use gssntlmssp from SMB server (NTLM authentication) and we need to enable logging feature for gssntlmssp. Logging is already supported here in some basic way via GSSNTLMSSP_DEBUG environment variable. However in the code 'secure_getenv' function is used to fetch it:

void gssntlm_debug_init(void)
{
...
    env = secure_getenv("GSSNTLMSSP_DEBUG");

From 'man secure_getenv':

The  GNU-specific secure_getenv() function is just like getenv() except that it returns NULL in cases where "secure execution" is required.
...
*  the process has a nonempty permitted capability set

Alas, in SMB server we need to listen SMB port 445 (lower port < 1024) so cap_net_bind_service is specially set for this. Bottom line - our process can't enable logging in any way because secure_getenv always returns NULL. We need some optional way to enable logging.

simo5 commented 3 years ago

The debug log can reveal security relevant information, and we do not want to accidentally do that in a setuid binary or similar, which is why secure_getenv() is used. I am open to ideas on how to handle this. Would your case be satisfied by providing a public callable symbol to turn on debugging ?

You would have to use dlsym() to make sure the symbol is currently available in the process to avoid a potential crash if the plugin is not actually loaded, but other than that it would give direct control to the calling binary w/o depending on an environment variable.

simo5 commented 3 years ago

Alternatively I think we could provide a facility via a gss_set_sec_context_option() call ?

simo5 commented 3 years ago

Uhm the problem with gss_set_sec_context_option() is that it requires a context object, so it would come too late ..

simo5 commented 3 years ago

A gss_set_cred_option() call could be used too ... not ideal but at least can be set before a ISC/ASC operation is performed by acquiring the credentials before hand ...

kvv81 commented 3 years ago

I'm afraid that using dlopen/dlsym for direct access to gssntlmssp library .so breaks the whole concept of GSSAPI.

What we actually do in our code is linking with '-lgssapi_krb5' library (MIT Kerberos GSSAPI library) and we use GSSAPI provided by it. Our binary doesn't do anything to load gssntlmssp, this is done internally by MIT library (i.e. gssntlmssp acts like a plugin of public GSSAPI to provide NTLM auth additionally to Kerberos). Moreover, the path to gssntlmssp.so is written in /etc/gss/mech.d/ntlmssp.conf (not hardcoded) and we don't want to duplicate the logic of GSSAPI to read and parse this file during our initialization.

We want to have more convenient public API for this; using dlsym looks like a hack...

kvv81 commented 3 years ago

The option with calling gss_set_cred_option() looks much better from API point of view.

I would refactored/split gssntlm_debug_init code to accept new path of log-file (NULL if should be disabled) and would make it re-enetable (current code never closes debug_fd and does not free associated resources). In case if debug_fd is already opened then gss_set_cred_option should make a transaction to open a new file and update debug_fd atomically (it's a pointer, we can rely on CPU atomic updates of machine-word variable) and then free old resources if needed.

simo5 commented 3 years ago

Ok, I need to carefully think about this. Esp in terms of how it will work once (one day) we finally get gss_create_sec_context() and will be finally able to set context options.

My intention with proposing context/cred options was to then tied debug logs to only calls that use those creds, and if a context is created from the creds then "inherit" the debug setting on the context.

Ie my intention is not to abuse a gss_set_cred_option() to set a global variable or even a per-thread variable.

Do these considerations change in any way your opinion on the viability of using gss_set_cred_option() ?

kvv81 commented 3 years ago

For our specific use-case we call gss_acquire_cred to fetch SMB server credentials and after that we use it for gss_accept_sec_context every time we need authentication via GSS. We call gss_acquire_cred during initialization of SMB server (usually done only once on the very start) so calling gss_set_cred_option doesn't change our main flow (not needed for each individual authentication). Having that, binding debugging log to the context of gss_cred_id_t is Ok for us. We don't care too much about other GSS calls outside of server credentials.

But from other side, if the problem happens in GSS layer BEFORE successful call of gss_accept_sec_context/gss_set_cred_option, we will be not able to see anything in logs (because they are not enabled yet); so the feature of 'global variable' introduced by gssntlm_debug_init is still useful in general case.

simo5 commented 3 years ago

Ok so I think there has been a previous misunderstanding that perhaps will make this simpler for you. I mentioned exposing a symbol from gssntlmssp that you could use directly to change debug level. In the same message I also said "You would have to use dlsym() to make sure the symbol is currently available" I think this mislead you to think you always have to use dlsym(0 to access this symbol. This is not the case. You can directly link to that symbol (which will be exposed via gssntlmssp.h as well, you would have to use dlsym() only if you need to support old and new versions of gssntlmssp with the same code, otherwise all you need to do is just guarantee that you are using a recent enough gssntlms shared object and that libgssapi has already been initialized. You can enforce that by querying the available mechanisms via a call like: gss_indicate_mechs() to insure the gssntlmssp module is available.

All that said I think we could aso use gssspi_mech_invoke() (sepxorted by gssapi_ext.h) to call a mechanism specific function. This is the safer option because it will simply return an error if the mechanism does not exist or does not expose gssspi_mech_invoke as a function. gssntlmssp currently does not implement this function, but it can be realtively easily be addedd.

kvv81 commented 3 years ago

Well, we don't need to support old and new versions of gssntlmssp with the same code. So exporting new function via gssntlmssp.h is a good option for us (and we don't need to parse ntlmssp.conf and call dlopen/dlsym). As for specific API - we just would like to keep this way simple enough - as before. Writing much new code for such a trivial feature (that already present in other non-suid applications) is not something I can easily explain to my manager when planning efforts for tasks :-)