Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

p11_child: do_card partially fix loop exit condition (redo of #5705) #5746

Closed
wants to merge 1 commit into from
Closed

Conversation

assafmo
Copy link
Contributor

@assafmo assafmo commented Aug 15, 2021

This PR 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 PR partially fixes issue #5025, thanks to this comment by @sumit-bose: #5025 (comment)

:relnote: p11_child does not stop at the first empty slot when searching for tokens

This PR is a redo of PR #5705, because it was orphaned and changes were requested by the maintainers.

…or token

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 partially fixes issue #5025,
thanks to this comment by @sumit-bose: #5025 (comment)

:relnote: p11_child does not stop at the first empty slot when searching for tokens

Co-Authored-By: Sumit Bose <sbose@redhat.com>
@sumit-bose
Copy link
Contributor

Hi,

thanks for the patch, the commit message is looking good now. ACK

bye,
Sumit

@pbrezina
Copy link
Member

Pushed PR: #5746

  • master
    • b9f8c2f - p11_child: do_card partially fix loop exit condition when searching for token

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Aug 23, 2021
@pbrezina pbrezina closed this Aug 23, 2021
@dpward
Copy link
Contributor

dpward commented Aug 23, 2021

FYI this change breaks some existing assumptions in the code after it

When this code is reached:

if (modules[c] == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "No removable slots found.\n");
ret = EIO;
goto done;
}

it will now exit if it did not find a removable slot with a token in it. So the wait_for_card handling right after it will never get executed:
if (!(info.flags & CKF_TOKEN_PRESENT)) {
DEBUG(SSSDBG_TRACE_ALL, "Token not present.\n");
if (p11_ctx->wait_for_card) {
ret = wait_for_card(modules[c], &slot_id);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "wait_for_card failed.\n");
goto done;
}
} else {
ret = EIO;
goto done;
}
}

@sumit-bose
Copy link
Contributor

Hi,

thanks for the hint, I have to admit that I didn't test the patch with wait_for_card.

@assafmo, would you like to add a patch that fixes this?

bye,
Sumit

@assafmo
Copy link
Contributor Author

assafmo commented Sep 4, 2021

Honestly I don't feel comfortable tinkering with this coee any more then I did. I can confirm that this patch did solve my issue and I was able to test it on multiple setups.

@dpward
Copy link
Contributor

dpward commented Sep 5, 2021

@assafmo Please see commit a9218fb.

pbrezina pushed a commit that referenced this pull request Sep 6, 2021
Previously, the loop in do_card() would find the first PKCS#11 slot with
support for removable tokens, whether or not a token was present. If one
was not, and --wait_for_card was specified, then it would wait for a token
to be inserted in this slot (or any slot in the same PKCS#11 module).

Commit b9f8c2f ("p11_child: do_card partially fix loop exit condition
when searching for token") changed the loop so it finds the first PKCS#11
slot that has a removable token present. Adjust this to allow the existing
handling of --wait_for_card to work when no token is found. Fixes #5746.

Reviewed-by: Sumit Bose <sbose@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants