-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LibGit2: improve error when CA root cert can't be set #38827
LibGit2: improve error when CA root cert can't be set #38827
Conversation
Working on adding some tests to this since doesn't seem to break anything. |
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`.
f48bb3d
to
9b4b9c7
Compare
How is CI still running on this? Six hours and counting... |
Linux32 has a long backlog |
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)
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)
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`.
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)
This addresses improving the error message in #38691.
This also fixes an insecure behavior: even if
set_ssl_cert_locations
failed,REFCOUNT
was still incremented, so subsequent calls toensure_initialized
didn't callinitialize
and 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_initialized
and decrementsREFCOUNT
if initialize throws an error, ensuring thatinitialize
succeeds at least once, including the call toset_ssl_cert_locations
.