Skip to content

Conversation

brozs
Copy link

@brozs brozs commented Feb 15, 2025

This is a follow-up to b665b28.

An attacker that is able to login into a token could bypass authentication by using its own certificate with any valid signature.

This change makes the default "ca, signature" with the only way to disable CA check by using "no_ca".

This, however, also makes the "none" option disabling CRL and OCSP checks only.

Resolves #80

This is a follow-up to b665b28.

An attacker that is able to login into a token could bypass
authentication by using its own certificate with any valid signature.

This change makes the default "ca, signature" with the only way to
disable CA check by using "no_ca".

This, however, also makes the "none" option disabling CRL and OCSP
checks only.

Resolves OpenSC#80
@brozs
Copy link
Author

brozs commented Mar 13, 2025

Updated the commit with the suggestions, and made an attempt to update the documentation, too.

Copy link
Member

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

not sure why the ci fails though

@wolneykien
Copy link
Member

@frankmorgner , what do you think about the proposed change? You were the starter! :-)

IMHO, the change seems to be logical. However, if applied, the "none" policy becomes the synonym for all essential checks (and the default policy?) and then its name — "none" — sounds quite odd and misleading.

Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I personally would change the explanation of the none setting (in manual and example configuration) to something like:

# "none"         included for legacy reasons, is equivalent to "ca,signature"

This legacy option should then be moved to the bottom of the lists

@frankmorgner
Copy link
Member

I just realized that the code already has support for configuration.policy.ocsp_policy=OCSP_NONE;, which cannot be triggered on an individual basis. The same holds for configuration.policy.crl_policy=CRLP_NONE;. I would suggest to introduce the new configuration values no_clr and no_ocsp and then state that none translates to no_crl,no_ocsp, which hopefully makes it more clear.

@wolneykien
Copy link
Member

I personally would change the explanation of the none setting (in manual and example configuration) to something like:

# "none"         included for legacy reasons, is equivalent to "ca,signature"

This legacy option should then be moved to the bottom of the lists

Wow, I like the idea.

brozs added 2 commits May 8, 2025 17:01
This change allows to individualy set
configuration.policy.ocsp_policy=OCSP_NONE and
configuration.policy.crl_policy=CRLP_NONE by configuration.

Also further clarify the cert_policy="none" option in documentation.
@brozs
Copy link
Author

brozs commented May 8, 2025

I have added the no_crl and no_ocsp options and, hopefully, further improved the clarity of the documentation based on your feedback.

Please let me know if you want to add or change anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checking signature without checking CA is as bad as not checking the signature
4 participants