Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: main
Are you sure you want to change the base?
Document peer certificate fingerprinting #1170
Changes from all commits
d0bd021
03e045c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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]]
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
thenAlso, I thought this setting was required for check_client_fp to work? That should be mentioned/linked in another sentence then.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
See [[link,passdb_check_client_fp]]
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?