From 40a2a28ebe38ad05fb69f301085d33ac735d8f76 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 23 Mar 2015 14:09:37 +0100 Subject: [PATCH] Enable -Werror and add XmppHostnameVerifier We now treat warnings as errors (-Werror). In order to do so, it was necessary to remove Java7HostnameVerifier since it depended on internal properietary API. XmppHostnameVerifier take the place of it. --- build.gradle | 1 + .../smack/util/IpAddressUtil.java | 59 +++++ .../smack/java7/Java7HostnameVerifier.java | 88 ------- .../smack/java7/Java7SmackInitializer.java | 2 +- .../smack/java7/XmppHostnameVerifier.java | 240 ++++++++++++++++++ 5 files changed, 301 insertions(+), 89 deletions(-) create mode 100644 smack-core/src/main/java/org/jivesoftware/smack/util/IpAddressUtil.java delete mode 100644 smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7HostnameVerifier.java create mode 100644 smack-java7/src/main/java/org/jivesoftware/smack/java7/XmppHostnameVerifier.java diff --git a/build.gradle b/build.gradle index a9ff5a787..8a648c493 100644 --- a/build.gradle +++ b/build.gradle @@ -118,6 +118,7 @@ allprojects { options.encoding = "utf8" options.compilerArgs = [ '-Xlint:all', + '-Werror', ] } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/IpAddressUtil.java b/smack-core/src/main/java/org/jivesoftware/smack/util/IpAddressUtil.java new file mode 100644 index 000000000..26c710acf --- /dev/null +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/IpAddressUtil.java @@ -0,0 +1,59 @@ +/** + * + * Copyright 2015 Florian Schmaus + * + * Licensed 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.jivesoftware.smack.util; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class IpAddressUtil { + + private static final Pattern IPV4_PATTERN = Pattern.compile("^(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})$"); + + public static boolean isIPv4LiteralAddress(String string) { + Matcher matcher = IPV4_PATTERN.matcher(string); + if (!matcher.matches()) { + return false; + } + + for (int i = 0; i < 3; i++) { + String ipSegment = matcher.group(i); + int ipSegmentInt; + try { + ipSegmentInt = Integer.valueOf(ipSegment); + } catch (NumberFormatException e) { + throw new AssertionError(e); + } + if (ipSegmentInt > 255) { + return false; + } + } + return true; + } + + public static boolean isIPv6LiteralAddress(final String string) { + final String[] octets = string.split(":"); + if (octets.length != 8) { + return false; + } + // TODO handle compressed zeros and validate octets + return true; + } + + public static boolean isIpAddress(String string) { + return isIPv4LiteralAddress(string) || isIPv6LiteralAddress(string); + } +} diff --git a/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7HostnameVerifier.java b/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7HostnameVerifier.java deleted file mode 100644 index 2d682acb1..000000000 --- a/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7HostnameVerifier.java +++ /dev/null @@ -1,88 +0,0 @@ -/** - * - * Copyright 2014 the original author or authors - * - * Licensed 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.jivesoftware.smack.java7; - -import java.security.Principal; -import java.security.cert.Certificate; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.security.auth.kerberos.KerberosPrincipal; - -import sun.security.util.HostnameChecker; - -/** - *

- * HostnameVerifier implementation which implements the same policy as the Java built-in - * pre-HostnameVerifier policy. - *

- *

- * Based on the work by Kevin - * Locke (released under CC0 1.0 Universal / Public Domain Dedication). - *

- */ -public class Java7HostnameVerifier implements HostnameVerifier { - - @Override - public boolean verify(String hostname, SSLSession session) { - HostnameChecker checker = HostnameChecker.getInstance(HostnameChecker.TYPE_TLS); - - boolean validCertificate = false, validPrincipal = false; - try { - Certificate[] peerCertificates = session.getPeerCertificates(); - - if (peerCertificates.length > 0 && peerCertificates[0] instanceof X509Certificate) { - X509Certificate peerCertificate = (X509Certificate) peerCertificates[0]; - - try { - checker.match(hostname, peerCertificate); - // Certificate matches hostname - validCertificate = true; - } - catch (CertificateException ex) { - // Certificate does not match hostname - } - } - else { - // Peer does not have any certificates or they aren't X.509 - } - } - catch (SSLPeerUnverifiedException ex) { - // Not using certificates for peers, try verifying the principal - try { - Principal peerPrincipal = session.getPeerPrincipal(); - if (peerPrincipal instanceof KerberosPrincipal) { - validPrincipal = HostnameChecker.match(hostname, - (KerberosPrincipal) peerPrincipal); - } - else { - // Can't verify principal, not Kerberos - } - } - catch (SSLPeerUnverifiedException ex2) { - // Can't verify principal, no principal - } - } - - return validCertificate || validPrincipal; - } -} diff --git a/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7SmackInitializer.java b/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7SmackInitializer.java index 482213219..a61ffd7f6 100644 --- a/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7SmackInitializer.java +++ b/smack-java7/src/main/java/org/jivesoftware/smack/java7/Java7SmackInitializer.java @@ -31,7 +31,7 @@ public class Java7SmackInitializer implements SmackInitializer { @Override public List initialize() { - SmackConfiguration.setDefaultHostnameVerifier(new Java7HostnameVerifier()); + SmackConfiguration.setDefaultHostnameVerifier(new XmppHostnameVerifier()); Base64.setEncoder(Java7Base64Encoder.getInstance()); Base64UrlSafeEncoder.setEncoder(Java7Base64UrlSafeEncoder.getInstance()); DNSUtil.setIdnaTransformer(new StringTransformer() { diff --git a/smack-java7/src/main/java/org/jivesoftware/smack/java7/XmppHostnameVerifier.java b/smack-java7/src/main/java/org/jivesoftware/smack/java7/XmppHostnameVerifier.java new file mode 100644 index 000000000..2935618d3 --- /dev/null +++ b/smack-java7/src/main/java/org/jivesoftware/smack/java7/XmppHostnameVerifier.java @@ -0,0 +1,240 @@ +/** + * + * Copyright 2015 Florian Schmaus + * + * Licensed 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.jivesoftware.smack.java7; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.security.Principal; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Collection; +import java.util.LinkedList; +import java.util.List; +import java.util.Locale; +import java.util.logging.Level; +import java.util.logging.Logger; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSession; +import javax.security.auth.kerberos.KerberosPrincipal; + +import org.jivesoftware.smack.util.IpAddressUtil; + +/** + * HostnameVerifier implementation for XMPP. + *

+ * Based on the work by Kevin + * Locke (released under CC0 1.0 Universal / Public Domain Dedication). + *

+ */ +public class XmppHostnameVerifier implements HostnameVerifier { + + private static final Logger LOGGER = Logger.getLogger(XmppHostnameVerifier.class.getName()); + + @Override + public boolean verify(String hostname, SSLSession session) { + boolean validCertificate = false, validPrincipal = false; + try { + Certificate[] peerCertificates = session.getPeerCertificates(); + if (peerCertificates.length == 0) { + return false; + } + if (!(peerCertificates[0] instanceof X509Certificate)) { + return false; + } + X509Certificate peerCertificate = (X509Certificate) peerCertificates[0]; + try { + match(hostname, peerCertificate); + // Certificate matches hostname + validCertificate = true; + } + catch (CertificateException e) { + LOGGER.log(Level.INFO, "Certificate does not match hostname", e); + } + } + catch (SSLPeerUnverifiedException e) { + // Not using certificates for peers, try verifying the principal + try { + Principal peerPrincipal = session.getPeerPrincipal(); + if (peerPrincipal instanceof KerberosPrincipal) { + validPrincipal = match(hostname, (KerberosPrincipal) peerPrincipal); + } + else { + LOGGER.info("Can't verify principal for " + hostname + ". Not kerberos"); + } + } + catch (SSLPeerUnverifiedException e2) { + LOGGER.log(Level.INFO, "Can't verify principal for " + hostname + ". Not kerberos", + e2); + } + } + + return validCertificate || validPrincipal; + } + + private static void match(String name, X509Certificate cert) throws CertificateException { + if (IpAddressUtil.isIpAddress(name)) { + matchIp(name, cert); + } + else { + matchDns(name, cert); + } + } + + private static boolean match(String name, KerberosPrincipal peerPrincipal) { + // TODO + LOGGER.warning("KerberosPrincipal validation not implemented yet. Can not verify " + name); + return false; + } + + private static final int ALTNAME_DNS = 2; + + private static void matchDns(String name, X509Certificate cert) throws CertificateException { + Collection> subjAltNames = cert.getSubjectAlternativeNames(); + if (subjAltNames != null) { + List nonMatchingDnsAltnames = new LinkedList<>(); + for (List san : subjAltNames) { + if (((Integer) san.get(0)).intValue() != ALTNAME_DNS) { + continue; + } + String dnsName = (String) san.get(1); + if (matchesPerRfc2818(name, dnsName)) { + return; + } + else { + nonMatchingDnsAltnames.add(dnsName); + } + } + if (!nonMatchingDnsAltnames.isEmpty()) { + // Reject if certificate contains subject alt names, but none of them matches + StringBuilder sb = new StringBuilder("No subject alternative DNS name matching " + + name + " found. Tried: "); + for (String nonMatchingDnsAltname : nonMatchingDnsAltnames) { + sb.append(nonMatchingDnsAltname).append(","); + } + throw new CertificateException(sb.toString()); + } + } + // TODO SubjectX500Name + throw new CertificateException("No name matching " + name + " found"); + } + + private static boolean matchesPerRfc2818(String name, String template) { + String[] nameParts = name.toLowerCase(Locale.US).split("."); + String[] templateParts = template.toLowerCase(Locale.US).split("."); + + if (nameParts.length != templateParts.length) { + return false; + } + + for (int i = 0; i < nameParts.length; i++) { + if (!matchWildCards(nameParts[i], templateParts[i])) { + return false; + } + } + + return true; + } + + /** + * Returns true if the name matches against the template that may contain the wildcard char '*'. + * + * @param name + * @param template + * @return true if name matches template. + */ + private static boolean matchWildCards(String name, String template) { + int wildcardIndex = template.indexOf("*"); + if (wildcardIndex == -1) { + return name.equals(template); + } + + boolean isBeginning = true; + String beforeWildcard = ""; + String afterWildcard = template; + while (wildcardIndex != -1) { + beforeWildcard = afterWildcard.substring(0, wildcardIndex); + afterWildcard = afterWildcard.substring(wildcardIndex + 1); + + int beforeStartIndex = name.indexOf(beforeWildcard); + if ((beforeStartIndex == -1) || (isBeginning && beforeStartIndex != 0)) { + return false; + } + isBeginning = false; + + name = name.substring(beforeStartIndex + beforeWildcard.length()); + wildcardIndex = afterWildcard.indexOf("*"); + } + + return name.endsWith(afterWildcard); + } + + private static final int ALTNAME_IP = 7; + + /** + * Check if the certificate allows use of the given IP address. + *

+ * From RFC2818 ยง 3.1: "In some cases, the URI is specified as an IP address rather than a + * hostname. In this case, the iPAddress subjectAltName must be present in the certificate and + * must exactly match the IP in the URI." + *

+ * + * @param expectedIP + * @param cert + * @throws CertificateException + */ + private static void matchIp(String expectedIP, X509Certificate cert) + throws CertificateException { + Collection> subjectAlternativeNames = cert.getSubjectAlternativeNames(); + if (subjectAlternativeNames == null) { + throw new CertificateException("No subject alternative names present"); + } + List nonMatchingIpAltnames = new LinkedList<>(); + for (List san : subjectAlternativeNames) { + if (((Integer) san.get(0)).intValue() != ALTNAME_IP) { + continue; + } + String ipAddress = (String) san.get(1); + if (expectedIP.equalsIgnoreCase(ipAddress)) { + return; + } + else { + try { + // See if the addresses match if we transform then, useful for IPv6 addresses + if (InetAddress.getByName(expectedIP).equals(InetAddress.getByName(ipAddress))) { + // expectedIP matches the given ipAddress, return + return; + } + } + catch (UnknownHostException | SecurityException e) { + LOGGER.log(Level.FINE, "Comparing IP strings failed", e); + } + } + nonMatchingIpAltnames.add(ipAddress); + } + StringBuilder sb = new StringBuilder("No subject alternative names matching IP address " + + expectedIP + " found. Tried: "); + for (String s : nonMatchingIpAltnames) { + sb.append(s).append(","); + } + throw new CertificateException(sb.toString()); + } +}