LibGit2: improve error when CA root cert can't be set #38827
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Working on adding some tests to this since doesn't seem to break anything. |
StefanKarpinski
force-pushed
the
sk/libgit2-improve-ssl-cert-location-error
branch
from
904876b
to
f48bb3d
Compare
|
Apparently LibGt2 does not error as expected with a bad (empty or non-existent) CA root certificates path on Linux AArch64. @Keno, are you the person to talk to about getting access to such a machine for debugging? |
|
I can get you access to the buildbot, but that sounds like an issue with the docker container that actually runs the test, which is the domain of the great and powerful wizard @staticfloat . |
|
This seems to work just fine when I'm actually logged into the aarch64 buildbot, so I don't get the failure. |
|
My working theory here is that some tests that use LibGit2 happened to get run in the same process before the LibGit2 tests, which would cause the library to already be initialized and therefore would cause the tests that expect initialization failure to fail to fail, as seen here. This wouldn't be specific to aarch64, which is supported by aarch64 tests passing when restarted, but means that I need to isolate these tests in a separate process. |
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`.
StefanKarpinski
force-pushed
the
sk/libgit2-improve-ssl-cert-location-error
branch
from
f48bb3d
to
9b4b9c7
Compare
|
How is CI still running on this? Six hours and counting... |
|
Linux32 has a long backlog |
StefanKarpinski
deleted the
sk/libgit2-improve-ssl-cert-location-error
branch
KristofferC
pushed a commit
that referenced
this pull request
Dec 17, 2020
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`. (cherry picked from commit 4dede6d)
staticfloat
pushed a commit
that referenced
this pull request
Jan 15, 2021
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`. (cherry picked from commit 4dede6d)
ElOceanografo
pushed a commit
to ElOceanografo/julia
that referenced
this pull request
May 4, 2021
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`.
staticfloat
pushed a commit
that referenced
this pull request
Dec 23, 2022
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`. (cherry picked from commit 4dede6d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This addresses improving the error message in #38691.
This also fixes an insecure behavior: even if
set_ssl_cert_locationsfailed,REFCOUNTwas still incremented, so subsequent calls toensure_initializeddidn't callinitializeand so there is never a successful call toset_ssl_cert_locations. Without this libgit2 defaults to not verifying host identities, which is insecure. To prevent this, this patch locks onensure_initializedand decrementsREFCOUNTif initialize throws an error, ensuring thatinitializesucceeds at least once, including the call toset_ssl_cert_locations.