1234- / gitblit

Automatically exported from code.google.com/p/gitblit
Apache License 2.0
1 stars 0 forks source link

Client Certifcates with a comma in the Subject will cause an exception when connecting #588

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.Connect to gitblit server using https and during the SSL/TLS negotiation have 
the browser present a 
client certificate that has a comma in the subject/ distinguished name.

What is the expected output?
Expect to See Normal Gitblit Home page

What do you see instead?
HTTP ERROR 500
Problem accessing /. Reason: 

    Server Error

Caused by:
java.lang.ArrayIndexOutOfBoundsException: 1
    at com.gitblit.utils.X509Utils.getMetadata(X509Utils.java:1123)
    at com.gitblit.utils.HttpUtils.getUserModelFromCertificate(HttpUtils.java:147)
    at com.gitblit.utils.HttpUtils.getUserModelFromCertificate(HttpUtils.java:134)
    at com.gitblit.manager.AuthenticationManager.authenticate(AuthenticationManager.java:229)
    at com.gitblit.manager.AuthenticationManager.authenticate(AuthenticationManager.java:177)
    at com.gitblit.servlet.EnforceAuthenticationFilter.doFilter(EnforceAuthenticationFilter.java:75)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1650)
    at com.gitblit.servlet.ProxyFilter$1.doFilter(ProxyFilter.java:74)
    at com.gitblit.servlet.ProxyFilter.doFilter(ProxyFilter.java:77)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1650)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:583)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:577)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1125)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1059)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
    at org.eclipse.jetty.server.Server.handle(Server.java:497)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:248)
    at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:540)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:610)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:539)
    at java.lang.Thread.run(Unknown Source)

What version of the product are you using? On what operating system?
Gitblit GO v1.6.2
Windows Server 2012 R2
Java Runtime Platform SE 8 U45
Internet Explorer 10

Please provide any additional information below.
In gitblit.properties: server.requireClientCertificates = false
Jetty requests a certificate from the client and if the client has only one cert
then internet explorer sends it without prompting.

The parsing of the certificate distinguished name in the getMetadata function
naively assumes that none of the relative distinguished name elements will 
contain a
comma.

I note that in the code the author doesn't trust the parsing with ldapname to 
maintain
the order or present the email address. 

I have tested with the ldap java extensions and it seems to preserve the order
of the rdn sequence in the certificate and retain email address.
Combined with a split limit of 2 each rdn element should be parsed correctly.
Perhaps it is worth revisiting the parsing with LdapName?

For what it's worth my test code is below.  Sorry, I'm not a programmer and
definitely not a java guy.

import java.io.FileInputStream;
import java.io.InputStream;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import javax.naming.ldap.LdapName;
import javax.naming.ldap.Rdn;

class test2 {
    public static void main(String[] args) {
    try {
        CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
            InputStream inStream = new FileInputStream("c:\\users\\rob\\desktop\\rob.crt");
            X509Certificate cert = (X509Certificate)certFactory.generateCertificate(inStream);
            inStream.close();

            String certdn = cert.getSubjectDN().getName();
        System.out.println(certdn);
        LdapName dn = new LdapName(certdn);
        System.out.println(dn + " has " + dn.size() + " RDNs: ");
        for (int i = 0; i < dn.size(); i++) {
            System.out.println(dn.get(i));
        String[] pair = dn.get(i).split("=",2);
        System.out.println("RDN type: " + pair[0]);
        System.out.println("RDN value: " + pair[1]);
            }

    }
    catch (Exception e) {
        System.out.println("Caught an exception");
        System.out.println(e.toString());
    }
    }
}

Original issue reported on code.google.com by rtho5...@bigpond.net.au on 26 Apr 2015 at 2:20

GoogleCodeExporter commented 9 years ago
I have pushed a fix based on your proposal.

Original comment by James.Mo...@gmail.com on 22 May 2015 at 3:27

GoogleCodeExporter commented 9 years ago
Thank-you James.  I'll attempt to compile it and report back as soon as I 
possibly can.

Original comment by rtho5...@bigpond.net.au on 31 May 2015 at 1:21