diff options
author | Robert Godfrey <rgodfrey@apache.org> | 2013-07-30 23:20:30 +0000 |
---|---|---|
committer | Robert Godfrey <rgodfrey@apache.org> | 2013-07-30 23:20:30 +0000 |
commit | c79bc28646a8b80901d87680dce63f49a364bc92 (patch) | |
tree | c8ee9035c9a351c106f7b500d08b7d4b299fae43 | |
parent | 246bec77156ea51f8a00f32e78c819faaa08b7cc (diff) | |
download | qpid-python-c79bc28646a8b80901d87680dce63f49a364bc92.tar.gz |
QPID-4875 : The parsing logic for certificate subjects does not work properly in all cases
git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1508680 13f79535-47bb-0310-9956-ffa450edef68
5 files changed, 132 insertions, 101 deletions
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java index 475f74180e..e1007d91e0 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java +++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java @@ -26,6 +26,7 @@ import javax.security.sasl.SaslServer; import org.apache.log4j.Logger; import org.apache.qpid.server.security.auth.UsernamePrincipal; +import org.apache.qpid.transport.network.security.ssl.SSLUtil; public class ExternalSaslServer implements SaslServer { @@ -88,7 +89,6 @@ public class ExternalSaslServer implements SaslServer if (_externalPrincipal instanceof X500Principal && !_useFullDN) { // Construct username as <CN>@<DC1>.<DC2>.<DC3>....<DCN> - String username; String dn = ((X500Principal) _externalPrincipal).getName(X500Principal.RFC2253); @@ -97,62 +97,21 @@ public class ExternalSaslServer implements SaslServer LOGGER.debug("Parsing username from Principal DN: " + dn); } - if (dn.contains("CN=")) + username = SSLUtil.getIdFromSubjectDN(dn); + if (username.isEmpty()) { - username = dn.substring(dn.indexOf("CN=") + 3, (dn.indexOf(",", dn.indexOf("CN=")) != -1) ? dn.indexOf(",", dn.indexOf("CN=")) : dn.length()); - - if (username.isEmpty()) - { - // CN is empty => Cannot construct username => Authentication failed => return null - if(LOGGER.isDebugEnabled()) - { - LOGGER.debug("CN value was empty in Principal name, unable to construct username"); - } - return null; - } - else - { - if (dn.contains("DC=")) - { - int start = 0; - String dc = ""; - - while (dn.indexOf("DC=", start) != -1) - { - int dcStart = dn.indexOf("DC=", start) + 3; - int dcEnd = (dn.indexOf(",", dn.indexOf("DC=", start)) != -1) ? dn.indexOf(",", dn.indexOf("DC=", start)) : dn.length(); - - if (dc.isEmpty()) - { - dc = dn.substring(dcStart, dcEnd); - } - else - { - dc = dc.concat(".").concat(dn.substring(dcStart, dcEnd)); - } - - start = dn.indexOf("DC=", start) + 1; - } - - username = username.concat("@").concat(dc); - } - } - + // CN is empty => Cannot construct username => Authentication failed => return null if(LOGGER.isDebugEnabled()) { - LOGGER.debug("Constructing Principal with username: " + username); + LOGGER.debug("CN value was empty in Principal name, unable to construct username"); } - return new UsernamePrincipal(username); + return null; } - else + if(LOGGER.isDebugEnabled()) { - // No CN => Cannot construct username => Authentication failed => return null - if(LOGGER.isDebugEnabled()) - { - LOGGER.debug("No CN= present in DN, unable to construct username"); - } - return null; + LOGGER.debug("Constructing Principal with username: " + username); } + return new UsernamePrincipal(username); } else { @@ -160,8 +119,7 @@ public class ExternalSaslServer implements SaslServer { LOGGER.debug("Using external Principal: " + _externalPrincipal); } - return _externalPrincipal; } } -}
\ No newline at end of file +} diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java index a5d087593a..61506777c5 100644 --- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java +++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java @@ -114,7 +114,7 @@ public class ExternalAuthenticationManagerTest extends QpidTestCase result.getStatus()); assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals()); - // Null princial + // Null principal saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", null); result = _manager.authenticate(saslServer, new byte[0]); diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayerFactory.java b/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayerFactory.java index 478355edc1..bfd1ae8181 100644 --- a/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayerFactory.java +++ b/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayerFactory.java @@ -124,7 +124,7 @@ public class SecurityLayerFactory public String getUserID() { - return SSLUtil.retriveIdentity(_engine); + return SSLUtil.retrieveIdentity(_engine); } } diff --git a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java index a527c436f8..7553e3dad8 100644 --- a/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java +++ b/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java @@ -1,5 +1,5 @@ /* - * + * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -7,19 +7,22 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. - * + * */ package org.apache.qpid.transport.network.security.ssl; +import javax.naming.InvalidNameException; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; import org.apache.qpid.transport.TransportException; import org.apache.qpid.transport.util.Logger; @@ -50,21 +53,21 @@ public class SSLUtil Certificate cert = engine.getSession().getPeerCertificates()[0]; Principal p = ((X509Certificate)cert).getSubjectDN(); String dn = p.getName(); - String hostname = null; - + String hostname = null; + if (dn.contains("CN=")) { hostname = dn.substring(3, dn.indexOf(",") == -1? dn.length(): dn.indexOf(",")); - } - + } + if (log.isDebugEnabled()) { log.debug("Hostname expected : " + hostnameExpected); log.debug("Distinguished Name for server certificate : " + dn); log.debug("Host Name obtained from DN : " + hostname); } - + if (hostname != null && !(hostname.equalsIgnoreCase(hostnameExpected) || hostname.equalsIgnoreCase(hostnameExpected + ".localdomain"))) { @@ -72,60 +75,78 @@ public class SSLUtil " Expected : " + hostnameExpected + " Found in cert : " + hostname); } - + } catch(SSLPeerUnverifiedException e) { log.warn("Exception received while trying to verify hostname",e); // For some reason the SSL engine sets the handshake status to FINISH twice - // in succession. The first time the peer certificate + // in succession. The first time the peer certificate // info is not available. The second time it works ! // Therefore have no choice but to ignore the exception here. } } - - public static String retriveIdentity(SSLEngine engine) + + public static String getIdFromSubjectDN(String dn) { - StringBuffer id = new StringBuffer(); + String cnStr = null; + String dcStr = null; + if(dn == null) + { + return ""; + } + else + { + try + { + LdapName ln = new LdapName(dn); + for(Rdn rdn : ln.getRdns()) + { + if("CN".equalsIgnoreCase(rdn.getType())) + { + cnStr = rdn.getValue().toString(); + } + else if("DC".equalsIgnoreCase(rdn.getType())) + { + if(dcStr == null) + { + dcStr = rdn.getValue().toString(); + } + else + { + dcStr = rdn.getValue().toString() + '.' + dcStr; + } + } + } + return cnStr == null || cnStr.length()==0 ? "" : dcStr == null ? cnStr : cnStr + '@' + dcStr; + } + catch (InvalidNameException e) + { + log.warn("Invalid name: '"+dn+"'. "); + return ""; + } + } + } + + + public static String retrieveIdentity(SSLEngine engine) + { + String id = ""; + Certificate cert = engine.getSession().getLocalCertificates()[0]; + Principal p = ((X509Certificate)cert).getSubjectDN(); + String dn = p.getName(); try { - Certificate cert = engine.getSession().getLocalCertificates()[0]; - Principal p = ((X509Certificate)cert).getSubjectDN(); - String dn = p.getName(); - - if (dn.contains("CN=")) - { - String str = dn.substring(dn.indexOf("CN=")+3, dn.length()); - id.append(str.substring(0, - str.indexOf(",") == -1? str.length(): str.indexOf(","))); - } - - if (dn.contains("DC=")) - { - id.append("@"); - int c = 0; - for (String toks : dn.split(",")) - { - if (toks.contains("DC")) - { - if (c > 0) {id.append(".");} - id.append(toks.substring( - toks.indexOf("=")+1, - toks.indexOf(",") == -1? toks.length(): toks.indexOf(","))); - c++; - } - } - } + id = SSLUtil.getIdFromSubjectDN(dn); } - catch(Exception e) + catch (Exception e) { - log.info("Exception received while trying to retrive client identity from SSL cert",e); + log.info("Exception received while trying to retrive client identity from SSL cert", e); } - log.debug("Extracted Identity from client certificate : " + id); - return id.toString(); + return id; } - + public static KeyStore getInitializedKeyStore(String storePath, String storePassword, String keyStoreType) throws GeneralSecurityException, IOException { KeyStore ks = KeyStore.getInstance(keyStoreType); @@ -137,7 +158,7 @@ public class SSLUtil { in = new FileInputStream(f); } - else + else { in = Thread.currentThread().getContextClassLoader().getResourceAsStream(storePath); } diff --git a/java/common/src/test/java/org/apache/qpid/transport/network/security/ssl/SSLUtilTest.java b/java/common/src/test/java/org/apache/qpid/transport/network/security/ssl/SSLUtilTest.java new file mode 100644 index 0000000000..2d17f7a3c7 --- /dev/null +++ b/java/common/src/test/java/org/apache/qpid/transport/network/security/ssl/SSLUtilTest.java @@ -0,0 +1,52 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.qpid.transport.network.security.ssl; + +import org.apache.qpid.test.utils.QpidTestCase; + +public class SSLUtilTest extends QpidTestCase +{ + public void testGetIdFromSubjectDN() + { + // "normal" dn + assertEquals("user@somewhere.example.org",SSLUtil.getIdFromSubjectDN("cn=user,dc=somewhere,dc=example,dc=org")); + // quoting of values, case of types, spacing all ignored + assertEquals("user2@somewhere.example.org",SSLUtil.getIdFromSubjectDN("DC=somewhere, dc=example,cn=\"user2\",dc=org")); + // only first cn is used + assertEquals("user@somewhere.example.org",SSLUtil.getIdFromSubjectDN("DC=somewhere, dc=example,cn=\"user\",dc=org, cn=user2")); + // no cn, no Id + assertEquals("",SSLUtil.getIdFromSubjectDN("DC=somewhere, dc=example,dc=org")); + // cn in value is ignored + assertEquals("",SSLUtil.getIdFromSubjectDN("C=CZ,O=Scholz,OU=\"JAKUB CN=USER1\"")); + // cn with no dc gives just user + assertEquals("someone",SSLUtil.getIdFromSubjectDN("ou=someou, CN=\"someone\"")); + // null results in empty string + assertEquals("",SSLUtil.getIdFromSubjectDN(null)); + // invalid name results in empty string + assertEquals("",SSLUtil.getIdFromSubjectDN("ou=someou, =")); + // component containing whitespace + assertEquals("me@example.com",SSLUtil.getIdFromSubjectDN("CN=me,DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB")); + // empty CN + assertEquals("",SSLUtil.getIdFromSubjectDN("CN=,DC=somewhere, dc=example,dc=org")); + + + } +} |