Skip to content
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

fix(federation): Do not overwrite certificate bundle #47340

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Aug 20, 2024

Signed-off-by: Julius Härtl jus@bitgrid.net

  • Resolves: #

Summary

One might still want to verify self-signed certificates and not disable verification entirely. Before this change the verify option was always overwritten with true which means that the system certificate store was used instead of the Nextcloud built in one. We should only pass verify if we actually intent to disable verification, else the http client is injecting the cert bundle properly in

RequestOptions::VERIFY => $this->getCertBundle(),

Checklist

@juliusknorr juliusknorr added this to the Nextcloud 31 milestone Aug 20, 2024
@juliusknorr juliusknorr requested review from a team, Altahrim and yemkareems and removed request for a team August 20, 2024 09:56
@juliusknorr juliusknorr force-pushed the fix/federation-certificate-store branch from 499c495 to 00b3bc4 Compare August 29, 2024 10:50
@juliusknorr
Copy link
Member Author

Added some more cases that I noticed when trying to debug something with @Antreesy where the request failed silently to also make use of the setting and log errors in apps/files_sharing/lib/External/Storage.php

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the fix/federation-certificate-store branch from 00b3bc4 to 232c22f Compare September 19, 2024 22:13
@juliusknorr juliusknorr merged commit e7f8ab1 into master Sep 19, 2024
175 checks passed
@juliusknorr juliusknorr deleted the fix/federation-certificate-store branch September 19, 2024 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants