[sssd PR#5760][opened] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: opened

PR body:
"""
This restores the previous behavior of `--wait_for_card`, and fixes issues with
initialization and cleanup.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][synchronized] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][synchronized] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][synchronized] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][synchronized] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

sumit-bose commented:
"""
Hi,

thank you for the update, code-wise I'm fine with the patches, I'd like to run
some tests during the weekend and will give my final ACK early next week.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-907041154

[sssd PR#5760][edited] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: edited

 Changed field: body
Original value:
"""
This restores the previous behavior of `--wait_for_card`, and fixes issues with
initialization and cleanup.
"""

[sssd PR#5760][+Waiting for review] p11_child: Fixes for init_p11_ctx() and do_card()

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: +Waiting for review

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

sumit-bose commented:
"""
Hi,

my testing went well. Now wait_for_card is working with multiple empty readers again. As
before p11_child can only wait on a single reader, in the old version it was the first,
now it is the last. But since the user already had to guess which reader to use in this
case I think it is ok.

I added an in-line comment about casting to int, it would be nice if you can fix this.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-911798286

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

alexey-tikhonov commented:
"""
>   As before p11_child can only wait on a single reader, in the old
> version it was the first, now it is the last.
>  
@spoore1 , can this ^^ affect tests?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-911802829

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[spoore1]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

spoore1 commented:
"""
>  > As before p11_child can only wait on a single reader, in the old
> version it was the first, now it is the last.
>  
>  @spoore1 , can this ^^ affect tests?
>  
That's a good question.  

It's just a change in where p11_child waits for a reader to respond right?  

Could that change affect p11_child timeouts?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-911823713

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

sumit-bose commented:
"""
>  > > As before p11_child can only wait on a single reader, in
> the old version it was the first, now it is the last.
>  > 
>  > 
>  > @spoore1 , can this ^^ affect tests?
>  
>  That's a good question.
>  
>  It's just a change in where p11_child waits for a reader to respond right?
>  
Hi,

it is about required Smartcard authentication (p11_child is run with the --wait_for_card
option) when multiple Smartcard readers are connected but no card inserted in any of the
readers. Do you have such a test?

>  
>  Could that change affect p11_child timeouts?
>  
no

bye,
Sumit

"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-911836329

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[spoore1]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

spoore1 commented:
"""
>  > > > As before p11_child can only wait on a single reader,
> in the old version it was the first, now it is the last.
>  > > 
>  > > 
>  > > @spoore1 , can this ^^ affect tests?
>  > 
>  > 
>  > That's a good question.
>  > It's just a change in where p11_child waits for a reader to respond right?
>  
>  Hi,
>  
>  it is about required Smartcard authentication (p11_child is run with the --wait_for_card
> option) when multiple Smartcard readers are connected but no card inserted in any of the
> readers. Do you have such a test?
>  
No, I don't have any p11_child specific tests like that.   So I don't think this
will affect existing tests that I'm aware of.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-911844277

[sssd PR#5760][synchronized] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

dpward commented:
"""
I agree with the comments here, except to clarify:

>  As before p11_child can only wait on a single reader, in the old
> version it was the first, now it is the last.
>  
p11_child waits on an entire _PKCS#11 module_ — such as
[OpenSC](https://github.com/OpenSC/OpenSC/wiki) or
[YKCS11](https://developers.yubico.com/yubico-piv-tool/YKCS11/). Before it waited on the
first module; now it waits on the last. When calling `wait_for_card()`, note that `module`
is an input parameter, but `slot_id` and `info` are output parameters:
https://github.com/SSSD/sssd/blob/859221a9471863d626e473b124f51ee1cd62994...

Once a card/token is inserted, it will be used. It will not even check if it matches the
URI.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-912484215

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

dpward commented:
"""
I agree with the comments here, except to clarify:

>  As before p11_child can only wait on a single reader, in the old
> version it was the first, now it is the last.
>  
p11_child waits on an entire _PKCS#11 module_ — such as
[OpenSC](https://github.com/OpenSC/OpenSC/wiki) or
[YKCS11](https://developers.yubico.com/yubico-piv-tool/YKCS11/). Before it waited on the
first module; now it waits on the last. When calling `wait_for_card()`, note that `module`
is an input parameter, but `slot_id` and `info` are output parameters:
https://github.com/SSSD/sssd/blob/859221a9471863d626e473b124f51ee1cd62994...

Once a card/token is inserted into any reader of that module, it will be used. It will not
even check if the reader matches the URI.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-912484215

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[dpward]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

dpward commented:
"""
I agree with the comments here, except to clarify:

>  As before p11_child can only wait on a single reader, in the old
> version it was the first, now it is the last.
>  
p11_child waits on an entire _PKCS#11 module_ — such as
[OpenSC](https://github.com/OpenSC/OpenSC/wiki) or
[YKCS11](https://developers.yubico.com/yubico-piv-tool/YKCS11/). Before it waited on the
first module; now it waits on the last. When calling `wait_for_card()`, note that `module`
is an input parameter, but `slot_id` and `info` are output parameters:
https://github.com/SSSD/sssd/blob/859221a9471863d626e473b124f51ee1cd62994...

Once a card/token is inserted into any reader/slot of that module, it will be used. It
will not even check if the slot matches the URI.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-912484215

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

sumit-bose commented:
"""
Hi,

thanks for the update, the CI failures are not related, ACK.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-912757562

[sssd PR#5760][-Waiting for review] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: -Waiting for review

[sssd PR#5760][+Accepted] p11_child: Fixes for init_p11_ctx() and do_card()

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: +Accepted

[sssd PR#5760][+Ready to push] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: +Ready to push

[sssd PR#5760][comment] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5760

* `master`
    * a036fc871588372a75c2111cf2075453f4a3eb7f - p11_child: Fix printing of
non-null-terminated strings in do_card()
    * f5a9d8141b4af2ba05d6ef12e990aadea59c878c - p11_child: Return updated CK_SLOT_INFO
from wait_for_card()
    * 3f1d03fc6cc6179a52f31e96802f322e9e3ac0a9 - p11_child: Handle failure from
p11_kit_uri_new()
    * f3aa4b47a2f896c251802868583e8b6a499b21f4 - p11_child: Ensure OpenSSL cleanup is
performed
    * a9218fbe0b36ec2b1ffba70ba6028aabfcfe4ef8 - p11_child: Restore functionality of
--wait_for_card

"""

See the full comment at https://github.com/SSSD/sssd/pull/5760#issuecomment-913589768

[sssd PR#5760][+Pushed] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: +Pushed

[sssd PR#5760][-Accepted] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: -Accepted

[sssd PR#5760][-Ready to push] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5760
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()

Label: -Ready to push

[sssd PR#5760][closed] p11_child: Fixes for init_p11_ctx() and do_card()

[pbrezina]

   URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
 Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760