kafs-client (recent strangeness of with add_key)

[Jeffrey E Altman]

Hello Spencer,

The change "remove -d (debug) option and improve -v (verbose) option" 
should in my opinion be inverted in order to improve compatibility with 
the OpenAFS aklog:

Usage: aklog [-d] [[-cell | -c] cell [-k krb_realm]] [[-p | -path] pathname]
     [-zsubs] [-hosts] [-noauth] [-noprdb] [-force] [-setpag]
     [-linked] [-insecure_des] [-524]

The various pam implementations that call an external aklog assume the 
OpenAFS command line interface.  Enabling verbose output is performing 
by specifying "-d".

I agree that there is little benefit to separate "-d" and "-v" options 
but I believe "-v" should be removed or that they should be synonyms.


In the OpenAFS aklog "-k" is used to specify the Kerberos realm to use 
for the preceding cell.   Note that aklog can be called with multiple 
cells each of which can be optionally bound to a Kerberos realm.  If a 
cell is not followed by "-k realm" then the default realm logic is 
used.   Using "-k" to specify the keyring type is likely to cause 
conflict.   Again, pam modules that execute an external aklog use -k to 
specify the Kerberos realm. --keyring ?

There is benefit to being able to specify the keyring to use. Might 
there be benefit to being able to specify an explicit keyring to use by 
name instead of the keyring type?


There is no benefit to implementing -zsubs, -hosts, -noauth, -524 
-insecure_des.

The -p pathname functionality is useful for obtaining tokens for $HOME 
directories.   A token is obtained for each cell crossed during the 
evaluation of a path.

The -linked option indicates that tokens should be obtained not only for 
a cell whether explicitly specified on the command line or derived from 
a path but should also obtain the tokens for the linked cells.   
Personally I think -linked should be enabled by default.   A linked cell 
is a cell an alternate cell to search for a volume name or id if the 
primary cell's location service does not include a volume of that name 
or id.   Linked cells are useful for building test environments where 
newer versions of volumes are placed into a cell and then the remaining 
production volumes are sourced from a linked cell.   Another use case is 
cell renaming where a volumes are migrated from the old cell to the new 
cell. Cache managers implement the linked cell behavior unconditionally 
so aklog should probably do so as well.

The -noprdb option is not relevant to kafs aklog at the moment because 
kafs aklog does not attempt to automatically register the user in a 
foreign cell nor does it query the token owner's PTS ID.

In all cases it might be a good idea to accept the unimplemented command 
line options and ignore them.

Other than these comments I believe the stack looks good.

Thank you for the follow up.

Jeffrey Altman

On 11/21/2022 10:00 AM, Spencer Olson (olsonse at umich.edu) wrote:
> I've been using this patched version without any issues for a while
> now such that I forgot that there was an issue (until a recent system
> upgrade).
>
> To make my changes easier to digest, I've split my commits on my
> github repository into two branches:
>    master  branch now only has changes to the base package.
>                 This includes several fixes and adds some other command
>                  line options for verbosity
>    debian  branch now has my debian build configuration rebased on
>                  the new master branch
>
> David:  I'd like to request you to do a pull from:
>      https://github.com/olsonse/kafs-client  master
> to get the changes I've made to the base package.  Let me know if  you
> really want me to send you a patch file for these various changes.
>
> Bill:  I'd like to request that you pull these same base package
> changes in for the debian package that you are maintaining.
>
> Spencer
>
>
> On Fri, Jun 3, 2022 at 3:30 PM Spencer Olson<olsonse at umich.edu>  wrote:
>> Ok, so I took the suggestions from Chaskiel, implemented them and
>> pushed my changes to my repository on GitHub.  Using this right now
>> with no issues yet.
>>
>> On Tue, May 31, 2022 at 8:24 AM Chaskiel Grundman<cgrundman at gmail.com>  wrote:
>>> The KEY_SPEC_USER_SESSION_KEYRING exists to be the fallback
>>> KEY_SPEC_SESSION_KEYRING.
>>>
>>>  From user-session-keyring.7:
>>>> The user session keyring is created on demand when a thread requests it or when a thread asks for its session-keyring(7) and that keyring doesn't exist. In the latter case, a user session keyring will be created and, ***if the session keyring wasn't to be created, the user session keyring will be set as the process's actual session keyring***
>>> You can try it yourself, with keyctl show
>>> Compare
>>> keyctl show @s
>>> keyctl show @us
>>> and keyctl session -- keyctl show @s
>>>
>>> the strace shows (session present)
>>> keyctl(KEYCTL_GET_KEYRING_ID, KEY_SPEC_SESSION_KEYRING, 0) = 66825801
>>> keyctl(KEYCTL_DESCRIBE, 66825801, NULL, 0) = 32
>>> keyctl(KEYCTL_DESCRIBE, 66825801, "keyring;1000;1000;3f030000;_ses", 32) = 32
>>>
>>> vs (session not present)
>>> keyctl(KEYCTL_GET_KEYRING_ID, KEY_SPEC_SESSION_KEYRING, 0) = 634641843
>>> keyctl(KEYCTL_DESCRIBE, 634641843, NULL, 0) = 42
>>> keyctl(KEYCTL_DESCRIBE, 634641843,
>>> "keyring;1000;65534;1f3f0000;_uid"..., 42) = 42
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4039 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-afs/attachments/20221121/ed99cde8/attachment.p7s>