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

OC 8.0: s2s does not recognise certificates from personal store (fix included) #14790

Closed
tjaehnel opened this issue Mar 11, 2015 · 7 comments
Closed
Labels

Comments

@tjaehnel
Copy link

The problem appears in OC 8.0.0 as well as the current GitHub master.

Reproduce:

  1. Import CACert root certificate (or any other that is not in the global store) into personal certificate store
  2. Accept a sharing from another cloud that uses a certificate, signed by the just imported root certificate
  3. Open the new shared folder
    -> Nothing happens / you receive an "Unknown Error"

Problem:
The Certificate Manager returns the certificate store path relative to the data directory in getCertificateBundle(). (see: lib/private/security/certificatemanager.php, Lines: 137/141)
As a result, the sharing code feeds cURL with this relative path, which it obviously cannot find (see: apps/files_sharing/lib/external/storage.php Lines: 229/231).

Solution:
Make Certificate Manager return the absolute path to the store.
Change in file lib/private/security/certificatemanager.php, line 141 such that it reads:

$path = is_null($this->uid) ? '/files_external/' : '/' . \OC_User::getHome($this->uid) . '/files_external/';

Note:
Other parts of the OC code do avoid the certificate manager and construct the cert path store themselves. These work without issues.

@ghost
Copy link

ghost commented Mar 11, 2015

Hi,

is this related to the already reported issue here: #14747 ?

@tjaehnel
Copy link
Author

Hi,

hmm, I don't think this is related. The stack trace in the mentioned issue references to dav.php, which does not use the certificate manager to determine the path to the store. See here.

But it definitely relates to commit 67da1f7 .
There was the user object removed from certificate manager and replaced by uid, which generally is a good thing.
Though in the same step getPathToCertificates() was changed such that it now does not return the full path anymore, but file_sharing (and a few others) rely on this.

Toby

@Colely
Copy link

Colely commented Apr 4, 2015

I tried S2S sharing with a folder from an owncloud 7.02 Server to a 8.02 Server, imported the self-signed certificate and made your alteration, but it still doesn't work. Is there a way to fix this?

@DeepDiver1975
Copy link
Member

@LukasReschke certs'n'stuff - thx

@tjaehnel
Copy link
Author

tjaehnel commented Apr 6, 2015

My patch did not work completely, because some parts of the code relate on absolute, some on relative paths.
But it seems that commit 5f044eb fixes this bug and makes things much more clean - so my solution is obsolete anyway.

As a temporary workaround until this commit gets into one of the next releases, open file lib/private/security/certificatemanager.php and in function getCertificateBundle replace
return $this->getPathToCertificates() . 'rootcerts.crt';
with
return \OC::$server->getConfig()->getSystemValue('datadirectory') . $this->getPathToCertificates() . 'rootcerts.crt';

This did it for me.

@tjaehnel tjaehnel closed this as completed Apr 6, 2015
@LukasReschke
Copy link
Member

Yep - there is a lot of broken stuff in SSL / TLS handling in all current ownCloud stable releases. This is not the only thing that is broken but one of the fews that can be detected most easily.

This will be properly addressed with ownCloud 8.1 where I refactored everything HTTP related - see #15195

I doubt that backporting this only single specific patch will make much sense… There is really a lot broken as stated at #14913 (comment) and at other places.

@oparoz
Copy link
Contributor

oparoz commented Apr 17, 2015

@LukasReschke - So no point reporting more SSL issues on 8.0? Fixes will never be backported?

@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants