Skip to content

Bite the bullet, and attempt something close to documentation for X509_verify_cert. - #31314

Closed
bob-beck wants to merge 5 commits into
openssl:masterfrom
bob-beck:x509-verify-cert-docs
Closed

bob-beck wants to merge 5 commits into
openssl:masterfrom
bob-beck:x509-verify-cert-docs

Conversation

@bob-beck

@bob-beck bob-beck commented May 28, 2026

Copy link
Copy Markdown
Contributor

I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details - I can't do that
without something like a real base to start from, and I don't want to both
rewrite the world and introduce my changes at the same time. So before
changing anything, let's put this in a state that talks about what it does
today. This is an attempt to get this documentation much closer to something
useful for these functions, as they work today.

So to summarize the changes:

  1. This documents the current way that X509_verify_cert goes about
    building and verifying a chain, identifying the relevant flags that
    can affect how this happens today, and cross references the correct page
    to find the rest of the exhaustive list of verification flags and what they do.
    It calls out the major salient differences in path building and verification
    from the standards and where the differences can cause X509_verify_cert
    to fail to find or verify a path where one may actually exist according to
    the standards.

  2. This documents the verification callback and how it can be used
    to affect the verification outcome, This includes warning potential
    users using the callback of what returning values from it means,
    the common ways to end up unintentionaly doing things you did not expect
    with the callback, and that it depends upon internals and should not be
    relied upon.

Checklist
  • documentation is added or updated
  • tests are added or updated

@github-actions github-actions Bot added the severity: fips change The pull request changes FIPS provider sources label May 28, 2026
Comment thread util/libcrypto.num Outdated
@bob-beck
bob-beck force-pushed the x509-verify-cert-docs branch from 7dc4cf5 to cc6fb06 Compare May 28, 2026 10:30
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label May 28, 2026
@github-actions github-actions Bot removed the severity: fips change The pull request changes FIPS provider sources label May 28, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.
@bob-beck
bob-beck force-pushed the x509-verify-cert-docs branch from cc6fb06 to 3ca4914 Compare May 28, 2026 10:45
@bob-beck
bob-beck marked this pull request as ready for review May 28, 2026 10:57
@bob-beck

Copy link
Copy Markdown
Contributor Author

And @botovq I would like you to read this. and I am sorry ;)

@bob-beck
bob-beck requested a review from jogme May 28, 2026 11:01
Comment thread doc/man3/X509_verify_cert.pod
@bob-beck bob-beck self-assigned this May 29, 2026
@bob-beck bob-beck moved this to Waiting Review in Development Board May 29, 2026
@t8m t8m added branch: master Applies to master branch triaged: documentation The issue/pr deals with documentation (errors) tests: exempted The PR is exempt from requirements for testing labels May 29, 2026
@t8m t8m added branch: 3.0 Applies to openssl-3.0 branch (EOL) branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 labels May 29, 2026
@t8m t8m closed this May 29, 2026
@bob-beck
bob-beck requested a review from t8m June 1, 2026 21:00
@bob-beck

bob-beck commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

(side note - if we backport this, probably the right move is to just copy the HEAD page overtop as the way to get rid of the conflicts, as this is basically a full rewrite - OTOH it has not been there for many years, so perhaps we don't need to
back port this to old versions)

Comment thread doc/man3/X509_verify_cert.pod
Comment thread doc/man3/X509_verify_cert.pod
Comment thread doc/man3/X509_verify_cert.pod
Comment thread doc/man3/X509_verify_cert.pod
@bob-beck
bob-beck requested a review from nhorman June 3, 2026 18:03

@Sashan Sashan left a comment

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.

I've read through this. I like it. I feel enlightened now. However I'm not expert in this area, so I don't feel qualified enough to approve it. thanks.

@bob-beck

bob-beck commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I've read through this. I like it. I feel enlightened now. However I'm not expert in this area, so I don't feel qualified enough to approve it. thanks.

If we wait for experts in how messed up the corner cases of X509_verify_cert are, we're going to wait a very long time for approval.

Assuming I can't approve my own PR :)

@bob-beck

bob-beck commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I've read through this. I like it. I feel enlightened now. However I'm not expert in this area, so I don't feel qualified enough to approve it. thanks.

If we wait for experts in how messed up the corner cases of X509_verify_cert are, we're going to wait a very long time for approval.

Assuming I can't approve my own PR :)

You should go ask @botovq if it's close enough for you to approve :) this will also force him to read it which I know he is avoiding because I would also avoid doing so, as everything in the BUGS and VERIFIER CALLBACK triggers PTSD in everyone who has ever dealt with it.

@bob-beck

bob-beck commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I've read through this. I like it. I feel enlightened now. However I'm not expert in this area, so I don't feel qualified enough to approve it. thanks.

If we wait for experts in how messed up the corner cases of X509_verify_cert are, we're going to wait a very long time for approval.
Assuming I can't approve my own PR :)

You should go ask @botovq if it's close enough for you to approve :) this will also force him to read it which I know he is avoiding because I would also avoid doing so, as everything in the BUGS and VERIFIER CALLBACK triggers PTSD in everyone who has ever dealt with it.

Or, alternatively, since I think your approach to reviewing this is incorrect, as you are expecting that review approval implies perfection, which is utter hogwash for doc pr's especially, IMO.

I think you should read the existing manual page for this function. Ask yourself if approval should be delayed because this might be worse documentation.

It can be iterated upon later ;)

@bob-beck

bob-beck commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Great work but it must not refer to internals.

Those should all be fixed.

@Sashan Sashan left a comment

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.

OK, so let's get this in. I don't think the doc update makes things worse.

@openssl-machine openssl-machine added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: review pending This pull request needs review by a committer approval: done This pull request has the required number of approvals labels Jun 5, 2026
@openssl-machine

Copy link
Copy Markdown
Collaborator

This pull request is ready to merge

@jogme jogme moved this from Waiting Review to Waiting Merge in Development Board Jun 8, 2026
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from #31314)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from #31314)

(cherry picked from commit b2a133e)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from #31314)

(cherry picked from commit b2a133e)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from #31314)

(cherry picked from commit b2a133e)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from #31314)

(cherry picked from commit b2a133e)
@t8m t8m removed the branch: 3.0 Applies to openssl-3.0 branch (EOL) label Jun 10, 2026
@t8m

t8m commented Jun 10, 2026

Copy link
Copy Markdown
Member

Norbert merged this to master.

I have cherry-picked it to the 4.0, 3.6, 3.5 and 3.4 branches. (with trivial conflict fix up)

The differences in 3.0 are non-trivial and I do not think this needs to be merged there.

@t8m t8m closed this Jun 10, 2026
@github-project-automation github-project-automation Bot moved this from Waiting Merge to Done in Development Board Jun 10, 2026
n13l pushed a commit to n13l/openssl that referenced this pull request Jul 25, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from openssl#31314)
rickyringler pushed a commit to rickyringler/openssl that referenced this pull request Aug 21, 2026
I am doing this because I have need to add information that really does
belong here, and the current stuff is less documentation than more or
less some hand waving about how it works without any details

So to summarize the changes:

1) This documents the current way that X509_verify_cert goes about
building and verifying a chain, identifying the relevant flags that
can affect how this happens today, and cross references the correct page
to find the rest of the exhaustive list of verification flags and what they do.

2) This documents the verification callback and how it can be used
to affect the verification outcome, This includes warning potential
users of the callback of what returning values from it means,
the common ways to end up unintentionaly doing things you did not expect
with the callback, and that it depends upon internals and should not be
relied upon.

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Wed Jun 10 11:33:13 2026
(Merged from openssl#31314)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 tests: exempted The PR is exempt from requirements for testing triaged: documentation The issue/pr deals with documentation (errors)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants