Skip to content
New issue

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

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

Already on GitHub? # to your account

Document peer certificate fingerprinting #1170

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cmouse
Copy link
Contributor

@cmouse cmouse commented Feb 12, 2025

JIRA: DOV-7328

values: setting_types.STRING,
text: `
An OpenSSL digest algorithm name to use to hash peer certificate names.
Setting this value enables \`ssl_client_cert_fp\` and \`ssl_client_cert_pubkey_fp\`
Copy link
Contributor

Choose a reason for hiding this comment

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

enables [[link,passdb_check_client_fp,ssl_client_cert_fp and ssl_client_cert_pubkey_fp]]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

um, no, those are variables, not extra fields

Copy link
Contributor

Choose a reason for hiding this comment

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

Should say %{ssl_client_cert_fp} and %{ssl_client_cert_pubkey_fp} variables then

Also, I thought this setting was required for check_client_fp to work? That should be mentioned/linked in another sentence then.


##### `check_client_cert_fp`

Match client certificate fingerprint. See [[setting,ssl_peer_certificate_fingerprint_hash]].
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

  • Match client certificate (but not public key) fingerprint.
  • See [[link,passdb_check_client_fp]]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think client certificate contains public key always, so it is implicitly checked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I intended to differentiate the different check_client_*_fp fields with that comment. If it public key fingerprint is somehow implicitly checked, how are check_client_cert_fp and check_client_fp any different?


This is intended to be used instead of verifying certificates with CA certificates. If you are using CA certificates,
you can only fail authentication if the value is non-empty and does not match. If CA certificates are not used, then
if no passdb matches the fingerprint, then authentication is failed.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's a bit strange text. "With CA authentication can only fail, but with fingerprints authentication is failed." so kind of the same. Also now it sounds like this is the main reason for using fingerprints. Should it be rather something like:

This can be used for verifying client certificates using per-user fingerprints stored in a database rather than relying on CA certificates.

If the fingerprints don't match, it's treated as a passdb failure ([[setting,passdb_result_failure]]). This allows the next passdb to continue authenticating the user in a different way. If CA certificates are used, the authentication can only fail without a fallback.

Although now I'm a bit confused about your sentence above "you can only fail authentication if the value is non-empty and does not match". Do you mean if CA certificates are used together with check_client_fp? Is there some point in doing that? Or what is this "value is non-empty" you refer to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i clarified the text a bit. the only point is to do public key pinning, so you can be sure that the client is not using a cert with similar username but issued by some other CA or reissued to different public key.

::warning If CA certificates are not used, a passdb must provide a valid check_client_fp (or variant) to validate the
client certificate. If none is provided, the authentication will fail.

This also requires [[setting,auth_ssl_require_client_cert,yes]] to work.
Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn't seem to be true. As long as a certificate is provided, passdb passes even with auth_ssl_require_client_cert=no. Should we change it in the code that it really is required to prevent misconfigurations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if auth_ssl_require_client_cert, then nothing enforces that the cert is valid.

you can only fail authentication if the value is non-empty and does not match. If CA certificates are not used, then
if no passdb matches the fingerprint, then authentication is failed.

See [[setting,ssl_server_request_client_cert]] on how to allow any certificate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we need a clear bulletin point list what is required for this to work as expected:

  • [[setting,auth_ssl_require_client_cert,yes]]
  • [[setting,ssl_server_request_client_cert,any-cert]] to skip CA certificate checks
  • passdb check_client_fp, check_client_cert_fp or check_client_publkey_fp where the value contains the user's client certificate's fingerprint

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

Successfully merging this pull request may close these issues.

2 participants