getUrlContent should verify the SSL certificate

[LukasReschke]

In case of TLS connections is getUrlContent currently not verifying the SSL certificate if curl is not available.

Additionally as a second layer of defense CURLOPT_SSL_VERIFYPEER = true is enforced, this should be anyways the default since cURL 7.10.

[LukasReschke]

@VicDeo Could you use this function in the updater app, see https://github.com/owncloud/apps/blob/master/updater/lib/downloader.php#L68 ?

[VicDeo]

@LukasReschke this will reintroduce #6910
I can't put entire package into variable.

[LukasReschke]

@VicDeo Allright! Would be awesome if you could then implement the SSL verification manually :-) (e.g. copy paste it)

[LukasReschke]

@schiesbn I've added the possibility to disable the SSL check. As far I can see the "From URL" function (

core/apps/files/ajax/newfile.php

Line 94 in 797e0a6

if($source) {

) is using an own implementation with progress bar etc... So this would actually only affect the OCS client (api.apps.owncloud.com) which is still provided by HTTP.

@karlitschek Would it be possible to add a matching SSL certificate to api.apps.owncloud.com?

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/3244/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/3246/

[scrutinizer-notifier]

A new inspection was created.

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/3279/

[LukasReschke]

@owncloud-bot Retest this please

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/3286/

[PVince81]

Code looks good 👍

@VicDeo can you have a look as well ?

[PVince81]

No backport for this I'd say because it changes the public API.

[PVince81]

@VicDeo any update ?

[bantu]

How is it determined whether a certificate is valid or not? Are the two implementations consistent with regards to this?

The PHP documentation about verify_peer is not very clear about this and it actually sounds like cafile or capath must be specified with verify_peer. See http://www.php.net/manual/en/context.ssl.php

Possibly unrelated: Do we also have to specify CN_match or is PHP intelligent enough to extract it from the URL?

[bantu]

Besides these open questions, this patch looks good. 👍

[LukasReschke]

Very good questions, because they showed that this PR will lead to problems
due to PHP madness and me running bleeding edge builds ;-)
Kudos for that.
(which is obviously no excuse for the fact that I haven't written an unit
test for this)

How is it determined whether a certificate is valid or not? Are the two
implementations consistent with regards to this?

PHP => 5.6: Tries to use the system certificate store.
PHP < 5.6: Will simply fail.
Curl (both): should use a bundled copy. But due to some distributions not
packaging this it should better get specified manually here too.

Possibly unrelated: Do we also have to specify CN_match or is PHP
intelligent enough to extract it from the URL?

Awesome PHP madness again:

PHP => 5.6: Does have sane defaults and will verify this.
PHP < 5.6: Will accept anything and does NOT support SAN..... (Will also
probably break things). Wildcard support is also only merged since 2013. (
https://bugs.php.net/bug.php?id=51100)

I will close this therefore for now and come up with a new PR which will
address this in a backwards compatible way. But this requires some research
and testing - could take some time.

Sent from mobile device

How is it determined whether a certificate is valid or not? Are the two
implementations consistent with regards to this?

The PHP documentation about verify_peer is not very clear about this and it
actually sounds like cafile or capath must be specified with verify_peer.
See http://www.php.net/manual/en/context.ssl.php

Possibly unrelated: Do we also have to specify CN_match or is PHP
intelligent enough to extract it from the URL?

—
Reply to this email directly or view it on
GitHubhttps://github.com//pull/7330#issuecomment-38367517
.