gitblit-org / gitblit

pure java git solution
http://gitblit.com
Apache License 2.0
2.28k stars 670 forks source link

support for public key based security? #402

Closed gitblit closed 9 years ago

gitblit commented 9 years ago

Originally reported on Google Code with ID 106

Are there plans to support public key based security (in addition to userid/password
login)?

https://help.github.com/articles/generating-ssh-keys

Thanks

Reported by gliptak on 2012-07-11 13:07:06

gitblit commented 9 years ago
If you mean SSH, then no.  If you mean https client certificate authentication, then
perhaps.  I'm not opposed to SSH, I just have no interest in implementing it.

Reported by James.Moger on 2012-07-11 13:19:52

gitblit commented 9 years ago
I would like to see some easy means of pushing a public certificate into the server,
and being able to avoid typing in a password on the command line. Thanks

Reported by gliptak on 2012-07-11 22:33:34

gitblit commented 9 years ago
So I'm not entirely sure if I should open a new issue for an enhancement or not but
for now I will add to this one. I would like to see the possibility for client certificate
authentication for the web interface and git client if possible with perhaps user information
stored in something like an ldap server. The question I have since I am still trying
to understand everything and am new to java web applications is if it would be as simple
as a new wicket filter or an extension to IUserService? I am willing to make an attempt
at it, although I won't make any promises, if I could get a decent overview of things
and where to start looking in to things. Thank you for your time.

Reported by andersonkw2 on 2012-09-19 01:06:02

gitblit commented 9 years ago
You would have to implement a servlet filter which would intercept all incoming requests
to grab the certificate from the request (I actually have no idea at the moment how
client certs work - is it a request header field?).

We will need an implementation of javax.servlet.Filter and we will have to modify the
web.xml file to make this filter the root filter for the app and then have Wicket next
in the filter chain.  As for the user service, we shouldn't need a new one but we will
need some way to map a cert to a user which probably will require adjusting the user
services.

I've been wanting to revise the filter chain so this is good timing.

Reported by James.Moger on 2012-09-19 12:20:08

gitblit commented 9 years ago
So I have made an attempt at this but not successfully yet. So the client cert is requested
and passed in a http attribute (javax.servlet.request.X509Certificate). 

So I am running gitblit in tomcat with tomcat set to require a certificate. After adding
the filter to the web.xml I am able to print out the distinguished name of the certificate
that is passed to the server. I would think using the distinguished name of the certificate
as the username would be best as it uniquely identifies the individual and then leave
the displayName up to the user or some other source. That would eliminate the need
to modify the user services I believe. 

Assuming I pass a client certificate to the server which proves to be authenticate
and extract the distinguished name out of the request, how do I then say to the main
servlet that the user is authenticated by the filter and automatically log them in
to the web interface and authenticate git actions (push,pull,branch,etc.)? I see that
there is an authenticate(username,password) method but how would it been done strictly
based off of the certificate without a password? 

Reported by andersonkw2 on 2012-09-26 00:05:32

gitblit commented 9 years ago
We'll need a new method in the user services to authenticate the certificate.  Something
like the authenticate(cookie) method that exists.

It is not clear to me if the cert is to be used in a public key/private key scenario
or if it is a more sophisticated cookie.  If the latter then perhaps we can take the
SHA1 of the entire cert and store that in a user field.  It also seems that we may
want to store several certs (or cert hashcodes) per user account.  Kinda like how GitHub
allows you to upload multiple SSH keys.  Alternatively we could base64 encode the cert
and store it as a field in a user.  Last resort would be store the cert to a file...
but then we'll run into trouble with federation.

How do you configure the git client to send the cert?  Are you modeling this based
on an existing implementation?

Reported by James.Moger on 2012-09-26 00:51:48

gitblit commented 9 years ago
I will look in to adding a new method to authenticate the certificate. The authenticate
method could take the certificate object, validate it, and return a user model similar
to what the cookie and password method do now from my understanding. 

This is only my interpretation but I believe it is based off of a public/private key
pair but they rarely are separated. I also think it would only be necessary to store
the distinguished name of the certificate. An example DN could be cn=unique identifier
or even email(username),ou=Marketing,o=Some Big Organization,l=Some State,st=No Where,c=us.

Ultimately if you are passed a certificate with a certain distinguished name and it
is validated by a certificate authority that you have set as trusted for your installation
it certifies that the user is who they say they are and that they are the only person
with that DN. With that being said if you only store the DN for the certificate when
it comes time for a certificate renewal the user can still access the server because
the DN will have remained the same and the new certificate should pass all validity
checks.

Also the approach I am trying to accomplish is more towards SSL X509 client certificates
than it is ssh key pairs. With each DN being unique like a username would it fit better
as a username with the display name being the user? Or perhaps the first cn entry of
the DN being the username? So a DN of cn=bob.smith,ou=developers,o=major corp,l=fairfax,st=virginia,c=us
would have a username bob.smith.

The git client can be configured for SSL client certificates with the http.sslCert
and http.sslKey option. What do you mean by existing implementation? Existing applications
perhaps? If so then yes. For my environment we have a policy to enable SSL certificate
authentication on all applications. We run our own certificate authority and pass out
certificates to everyone to authenticate them to applications. This has the advantage
of standardizing the authentication and freeing people of having to remember more passwords.

I appreciate all of the feedback thus far. This is all new to me but I decided the
best way to learn is to jump in and try to get something working. :)

Reported by andersonkw2 on 2012-09-26 02:20:18

gitblit commented 9 years ago
Ok, I think I get it.

I'm wondering if Gitblit should generate a client certificate and that cert could then
be downloaded by the user to their machine and configured using http.sslCert.  Essentially,
make Gitblit a CA for itself.

It is not what you need, but for those who want client certificate auth with minimal
fuss, it might make things easier.  This would be analogous to GO creating it's own
self-signed cert on first execution so that you can start with https security right
away.

Or perhaps I still don't have a complete understanding here.

Reported by James.Moger on 2012-09-26 12:06:11

gitblit commented 9 years ago
Are you still working on this capability?

Reported by James.Moger on 2012-10-19 18:35:17

gitblit commented 9 years ago
I am still working on at the very least getting basic client certificate authentication
working. I've been unable to dedicate any substantial amount of time to it right now
though so it is taking longer than expected. 

Reported by andersonkw2 on 2012-10-19 19:13:26

gitblit commented 9 years ago
That is fine.  Work and life make it hard to make time for these things.  I have that
problem too.  :)  No hurry.  I was just interested to know of your status.  Keep me
posted if anything exciting happens!

Reported by James.Moger on 2012-10-19 19:15:45

gitblit commented 9 years ago
So I just wanted to post that I finally have something working! It currently authenticates
access to the web application and to git clones and pushes. Basically it intercepts
the request in the AccessRestrictionFilter and implements a custom RequestCycle and
checks for a certificate to be present. If it finds one it searches for a user model,
which if found gets set as the authenticated user. If it does not find a certificate
or a valid user for that certificate it acts like normal and presents the standard
anonymous web page.  

Tomorrow I will try to work on cleaning up the code I have and uploading it to a Github
repository. This has been a learning experience for me in both Java web applications
and Wicket so I would be interested in your feedback on all of it.

I do have one issue that I'm currently thinking of how to solve. Basically my initial
idea was to store the entire certificate distinguished name as the username in the
user model as a string. I thought this was a great idea till something didn't like
the string representation of the certificate. :) I think it was related to the commas
that are present. I thought of just shortening the username to just the common name
of the certificate but I don't feel like that is ideal because there could be collisions
with certificates. I may try sanitizing the DN of commas and storing that to see if
that causes errors or not. Currently the code I have though pulls out the common name.

Reported by andersonkw2 on 2012-10-21 23:37:28

gitblit commented 9 years ago
Cool.  I am very interested in grafting your code into Gitblit.

I'm not sure I get the exact connection between username and DN.  Shouldn't the cert
be an id card for the person (UserModel)?  If so the usermodel should not have it's
primary attribute overriden/set by the cert.  The cert should identify the user, but
not BE the user.

Do you forsee an issue with storing the hash of the cert in the UserModel rather than
the entire cert?

UserModel {
    List<String> certificates;
}

And then something like:

IUserService {
    authenticate(cert) {
        String hash = hash(cert);
        UserModel user = findUserForCertHash(hash);
        return user;
    }
}

Actually, I think I will be adding an authenticate(HttpServletRequest req) to IUserService
for Gerrit so we can probably take advantage of that.

Reported by James.Moger on 2012-10-22 12:34:54

gitblit commented 9 years ago
So I got the code that I had cleaned up and pushed to github. The link is: https://github.com/kevinanderson1/gitblit/tree/workingClientCertificate

The reason I didn't do something like a hash of the certificate is because when the
validity period of a certificate changes, for instance a certificate is renewed due
to expiration, that hash will change and no longer be valid. 

I chose the username because the subject distinguished name of a certificate should
uniquely identify an individual in the same way a username uniquely identity an individual.
There should never be two certificates with the same distinguished name and considering
a user should not have to every have to type it I thought it would be OK. 

If you want to avoid storing the distinguished name in the username field then I think
my suggestion would be to add a field to the user service to hold the subject distinguished
name of the certificate and match that similar to how the getUserModel(string) function
works now. Instead it would return a user model or null when passed the distinguished
name of a cert. So for instance getUserDistinguishedName(subjectDN); would return the
appropriate user model or null if no match is found. 

In the case of Apache Tomcat and I believe all application servers by the time a request
reaches a web application requesting a certificate a few checks have been completed.
Those checks include the certificate is signed by a Certificate Authority residing
in the server's trust store, the certificate does not reside on a certificate revocation
list if the server is configured to use one, and that the certificate is valid. With
that being said the presence of the ssl certificate variable implies that it has passed
those checks and proves the identity of the request and that it can be trusted for
authentication purposes so a hash of the certificate and storing other attributes basically
shouldn't be necessary. 

With the authenticate(HttpServletRequest req) proposal would it be possible to have
that called when a new request is received without any user interaction? Basically
if I go to the web app will I be logged in seamlessly? Also would it have access to
the request before it gets modified by any other filter or servlet? 

Let me know your thoughts on all of this. Thank you in advance for taking the time
to look at things. :)

Reported by andersonkw2 on 2012-10-22 21:47:51

gitblit commented 9 years ago
Sorry it has taken so long for me to review.  I'm submerged in the permissions overhaul.
 Taking a short breather to review your code.

It is surprisingly concise and I think you made some good choices.

During our discussion I envisioned a servlet filter specified in the web.xml before
the Wicket filter, but that wouldn't set the Wicket session.  So because of that, your
approach of modifying the authentication filter (grand-daddy of all servlet services)
and the Wicket request cycle are excellent alternatives.  I would move the mechanics
of extracting the username from the httprequest into a static method in com.gitblit.utils.HttpUtils
to cut down on duplication.

CN & Username.
I like this approach.  So right now you are depending on manual account creation by
an admin with the CN that matches the username.  That fits in with how I would like
to use this.

I'm considering having Gitblit GO offer downloadable dynamically generated user certs
signed(?) by it's own SSL cert - or perhaps a different self-signed cert.  I'm not
sure if signing against the SSL cert is a security issue or not - the certificate trust
chain is a little fuzzy to me.  The goal would be to simplify use of http client cert
usage for those without a local CA.  Gitblit GO would be it's own CA.  Do you see any
problem with that idea?

Reported by James.Moger on 2012-11-02 19:09:54

gitblit commented 9 years ago
I will work on moving the username extraction code in to com.gitblit.utils.HttpUtils
sometime this week when I get some free time. 

In regards to the dynamically generated certificates I do not think it would be a bad
idea but I would suggest it carry some type of warning about ensuring the key is protected
and not used on public computers. The main thing to think of with certificates is how
do you handle a compromise of a certificate? Typically with an enterprise certificate
authority it could be handled with certificate revocation lists. I think it would be
possible to do, it would just be something to keep in mind.

Reported by andersonkw2 on 2012-11-05 01:41:02

gitblit commented 9 years ago
Hi Kevin,

This past weekend I harvested the essential bits of your changes and tried to get them
working with Gitblit as it's own CA.  As expected I have run into a few complications.
 :)  First and foremost of those is the lack of documentation around git http.sslCert.
 It is not clear to me if both http.sslKey and http.sslCert are required or if just
http.sslCert is necessary.  Additionally, it is not clear if those files should be
DER formatted, PEM formatted, or some permutation of those two.  Did you also need
http.sslCAInfo?

I'll probably simplify my approach for now to just plain certs without adding the CA
element.  Once I get that working I will re-try the CA thing.  Any details you can
provide on how you setup the git config fields would be appreciated.

Reported by James.Moger on 2012-11-12 13:47:43

gitblit commented 9 years ago
Hi James,
  The main documentation I found was contained in the git-config man page. An online
copy can be found here: http://www.kernel.org/pub/software/scm/git/docs/git-config.html.
I can certainly understand how you feel about the documentation though. :) I believe
that both variables are necessary as one contains the private key and the other contains
the public certificate. 

With regard to the certificate formats I believe they are both PEM formatted. Also
you should only need sslCAInfo if sslVerify is not disabled. 

Reported by andersonkw2 on 2012-11-14 02:31:15

gitblit commented 9 years ago
I've made some good progress here.

I stepped away from getting Git to talk with certs and I'm just working with getting
the browser to authenticate by certs.  That seems a little more well defined/documented.
 If I generate the CA and client certs with openssl, I can get browser authentication
working.  Generating CA and _valid_ client certs with Bouncy Castle is causing me migraines
for which I will be seeking help shortly.  But I think I am really close.

I believe in your code you are using the complete DN of the client cert as the username
(CN=James, OU=MyGroup, O=MyCompany).  My plan is to extract the common name (CN) from
the DN and use that as the username.  Would that work for your setup or would we have
to make username determination configurable?

Reported by James.Moger on 2012-11-14 15:45:31

gitblit commented 9 years ago
When I was initially trying to get things to work I had trouble with Bouncy Castle as
well and eventually reverted to openSSL since that is what I was the most comfortable
with. :)

So the code I posted would take the CN out of the full DN of the cert. So for example
the distinguished name you provided would pull out and use "CN=James" as the username.
The specific portion that does it would be:

LdapName dn = new LdapName(certChain[0].getSubjectX500Principal().getName("CANONICAL"));
username = dn.get(dn.size() - 1);

The ldap method dn.get basically makes each section of the DN a separate field and
the dn.size()-1 says take the first field so in this instance the CN. I think extracting
the CN out and using that is a good idea and was what I intended to do but I wasn't
sure of the best way to do it. :)

I think it should be configurable just because organizations may form the subject distinguished
name in different ways. One may do Lastname Firstname MI and another might be Firstname
Lastname emailAddress. 

Reported by andersonkw2 on 2012-11-15 01:07:20

gitblit commented 9 years ago
Kevin,
I have integrated your authentication changes (more-or-less), added some new settings,
and created a server-side tool to generate client certificates.  The new tool is not
quite finished, but it is reasonably functional.

Reported by James.Moger on 2012-11-23 17:09:03

gitblit commented 9 years ago
@gliptak, if you are still listening:

The upcoming release will support:
1. x509 certificate authentication
2. cookie authentication
3. basic username/password authentication

All 3 of these can be used in a password-less workflow in native git:
http://git-scm.com/docs/gitcredentials.html covers 2 & 3

1 is decrypt your private key with openssl then
git config [--global] http.sslKey path/to/decrypted/key

Reported by James.Moger on 2012-12-06 01:01:52

gitblit commented 9 years ago
Thank you James

Reported by gliptak on 2012-12-06 02:31:18

gitblit commented 9 years ago
v1.2.0 has been deployed.

Reported by James.Moger on 2013-01-01 01:06:25

gitblit commented 9 years ago
James thank you for your help with everything. :)

Reported by andersonkw2 on 2013-01-02 00:57:57