ovis
ovis copied to clipboard
Default permsissions in ldmsd
It would be very nice to have a way to set default permissions one time in ldmsd that are used by all metrics. @baallan suggested having environment variables to pass those values to ldmsd. That seems reasonable, and consistent with current configuration of ldmsd (the other option being command line options).
So I would propose that we create following option environment variables:
LDMSD_AUTH_UID
LDMSD_AUTH_GID
LDMSD_AUTH_PERM
I would like to see these values automatically pre-set as the default values in the ldms_set_t when ldms_set_new() is called.
There are currently two ways that plugin can set uid/gid/perm:
ldms_set_t ldms_set_new_with_auth(const char *instance_name,
ldms_schema_t schema,
uid_t uid, gid_t gid, mode_t perm);
int ldms_set_config_auth(ldms_set_t set, uid_t uid, gid_t gid, mode_t perm);
There is a compatibility issue between these two functions and the proposed defaults. One, two, or all three of uid, gid, and perm to be specified on a plugin's configuration line. These two functions only permit all three to be set at once. This would not allow a plugin to accept ldmsd's defaults for the values it does not want to set.
So first of all, I would recommend that we deprecate ldms_set_new_with_auth(), because the only case in which that would be useful when defaults are added is if the plugin is going to set all three of the options. I see no issue with calling "ldms_set_new()" followed by a call, or calls, to ldms_set_config_auth*() functions. Presumably the new set will not be visible to consumers until ldms_set_publish() is called, so there is no risk of the metric being temporarily exposed with incorrect permissions between ldms_set_new() and ldms_set_config_auth*().
Next, I see two main solutions for ldms_set_config_auth():
- Introduce an ldms_set_config_auth_get() function. This would allow the plugin to read the default values and supply those to ldms_set_config_auth() for any options it does not choose to override.
- Split ldms_set_config_auth() into three separate functions: ldms_set_config_auth_uid(), ldms_set_config_auth_gid(), and ldms_set_config_auth_perm().
It looks like sampler_base.c already uses ldms_set_new() and ldms_set_config_auth() separately rather than using ldms_set_new_with_auth(), and only four other samplers call lmds_set_with_new_auth(), so this is probably a pretty low impact change to implement.
It seems like a reasonable extension to put this on the command line as well would be; -a perm=string where we can argue about whether string is something like 600 or the full gnu chmod perm syntax in a separate thread. I love having things NOT require env vars to be controllable.
RE the C api, perhaps modify set_new_with_auth(..., uid, gid, perm) to set_new_with_auth(..., struct auth_data *d); and a helper function:
xxx_set_auth_data_set(struct auth_data *data, uid, gid, perm, int adflags)
#define AD_PERM 1
#define AD_UID 2
#define AD_GID 4
The proliferation of getter/setter is somewhat annoying.