Skip to content

Pkcs11 soft ocsp tests#8557

Open
krishnavema wants to merge 2 commits intoSSSD:masterfrom
krishnavema:pkcs11-soft-ocsp-tests
Open

Pkcs11 soft ocsp tests#8557
krishnavema wants to merge 2 commits intoSSSD:masterfrom
krishnavema:pkcs11-soft-ocsp-tests

Conversation

@krishnavema
Copy link
Copy Markdown
Contributor

@krishnavema krishnavema commented Mar 27, 2026

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses RHEL-5043 by improving OCSP timeout handling in p11_child. It introduces a default timeout for soft_ocsp requests to prevent indefinite blocking and adjusts the OCSP deadline calculation to use half of the total allocated timeout, providing a buffer for result processing. Additionally, a new suite of system tests has been added to verify smart card authentication behavior under various OCSP responder availability scenarios. I have no feedback to provide.

@alexey-tikhonov
Copy link
Copy Markdown
Member

@krishnavema, @spoore1, what branches does this target?

Copy link
Copy Markdown
Contributor

@thalman thalman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests and change looks good, but the requirements should not point to private fork

git+https://github.com/next-actions/pytest-tier
git+https://github.com/next-actions/pytest-output
git+https://github.com/SSSD/sssd-test-framework
#git+https://github.com/SSSD/sssd-test-framework
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be reverted before merging

Copy link
Copy Markdown
Contributor

@spoore1 spoore1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main thing I think is to move the tests into the existing test_smartcard.py file and use it's helper functions. Besides that, mostly questions for clarification.

Comment thread src/p11_child/p11_child_openssl.c Outdated
Comment thread src/p11_child/p11_child_openssl.c Outdated
Comment thread src/tests/system/tests/test_smartcard_soft_ocsp.py Outdated
Comment thread src/tests/system/tests/test_smartcard_soft_ocsp.py Outdated
time.sleep(VIRT_CACARD_SETTLE_SECONDS)


def _assert_smartcard_auth_success(client: Client, username: str) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you write an authentication util for su for this in another PR? I think that should be used here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there is no existing util , do you mean somewhere in specific ?

Copy link
Copy Markdown
Contributor

@thalman thalman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scott's questions made me go through the code once more and I think that this is bad approach. See my comment.

Comment thread src/p11_child/p11_child_openssl.c
…mart card authentication (resolves: RHEL-5043)
@krishnavema krishnavema force-pushed the pkcs11-soft-ocsp-tests branch from 8845bce to 49f5b90 Compare April 17, 2026 16:50
@krishnavema krishnavema requested review from spoore1 and thalman April 17, 2026 16:59
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.

4 participants