[sssd PR#5705][opened] p11_child: do_card fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Author: assafmo
 Title: #5705: p11_child: do_card fix loop exit condition
Action: opened

PR body:
"""
This commit fixes the exit condition when searching for a token in p11_child/do_card, specifically in case a token is present in a slot, but there are empty slots before it.

This commit fixes issue #5025, thanks to this comment by @sumit-bose: https://github.com/SSSD/sssd/issues/5025#issuecomment-801842175

Closes #5025 
"""

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

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[dpward]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

dpward commented:
"""
Can we please remove the Closes: tag? This PR addresses one specific case, but doesn't address the full scope of the problem described in #5025.
"""

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

[sssd PR#5705][edited] p11_child: do_card fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Author: assafmo
 Title: #5705: p11_child: do_card fix loop exit condition
Action: edited

Changed field: body
Original value:
"""
This commit fixes the exit condition when searching for a token in p11_child/do_card, specifically in case a token is present in a slot, but there are empty slots before it.

This commit fixes issue #5025, thanks to this comment by @sumit-bose: https://github.com/SSSD/sssd/issues/5025#issuecomment-801842175

Closes #5025 
"""

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

assafmo commented:
"""
> Can we please remove the Closes: tag? This PR addresses one specific case, but doesn't address the full scope of the problem described in #5025.

Yes, done.
"""

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

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[dpward]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

dpward commented:
"""
Can we please remove the Closes: tag? This PR addresses one specific case, but doesn't address the full scope of the problem described in #5025. See comment: https://github.com/SSSD/sssd/issues/5025#issuecomment-802072952
"""

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

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

assafmo commented:
"""
Also, the most recent version for Debian is v2.2.3 (8607b4822e4b6437d87dabf714882407f8959ef2), so here's the fix for it:

```diff
[p11_child.patch.txt](https://github.com/SSSD/sssd/files/6820708/p11_child.patch.txt)
[p11_child.patch.txt](https://github.com/SSSD/sssd/files/6820710/p11_child.patch.txt)

diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c
index d3064ff..a7520df 100644
--- a/src/p11_child/p11_child_nss.c
+++ b/src/p11_child/p11_child_nss.c
@@ -595,7 +595,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
                       (slInfo.flags & CKF_REMOVABLE_DEVICE) ? "true": "false",
                       (slInfo.flags & CKF_TOKEN_PRESENT) ? "true": "false");

-                if (rv == SECSuccess && (slInfo.flags & CKF_REMOVABLE_DEVICE)) {
+                if (rv == SECSuccess && (slInfo.flags & CKF_REMOVABLE_DEVICE) && (slInfo.flags & CKF_TOKEN_PRESENT)) {
                     slot = PK11_ReferenceSlot(mod_list_item->module->slots[s]);
                     break;
                 }
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 56601b1..586af33 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1714,7 +1714,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,

}

-                if ((info.flags & CKF_REMOVABLE_DEVICE)) {
+                if ((info.flags & CKF_REMOVABLE_DEVICE) && (info.flags & CKF_TOKEN_PRESENT)) {
                     break;
                 }
             }
```
"""

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

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

alexey-tikhonov commented:
"""
> Also, the most recent version for Debian is v2.2.3 ([8607b48](https://github.com/SSSD/sssd/commit/8607b4822e4b6437d87dabf714882407f8959ef2)), so here's the fix for it:
> diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c

IIUC, `sid` and `bullseye` use 2.4.1 that should use openssl.
`buster` uses 1.16.3, probably with nss, but frankly status of 1.16 upstream branch doesn't expect to recv this kind of updates.

As for PR, it will have to wait @sumit-bose' approval. Sorry for a delay.
"""

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

[sssd PR#5705][comment] p11_child: do_card fix loop exit condition

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card fix loop exit condition

sumit-bose commented:
"""
Hi,

thanks for the patch, I agree with the change. I would like to ask you to update the commit message a bit.

First, please says `partially fixes issue #5025` or similar.

Second, I think it would be good to add a `:relnote:` tag with something like 'p11_child does not stop at the first empty slot when searching for tokens` to make sure this change gets mentioned in the release notes.

bye,
Sumit
"""

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

[sssd PR#5705][edited] p11_child: do_card partially fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Author: assafmo
 Title: #5705: p11_child: do_card partially fix loop exit condition
Action: edited

Changed field: title
Original value:
"""
p11_child: do_card fix loop exit condition
"""

[sssd PR#5705][comment] p11_child: do_card partially fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card partially fix loop exit condition

assafmo commented:
"""
@sumit-bose Unfortunately I accidentally made this PR an orphan, so I cannot update the commit or push new commits. What do you want me to do?
"""

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

[sssd PR#5705][comment] p11_child: do_card partially fix loop exit condition

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card partially fix loop exit condition

sumit-bose commented:
"""
> @sumit-bose Unfortunately I accidentally made this PR an orphan, so I cannot update the commit or push new commits. What do you want me to do?

Hi,

if you cannot push to this PR anymore I guess the most easy solution is to close this one and open a new one.

bye,
Sumit
"""

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

[sssd PR#5705][closed] p11_child: do_card partially fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Author: assafmo
 Title: #5705: p11_child: do_card partially fix loop exit condition
Action: closed

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

[sssd PR#5705][comment] p11_child: do_card partially fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card partially fix loop exit condition

assafmo commented:
"""
See #5746 for resolution of this PR.
"""

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

[sssd PR#5705][comment] p11_child: do_card partially fix loop exit condition

[assafmo]

URL: https://github.com/SSSD/sssd/pull/5705
Title: #5705: p11_child: do_card partially fix loop exit condition

assafmo commented:
"""
Closing, please see #5746 for resolution of this PR.
"""

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