On Aug 4, 2015, at 9:29 AM, Steve Dickson <SteveD@xxxxxxxxxx> wrote:
> Hello,
>
> On 07/31/2015 06:13 PM, Chuck Lever wrote:
>> User space can see the keys, but not their contents.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> utils/nfsidmap/nfsidmap.c | 85 ++++++++++++++++++++++++++++++++++++++++++-
>> utils/nfsidmap/nfsidmap.man | 15 ++++++++
>> 2 files changed, 98 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
>> index 44b8b4b..ed397c6 100644
>> --- a/utils/nfsidmap/nfsidmap.c
>> +++ b/utils/nfsidmap/nfsidmap.c
>> @@ -108,6 +108,81 @@ static int display_default_domain(void)
>> return EXIT_SUCCESS;
>> }
>>
>> +static void list_key(key_serial_t key)
>> +{
>> + char *buffer, *c;
>> + int rc;
>> +
>> + rc = keyctl_describe_alloc(key, &buffer);
>> + if (rc < 0) {
>> + switch (errno) {
>> + case EKEYEXPIRED:
>> + printf("Expired key not displayed\n");
>> + break;
>> + default:
>> + xlog_err("Failed to describe key: %m");
>> + }
>> + return;
>> + }
>> +
>> + c = strrchr(buffer, ';');
>> + if (!c) {
>> + xlog_err("Unparsable key not displayed\n");
>> + goto out_free;
>> + }
>> + printf(" %s\n", ++c);
>> +
>> +out_free:
>> + free(buffer);
>> +}
>> +
>> +static void list_keys(const char *ring_name, key_serial_t ring_id)
>> +{
>> + key_serial_t *key;
>> + void *keylist;
>> + int count;
>> +
>> + count = keyctl_read_alloc(ring_id, &keylist);
>> + if (count < 0) {
>> + xlog_err("Failed to read keyring %s: %m", ring_name);
>> + return;
>> + }
>> + count /= (int)sizeof(*key);
>> +
>> + switch (count) {
>> + case 0:
>> + printf("No %s keys found.\n", ring_name);
>> + break;
>> + case 1:
>> + printf("1 %s key found:\n", ring_name);
>> + break;
>> + default:
>> + printf("%u %s keys found:\n", count, ring_name);
>> + }
>> +
>> + for (key = keylist; count--; key++)
>> + list_key(*key);
>> +
>> + free(keylist);
>> +}
>> +
>> +/*
>> + * List all keys on a keyring
>> + */
>> +static int list_keyring(const char *keyring)
>> +{
>> + key_serial_t key;
>> +
>> + key = find_key_by_type_and_desc("keyring", keyring, 0);
>> + if (key == -1) {
>> + xlog_err("'%s' keyring was not found.", keyring);
>> + return EXIT_FAILURE;
>> + }
>> +
>> + list_keys(keyring, key);
>> + return EXIT_SUCCESS;
>> +}
>> +
>> /*
>> * Find either a user or group id based on the name@domain string
>> */
>> @@ -277,7 +352,7 @@ int main(int argc, char **argv)
>> int timeout = 600;
>> key_serial_t key;
>> char *progname, *keystr = NULL;
>> - int clearing = 0, keymask = 0, display = 0;
>> + int clearing = 0, keymask = 0, display = 0, list = 0;
>>
>> /* Set the basename */
>> if ((progname = strrchr(argv[0], '/')) != NULL)
>> @@ -287,11 +362,14 @@ int main(int argc, char **argv)
>>
>> xlog_open(progname);
>>
>> - while ((opt = getopt(argc, argv, "du:g:r:ct:v")) != -1) {
>> + while ((opt = getopt(argc, argv, "du:g:r:ct:vl")) != -1) {
>> switch (opt) {
>> case 'd':
>> display++;
>> break;
>> + case 'l':
>> + list++;
>> + break;
>> case 'u':
>> keymask = UIDKEYS;
>> keystr = strdup(optarg);
>> @@ -328,6 +406,9 @@ int main(int argc, char **argv)
>>
>> if (display)
>> return display_default_domain();
>> + else if (list)
>> + return list_keyring(DEFAULT_KEYRING);
>> +
> Just curious as to why the else clause... Are you
> thinking it does not make sense to display the
> domain and list out the key ring?
Well, display already has a hard "return display_default_domain()".
If you specify "-l" you're not getting anything else. The "else"
can probably be removed.
The idea is that "nfsidmap -l" or "nfsidmap -d" can be used
in scripts too. In that context, specifying both would make
parsing the output more difficult. And I think you would use
these in different situations. "-l" might be used a lot. "-d"
might be used once to check the configuration.
And IMO these new options should be careful to avoid updating
or revoking any key, so the logic exits right there, as a
defensive precaution.
We're overloading a kernel API here, after all. Maybe this kind
of debugging belongs in a separate command. But this seems safe
enough.
> steved.
>
>> if (keystr) {
>> rc = key_invalidate(keystr, keymask);
>> return rc;
>> diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
>> index 04ddff6..0275bdf 100644
>> --- a/utils/nfsidmap/nfsidmap.man
>> +++ b/utils/nfsidmap/nfsidmap.man
>> @@ -13,6 +13,8 @@ nfsidmap \- The NFS idmapper upcall program
>> .B "nfsidmap [-v] [-u|-g|-r user]"
>> .br
>> .B "nfsidmap -d"
>> +.br
>> +.B "nfsidmap -l"
>> .SH DESCRIPTION
>> The NFSv4 protocol represents the local system's UID and GID values
>> on the wire as strings of the form
>> @@ -50,6 +52,13 @@ can also clear cached ID map results in the kernel,
>> or revoke one particular key.
>> An incorrect cached key can result in file and directory ownership
>> reverting to "nobody" on NFSv4 mount points.
>> +.PP
>> +In addition, the
>> +.B -d
>> +and
>> +.B -l
>> +options are available to help diagnose misconfigurations.
>> +They have no effect on the keyring containing ID mapping results.
>> .SH OPTIONS
>> .TP
>> .B -c
>> @@ -62,6 +71,12 @@ Display the system's effective NFSv4 domain name on
>> .B -g user
>> Revoke the gid key of the given user.
>> .TP
>> +.B -l
>> +Display on
>> +.I stdout
>> +all keys currently in the keyring used to cache ID mapping results.
>> +These keys are visible only to the superuser.
>> +.TP
>> .B -r user
>> Revoke both the uid and gid key of the given user.
>> .TP
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
