check for protected authentication path

[hypp]

This is a pull request for issue #7011

It checks if the flag CKF_PROTECTED_AUTHENTICATION_PATH is present,
and does C_Login with NULL if it is present, ignoring any pin passed from the user.

[sumit-bose]

Hi,

thank for the update, patches are working fine for me, ACK. But please squash the three patches into one before it will be committed.

For others who would like to test. With this patch the user will still be prompted for a PIN at the long prompt but at least when p11_child is doing the authentication this PIN is ignored if the reader has its own keypad. In my case the reader is indicating that a PIN input is expected, so the PIN has to be entered a second time on the keypad of the reader. Please note, I had to set enable_pinpad = true; in /etc/opensc-x86_64.conf in the app default reader_driver pcsc section, see man opensc.conf for details.

So, this is just the first step to fully integrated readers with keypads. Next would be to check for CKF_PROTECTED_AUTHENTICATION_PATH in the --pre run as well and return to the user that the PIN should be entered at the reader. Would you be interested to continue the work on this?

bye,
Sumit

[hypp]

Hi,

I'm a beginner at this, and I don't know how to squash commits. Could you please provide instructions?

I'd be happy to continue with the work on p11_child.

Regards,
/Mathias

[sumit-bose]

Hi,

I'm a beginner at this, and I don't know how to squash commits. Could you please provide instructions?

Hi,

if you call git rebase -i origin/master in your source tree an editor should be opened with the content like:

pick c4d72219f check for protected authentication path
pick 1aa52cdd1 fixed coding style
pick ff087435f removed unnecessary call to C_GetTokenInfo

# Rebase 098bf64a0..ff087435f onto 098bf64a0 (3 commands)
#
# Commands:
# p, pick <commit> = use commit
# r, reword <commit> = use commit, but edit the commit message
# e, edit <commit> = use commit, but stop for amending
# s, squash <commit> = use commit, but meld into previous commit
# f, fixup [-C | -c] <commit> = like "squash" but keep only the previous
#                    commit's log message, unless -C is used, in which case
#                    keep only this commit's message; -c is same as -C but
#                    opens the editor
# x, exec <command> = run command (the rest of the line) using shell
# b, break = stop here (continue rebase later with 'git rebase --continue')
...

If you now replace pick with fixup in the second and third line, write out and leave the editor, git should combine all three patches into one with the commit message of the first one.

I'd be happy to continue with the work on p11_child.

Great. As already mentioned, it will not only touch the p11_child code. But if you are interested I can add some comments, e.g. to #5371 about where I think changes might be needed.

bye,
Sumit

Regards, /Mathias

[hypp]

Hi,

hopefully I got the squashing correct.

Please give me some details on the things that need to be changed, so I can estimated the amount of work required.

Regards,
/Mathias

[alexey-tikhonov]

Would it be possible to just set pin=NULL/len=0 instead of copy&pasting entire block?

[hypp]

I tried that, but it didn't work for me.
Also the specification PKCS11 says "To log into a token with a protected authentication path, the pPin parameter to C_Login should be NULL_PTR."

[alexey-tikhonov]

I tried that, but it didn't work for me.

What exactly did you try?

Also the specification PKCS11 says "To log into a token with a protected authentication path, the pPin parameter to C_Login should be NULL_PTR."

But I don't propose anything different.

Just something like (schematically):

if (token_info.flags & CKF_PROTECTED_AUTHENTICATION_PATH) {
    DEBUG(SSSDBG_TRACE_ALL, "Protected authentication path, ignoring pin.\n");
    pin = NULL;
}
...
module->C_Login(session, CKU_USER, discard_const(pin), pin?strlen(pin):0);

[hypp]

I tried that, but it didn't work for me.

What exactly did you try?

I tried setting pin = NULL and calling C_Login similar to your example.
I could not get it to work, hence the copy and paste.

[alexey-tikhonov]

Could you show the patch that didn't work?

[hypp]

I agree, it should work, but I failed.
Feel free to change the code to your liking.

[alexey-tikhonov]

I don't have a device to test.

Can you check if #7064 still works for you?

copr builds are available here: https://copr.fedorainfracloud.org/coprs/g/sssd/pr7064/

[hypp]

I have tried it and it works for me. Great!

[alexey-tikhonov]

Ok, thanks for testing, then please either cherry-pick my additional patch or let's continue in #7064

[hypp]

Let's continue in #7064

[alexey-tikhonov]

Superseded by #7064