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

TLS Client certificate support [$110] #69

Closed
4 tasks done
martinKupec opened this issue Nov 12, 2012 · 129 comments
Closed
4 tasks done

TLS Client certificate support [$110] #69

martinKupec opened this issue Nov 12, 2012 · 129 comments
Assignees
Labels
bounty Enhancement ReadyToTest QA, please validate the fix/enhancement
Milestone

Comments

@martinKupec
Copy link

martinKupec commented Nov 12, 2012

Hi,

I am deploying ownCloud to private environment . As I want to upload to my cloud confidental documents, I need some good security.

I have decided, that it would be nice to require SSL certificates from clients. This is pretty strong security measure. But I have found, that there is no support for this in mirall.

I kindly ask any mirall/csync developer, if it would be possible to add configure option for SSL client certificate. All what is needed is is to provide certificate when connecting to server. It should be simple task to someone familiar with the code.

Mirall uses QSslSocket and there are QSslSocket::setLocalCertificate and QSslSocket::setPrivateKey functions to set the certificate.

csync uses neon and it has ne_ssl_set_clicert function.

I will be happy to answer any question or test any code.
Thank you.

Update from @danimo:

Ok, for someone who likes to pick this feature up, here is what needs to be done in more detail:

  • Modify wizard to alternatively accept auth via client cert on top of password (or is there a way to tell owncloud to accept client auth only?)
  • If certificate is encrypted, ask for its password and store via qtkeychain.
  • Add cert/key to QSslConfiguration of the global QNetworkAccessManager.
  • Add property to csync_owncloud.c to pass the certificate (either unencryted or encrypted along with Password) to neon as used in csync (Neon API).

If you want to start working on it, please contact me.

--- Did you help close this issue? Go claim the **[$110 bounty](https://www.bountysource.com/issues/905047-tls-client-certificate-support?utm_campaign=plugin&utm_content=tracker%2F216457&utm_medium=issues&utm_source=github)** on [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F216457&utm_medium=issues&utm_source=github).
@wdehoog
Copy link

wdehoog commented Jan 23, 2013

We have the same situation and cannot use the client until it can provide certificates.

Preferebly it would use the windows certificate store or the one from firefox.

@kzajacc
Copy link

kzajacc commented May 28, 2013

We're just another group of people which is interested in this functionality. It would appreciated if You would implement this.

@frijsdijk
Copy link

Yessir, interested in such a feature!

@danimo
Copy link
Contributor

danimo commented May 28, 2013

If there is so much interest, why does nobody come up with a patch then?

@pretjep
Copy link

pretjep commented Jun 11, 2013

Waiting for this!!

@rbardoel
Copy link

rbardoel commented Jul 6, 2013

+1 for this feature!

@danimo
Copy link
Contributor

danimo commented Jul 6, 2013

Added outline on how to implement the feature.

@natschil
Copy link

I'm currently using stunnel to create a https->http "tunnel" as an ugly hack to get this functionality. It works okay-ish, but having to start stunnel every time I start mirall is far from optimal.

@natschil
Copy link

Does anyone know whether there's any work happening on this?
If not, I would be interested in looking at the source code to see what I can do.

@danimo
Copy link
Contributor

danimo commented Aug 22, 2013

@natschil I am not aware about anyone working on this. See my descriptions in the original report on what needs to be done. Don't hesitate to ask if you need further help.

@natschil
Copy link

I'll have a look at what I can do in the next few days. Is there an irc
channel for mirall development somewhere?

@danimo
Copy link
Contributor

danimo commented Aug 22, 2013

#owncloud-client-dev

@think-nice-things
Copy link

I just patched ocsync to allow for CA and client certificates.
I mailed the patch to owncloud@kde.org as I don't know how to add it here.

The syntax is pretty obvious IMHO and is described in the new help:

--ca-cert=<file>       file name of CA certificate
--client-cert=<file>   file name of client certificate
--client-cert-pass=<p> password of client certificate

This allows ocsync to connect to a server which is protected by a (self-signed) client certificate.

The ca file will usually be a PEM file, the client certificate will usually be in p12 format.

I added corresponding properties ca_certificate, client_certificate and client_certificate_pass to csync_owncloud.[ch], which could also be used by the gui.

Anyone volunteers to do the gui stuff? (I'm afraid of beeing not experienced enough to do this).

It would be nice if this could make it to the official sources soon.

@natschil
Copy link

natschil commented Nov 3, 2013

Thanks for doing this!
I also have a set of (untested) patches for ocsync with client side ssl
certificates, I can upload them somewhere if you want to compare yours
with mine. Note that my patches only change the owncloud module of
csync, and hence have no commandline arguments, but if I remember
correctly, owncloud doesn't actually call the csync application w/
commandline parameters but instead dynamically loads a shared library or
something. (I coded this about a month ago, and I don't remember the
details).

I had a look at some of the GUI stuff too and made some preliminary
changes ( such as changing the .ui files to accept client side ssl
certificates, and changing some of the configuration file abstraction to
be able to set client side ssl cert configuration options, but then I
ran out of free time and didn't actually get to anything that works
(writing Qt GUIs is not really my area of expertise either).

On 11/02/2013 10:09 PM, joze- wrote:

I just patched ocsync to allow for CA and client certificates.
I mailed the patch to owncloud@kde.org mailto:owncloud@kde.org as I
don't know how to add it here.

The syntax is pretty obvious IMHO and is described in the new help:

|--ca-cert= file name of CA certificate
--client-cert= file name of client certificate
--client-cert-pass=

password of client certificate
|

This allows ocsync to connect to a server which is protected by a
(self-signed) client certificate.

The ca file will usually be a PEM file, the client certificate will
usually be in p12 format.

I added corresponding properties ca_certificate, client_certificate
and client_certificate_pass to csync_owncloud.[ch], which could also
be used by the gui.

Anyone volunteers to do the gui stuff? (I'm afraid of beeing not
experienced enough to do this).

It would be nice if this could make it to the official sources soon.


Reply to this email directly or view it on GitHub
#69 (comment).

@think-nice-things
Copy link

natschil // yes please make your code available, I'll check if I can make the best from both versions

@moscicki
Copy link
Contributor

moscicki commented Nov 3, 2013

Hi guys,

Funny enough, I also have a patch for related to ssl certificates: disabling the check altogether (originally because of self-signed cert on the server). It is actually controlled by the PATCH variable in my compilation script that was discussed on the mailing list.
Not sure if we will need this anymore if we have --cert options your developed.

kuba

On Nov 3, 2013, at 8:24 PM, joze- notifications@github.com wrote:

natschil // yes please make your code available, I'll check if I can make the best from both versions


Reply to this email directly or view it on GitHub.

@natschil
Copy link

natschil commented Nov 4, 2013

This issue isn't related to ssl checks though, but to being able to
provide a client side ssl certificate.

On 11/03/2013 08:37 PM, moscicki wrote:

Hi guys,

Funny enough, I also have a patch for related to ssl certificates:
disabling the check altogether (originally because of self-signed cert
on the server). It is actually controlled by the PATCH variable in my
compilation script that was discussed on the mailing list.
Not sure if we will need this anymore if we have --cert options your
developed.

kuba

On Nov 3, 2013, at 8:24 PM, joze- notifications@github.com wrote:

natschil // yes please make your code available, I'll check if I can
make the best from both versions


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#69 (comment).

@icetype
Copy link

icetype commented Jan 22, 2014

Is there any plan to implement the function of client side ssl certificate in the Desktop Sync Client softwares? This functionality would be very much appreciated!

@danimo
Copy link
Contributor

danimo commented Jan 22, 2014

@icetype The feature is not scheduled for the next two major releases. Which pretty much means it's not on the roadmap. As indicated above, we're glad to assist anyone who attempts to implement SSL client certs as a coherent feature in the ownCloud Client.

Alternatively, you can always influence the main developers' priorities by purchasing an ownCloud commercial license and telling sales you need this particular feature.

@icetype
Copy link

icetype commented Jan 22, 2014

Thank you for the information, danimo!

@laurivosandi
Copy link

Could you guys please share patches you've got?

@danimo
Copy link
Contributor

danimo commented Jan 25, 2014

@natschil @moscicki Care to share your patches with @v6sa?

@natschil
Copy link

Sorry for not doing this earlier, I've been relatively busy recently and haven't gotten to working on this.... I've put a patch for csync and one for mirall here: https://github.com/natschil/owncloud_client_side_ssl_stuff

Notes:
The csync patch is far more complete than the mirall one. I didn't completely understand the mirall codebase iirc, and so only made a few tentative changes. It may be of some help to someone looking at the scope of what needs to be change, but I would caution you that especially the mirall patch is far from comprehensive. The csync patch is, as far as I remember, relatively complete. (It is against git://git.csync.org/users/owncloud/csync.git )

@icetype
Copy link

icetype commented Jan 28, 2014

Thank you, @natschil , for kindly proving your patches. I'm a complete newbie but will give it try to build them.

@natschil
Copy link

@icetype You can try to build the csync one (I think it compiles), but the owncloud patch doesn't actually do anything.....These patches are mainly meant for someone trying to develop client side ssl functionality to have a look at, they aren't actually working code. Sorry for being unclear...

@natschil
Copy link

@icetype If you're looking for using client side ssl certificates, I suggest you use stunnel to connect to your server, that has worked for me in the past.

@icetype
Copy link

icetype commented Jan 28, 2014

@natschil Thank you for the information and kind suggestion. In fact, I tried to use ssh dynamic port forwarding with client-side ssl certificates and SOCK5 proxy setting in the ownCloud client. It seems to be working OK so far, although it may not be an ideal solution.

@Raptormagnum
Copy link

With a colleague, we did it with the last client's version (1.6) and Qt 5.0.

The client need a PKCS12 container to match the server certificate and established the SSL connection. Otherwise, the connection can't be established.

The window to configure the PKCS12 path pops when the returned SSL error is SSL_ERROR_HANDSHAKE_FAILURE_ALERT.

@nimminimminot
Copy link

nimminimminot commented Sep 20, 2016

Ok, your way! Question: is it possible to use SSL AUTH (with or without regular password auth) with native owncloud client? The idea: httpd allow to https://my_own_cloud only devices with ssl certificate, which signed my own CA. Option "SSLVerifyClient require" for httpd. This feature can seriously increase security.

Add: this question is for Windows, MacOS, iOS.

@AnrDaemon
Copy link

No, it can not increase security.

@moscicki
Copy link
Contributor

@AnrDaemon: you rightfully complained about the previous post of @billybons2006 of not being constructive and of little value. You last answer is equally unconstructive and unfriendly. If you consider that @billybons2006 is a relatively unexperienced person (which I would infer from their posts) your answer will result in this person either being put off from the project or continue this conversation about "can or cannot increase security". Either way, this does not bring much value to this thread nor to implementing this feature.

If you are a developer and can do something about this feature then provide an appropriate comment. If you are a community member that cares about quality of posts from less experienced members, then please do it decently.

If am neither (nor a github policeman) but I have a feeling that there is a lot of energy and time wasted here from all people involved in this and other threads.

Many thanks for your understanding.

@AnrDaemon
Copy link

Look, I could write a lengthy essay on X.509 and SSL usage in general, but the bottom line would be: client certificates ease automation and provide consistent access control for automated services, but they do NOT increase security by any means imaginable.
The problem is that people are told "SSL is good" and they take it for granted without an afterthought. In the light of this trend, I see HTTPS itself as useful as plain HTTP, and about as secure.
Just a single quote from my article:

Entire certification system is built upon practically ifinitely inherited trust.
Installing a root CA certificate into your system, you are with your very hands telling it that:
"I(you!) trust the issuer of this certificate and I will consider trusted every certificate this entity issued."
(Now, for a moment: your system has about a hundred of root certificates preinstalled without your full consent, to which it doubtlessly trust.)

@nimminimminot
Copy link

Dear @AnrDaemon, by saing "increase security" I meant that using ssl is additional layer (something two-factor) of auth. On the other hand, using one long and very streight password (which hard to remember) VS using SSL (from outside attackers) and regular password (from my so-workers) can be much more useful.

I can use (and use ;)) fail2ban, but if several people using one owncloud server, ban by IP will ban all users, not only one.

Any way, owncloud very good, I use it and advise my friends.

I agree with @moscicki and ready to close my question. Thank you all!

guruz added a commit that referenced this issue Oct 19, 2016
This also nicely displays the 'Untrusted domain' message of oC.
The link to add a trusted domain (via web browser) is clickable.
@guruz guruz assigned guruz and unassigned danimo Oct 21, 2016
@guruz
Copy link
Contributor

guruz commented Oct 21, 2016

I've picked this up again and I'm working on several things:

  • Re-enabling the certificate config dialog that had been contributed for usage from the wizard
  • Storing of cert and key inside the keychain instead of storing p12 path and password in the config.
  • Storing per account instead of global

@guruz guruz added this to the 2.3.0 milestone Oct 21, 2016
@guruz guruz changed the title SSL Client certificate TLS Client certificate support Oct 21, 2016
guruz added a commit that referenced this issue Oct 31, 2016
The re-enables the UI, uses Qt API for importing and
stores the certificate/key in the keychain.
guruz added a commit that referenced this issue Nov 23, 2016
This also nicely displays the 'Untrusted domain' message of oC.
The link to add a trusted domain (via web browser) is clickable.
guruz added a commit that referenced this issue Nov 23, 2016
This also nicely displays the 'Untrusted domain' message of oC.
The link to add a trusted domain (via web browser) is clickable.
guruz added a commit that referenced this issue Dec 5, 2016
The re-enables the UI, uses Qt API for importing and
stores the certificate/key in the system keychain.
People who had set up client certs need to re-setup the account. This is ok
since it was an undocumented feature anyway.
@guruz
Copy link
Contributor

guruz commented Dec 5, 2016

PR in #5289 is ready to test.
Please compile from fix_client_certs branch and (re?)-setup the specific account. It should show a dialog in the wizard that lets you set the client certificate.

Please comment your test findings on #5289 .. not here.

@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Dec 5, 2016
guruz added a commit that referenced this issue Jan 2, 2017
The re-enables the UI, uses Qt API for importing and
stores the certificate/key in the system keychain.
People who had set up client certs need to re-setup the account. This is ok
since it was an undocumented feature anyway.
@guruz
Copy link
Contributor

guruz commented Jan 3, 2017

The PR is merged. We would need some testing from you guys:)
There was an issue with updating the nightlies on Linux which should be fixed now.

Please download from or compile yourself (master branch)
https://owncloud.org/install/#testing-development

Note that it needs a current Qt version (>= 5.5 or so)

@guruz
Copy link
Contributor

guruz commented Jan 25, 2017

Done as part of 2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty Enhancement ReadyToTest QA, please validate the fix/enhancement
Projects
None yet
Development

No branches or pull requests