Merge pull request #1031 from guusdk/OF-1495_Add-SANs

OF-1495: Add subject alternative names to certs and CSRs

src/i18n/openfire_i18n_en.properties

@@ -2383,6 +2383,9 @@ ssl.certificates.keystore.restart_server=Certificates were modified so HTTP serv
 ssl.certificates.keystore.io_error=Unable to access the certificate key store. The file may be corrupt.
 ssl.certificates.keystore.no_installed=One or more certificates are missing. Click {0}here{1} to generate self-signed \
   certificates or {2}here{3} to import a signed certificate and its private key.
+ssl.certificates.keystore.no_complete_installed=The installed certificates do not cover all of the identities of this \
+  server. Click {0}here{1} to replace your certificates with self-signed certificates that do, or {2}here{3} to import \
+  a signed certificate and its private key.
 ssl.certificates.keystore.error_importing-reply=An error occurred while importing the Certificate Authority reply. \
   Verify that the reply is correct and that it belongs to the correct certificate.
 ssl.certificates.keystore.issuer-updated=Issuer information updated successfully.

src/java/org/jivesoftware/openfire/keystore/IdentityStore.java

@@ -13,6 +13,7 @@ import java.io.IOException;
 import java.security.*;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
+import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
 import java.util.*;
 
@@ -110,7 +111,7 @@ public class IdentityStore extends CertificateStore
 
             return pemCSR;
         }
-        catch ( IOException | KeyStoreException | UnrecoverableKeyException | NoSuchAlgorithmException | OperatorCreationException e )
+        catch ( IOException | KeyStoreException | UnrecoverableKeyException | NoSuchAlgorithmException | OperatorCreationException | CertificateParsingException e )
         {
             throw new CertificateStoreConfigException( "Cannot generate CSR for alias '"+ alias +"'", e );
         }
@@ -442,6 +443,69 @@ public class IdentityStore extends CertificateStore
         }
     }
 
+    /**
+     * Checks if the store contains a certificate of a particular algorithm that contains at least all of the identities
+     * of this server (which includes the XMPP domain name, but also its hostname, and XMPP addresses of components
+     * that are currently being hosted).
+     *
+     * This method will not distinguish between self-signed and non-self-signed certificates.
+     */
+    public synchronized boolean containsAllIdentityCertificate( String algorithm ) throws CertificateStoreConfigException
+    {
+        final Collection<String> dns = CertificateManager.determineSubjectAlternateNameDnsNameValues();
+
+        try
+        {
+            for ( final String alias : Collections.list( store.aliases() ) )
+            {
+                final Set<String> missingDns = new HashSet<>();
+
+                final Certificate certificate = store.getCertificate( alias );
+                if ( !( certificate instanceof X509Certificate ) )
+                {
+                    continue;
+                }
+
+                if ( !certificate.getPublicKey().getAlgorithm().equalsIgnoreCase( algorithm ) )
+                {
+                    continue;
+                }
+
+                final List<String> serverIdentities = CertificateManager.getServerIdentities( (X509Certificate) certificate );
+
+                // Are all of our DNS names covered?
+                for ( String dnsId : dns )
+                {
+                    boolean found = false;
+                    for ( String identity : serverIdentities )
+                    {
+                        if ( !DNSUtil.isNameCoveredByPattern( dnsId, identity ) )
+                        {
+                            found = true;
+                            break;
+                        }
+                    }
+
+                    if ( !found )
+                    {
+                        Log.info( "Certificate with alias '{}' is missing DNS identity '{}'.", alias, dnsId );
+                        missingDns.add( dnsId );
+                    }
+                }
+
+                if ( missingDns.isEmpty() )
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+        catch ( KeyStoreException e )
+        {
+            throw new CertificateStoreConfigException( "An exception occurred while searching for " + algorithm + " certificates that match the Openfire domain.", e );
+        }
+    }
+
     /**
      * Populates the key store with a self-signed certificate for the domain of this XMPP service.
      */
@@ -469,6 +533,7 @@ public class IdentityStore extends CertificateStore
         final String name = JiveGlobals.getProperty( "xmpp.domain" ).toLowerCase();
         final String alias = name + "_" + algorithm.toLowerCase();
         final int validityInDays = 5*365;
+        final Set<String> sanDnsNames = CertificateManager.determineSubjectAlternateNameDnsNameValues();
 
         Log.info( "Generating a new private key and corresponding self-signed certificate for domain name '{}', using the {} algorithm (sign-algorithm: {} with a key size of {} bits). Certificate will be valid for {} days.", name, algorithm, signAlgorithm, keySize, validityInDays );
         // Generate public and private keys
@@ -477,7 +542,7 @@ public class IdentityStore extends CertificateStore
             final KeyPair keyPair = generateKeyPair( algorithm.toUpperCase(), keySize );
 
             // Create X509 certificate with keys and specified domain
-            final X509Certificate cert = CertificateManager.createX509V3Certificate( keyPair, validityInDays, name, name, name, signAlgorithm );
+            final X509Certificate cert = CertificateManager.createX509V3Certificate( keyPair, validityInDays, name, name, name, signAlgorithm, sanDnsNames );
 
             // Store new certificate and private key in the key store
             store.setKeyEntry( alias, keyPair.getPrivate(), configuration.getPassword(), new X509Certificate[]{cert} );

src/java/org/jivesoftware/util/CertificateManager.java

@@ -17,11 +17,13 @@
 package org.jivesoftware.util;
 
 import org.bouncycastle.asn1.*;
+import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers;
 import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
 import org.bouncycastle.asn1.x500.X500Name;
 import org.bouncycastle.asn1.x500.X500NameBuilder;
 import org.bouncycastle.asn1.x500.style.BCStyle;
 import org.bouncycastle.asn1.x509.Extension;
+import org.bouncycastle.asn1.x509.ExtensionsGenerator;
 import org.bouncycastle.asn1.x509.GeneralName;
 import org.bouncycastle.asn1.x509.GeneralNames;
 import org.bouncycastle.cert.CertException;
@@ -47,6 +49,8 @@ import org.bouncycastle.pkcs.PKCSException;
 import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequestBuilder;
 import org.bouncycastle.util.io.pem.PemObjectGenerator;
 import org.bouncycastle.util.io.pem.PemWriter;
+import org.jivesoftware.openfire.XMPPServer;
+import org.jivesoftware.openfire.disco.DiscoItem;
 import org.jivesoftware.openfire.keystore.CertificateStore;
 import org.jivesoftware.openfire.keystore.CertificateUtils;
 import org.jivesoftware.util.cert.CNCertificateIdentityMapping;
@@ -61,6 +65,7 @@ import java.nio.charset.StandardCharsets;
 import java.security.*;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
+import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
 import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -232,13 +237,85 @@ public class CertificateManager {
      * @param privKey the private key of the certificate.
      * @return the content of a new singing request for the specified certificate.
      */
-    public static String createSigningRequest(X509Certificate cert, PrivateKey privKey) throws OperatorCreationException, IOException {
-
+    public static String createSigningRequest(X509Certificate cert, PrivateKey privKey) throws OperatorCreationException, IOException, CertificateParsingException
+    {
         JcaPKCS10CertificationRequestBuilder csrBuilder = new JcaPKCS10CertificationRequestBuilder( //
                 cert.getSubjectX500Principal(), //
                 cert.getPublicKey() //
                 );
 
+        // Add SubjectAlternativeNames (SANs)
+        final ASN1EncodableVector subjectAlternativeNames = new ASN1EncodableVector();
+
+        final Collection<List<?>> certSans = cert.getSubjectAlternativeNames();
+        if ( certSans != null )
+        {
+            for ( final List<?> certSan : certSans )
+            {
+                final int nameType = (Integer) certSan.get( 0 );
+                final Object value = certSan.get( 1 ); // this is either a string, or a byte-array that represents the ASN.1 DER encoded form.
+                switch ( nameType )
+                {
+                    case 0:
+                        // OtherName: search for "id-on-xmppAddr" or 'sRVName' or 'userPrincipalName'
+                        try ( final ASN1InputStream decoder = new ASN1InputStream( (byte[]) value ) )
+                        {
+                            // By specification, OtherName instances must always be an ASN.1 Sequence.
+                            final ASN1Primitive object = decoder.readObject();
+                            final ASN1Sequence otherNameSeq = (ASN1Sequence) object;
+
+                            // By specification, an OtherName instance consists of:
+                            // - the type-id (which is an Object Identifier), followed by:
+                            // - a tagged value, of which the tag number is 0 (zero) and the value is defined by the type-id.
+                            final ASN1ObjectIdentifier typeId = (ASN1ObjectIdentifier) otherNameSeq.getObjectAt( 0 );
+                            final ASN1TaggedObject taggedValue = (ASN1TaggedObject) otherNameSeq.getObjectAt( 1 );
+
+                            final int tagNo = taggedValue.getTagNo();
+                            if ( tagNo != 0 )
+                            {
+                                throw new IllegalArgumentException( "subjectAltName 'otherName' sequence's second object is expected to be a tagged value of which the tag number is 0. The tag number that was detected: " + tagNo );
+                            }
+                            subjectAlternativeNames.add(
+                                new DERTaggedObject( false,
+                                                     GeneralName.otherName,
+                                                     new DERSequence(
+                                                         new ASN1Encodable[] {
+                                                             typeId,
+                                                             taggedValue
+                                                         }
+                                                     )
+                                )
+                            );
+                        }
+                        catch ( Exception e )
+                        {
+                            Log.warn( "Unable to parse certificate SAN 'otherName' value", e );
+                        }
+                        break;
+                    case 2:
+                        // DNS
+                        subjectAlternativeNames.add( new GeneralName( GeneralName.dNSName, (String) value ) );
+                        break;
+                    case 6:
+                        // URI
+                        subjectAlternativeNames.add( new GeneralName( GeneralName.uniformResourceIdentifier, (String) value ) );
+                        break;
+                    default:
+                        // Not applicable to XMPP, so silently ignore them
+                        break;
+                }
+
+            }
+        }
+
+        final GeneralNames subjectAltNames = GeneralNames.getInstance(
+            new DERSequence( subjectAlternativeNames )
+        );
+
+        final ExtensionsGenerator extGen = new ExtensionsGenerator();
+        extGen.addExtension(Extension.subjectAlternativeName, false, subjectAltNames);
+        csrBuilder.addAttribute( PKCSObjectIdentifiers.pkcs_9_at_extensionRequest, extGen.generate());
+
         String signatureAlgorithm = "SHA256WITH" + cert.getPublicKey().getAlgorithm();
 
         ContentSigner signer = new JcaContentSignerBuilder(signatureAlgorithm).build(privKey);
@@ -443,6 +520,13 @@ public class CertificateManager {
                                                                         String subjectCommonName, String domain,
                                                                         String signAlgoritm)
             throws GeneralSecurityException, IOException {
+        return createX509V3Certificate( kp, days, issuerCommonName, subjectCommonName, domain, signAlgoritm, null );
+    }
+
+    public static synchronized X509Certificate createX509V3Certificate(KeyPair kp, int days, String issuerCommonName,
+                                                                        String subjectCommonName, String domain,
+                                                                        String signAlgoritm, Set<String> sanDnsNames)
+            throws GeneralSecurityException, IOException {
 
         // subjectDN
         X500NameBuilder subjectBuilder = new X500NameBuilder();
@@ -452,7 +536,7 @@ public class CertificateManager {
         X500NameBuilder issuerBuilder = new X500NameBuilder();
         issuerBuilder.addRDN(BCStyle.CN, issuerCommonName);
 
-        return createX509V3Certificate(kp, days, issuerBuilder, subjectBuilder, domain, signAlgoritm);
+        return createX509V3Certificate(kp, days, issuerBuilder, subjectBuilder, domain, signAlgoritm, sanDnsNames);
     }
 
     /**
@@ -469,7 +553,13 @@ public class CertificateManager {
      * @throws IOException
      */
     public static synchronized X509Certificate createX509V3Certificate(KeyPair kp, int days, X500NameBuilder issuerBuilder,
-            X500NameBuilder subjectBuilder, String domain, String signAlgoritm) throws GeneralSecurityException, IOException {
+            X500NameBuilder subjectBuilder, String domain, String signAlgoritm ) throws GeneralSecurityException, IOException
+    {
+        return createX509V3Certificate( kp, days, issuerBuilder, subjectBuilder, domain, signAlgoritm, null );
+    }
+
+    public static synchronized X509Certificate createX509V3Certificate(KeyPair kp, int days, X500NameBuilder issuerBuilder,
+            X500NameBuilder subjectBuilder, String domain, String signAlgoritm, Set<String> sanDnsNames ) throws GeneralSecurityException, IOException {
         PublicKey pubKey = kp.getPublic();
         PrivateKey privKey = kp.getPrivate();
 
@@ -492,18 +582,11 @@ public class CertificateManager {
                 pubKey //
                 );
 
-        // add subjectAlternativeName extension
-        boolean critical = subjectDN.getRDNs().length == 0;
-        ASN1Sequence othernameSequence = new DERSequence(
-            new ASN1Encodable[] {
-                new ASN1ObjectIdentifier("1.3.6.1.5.5.7.8.5"),
-                new DERTaggedObject( true, GeneralName.otherName, new DERUTF8String( domain ) )
-            }
-        );
-        DERTaggedObject othernameGN = new DERTaggedObject(false, GeneralName.otherName, othernameSequence);
+        // add subjectAlternativeName extension that includes all relevant names.
+        final GeneralNames subjectAlternativeNames = getSubjectAlternativeNames( sanDnsNames );
 
-        GeneralNames subjectAltNames = GeneralNames.getInstance( new DERSequence( othernameGN ) );
-        certBuilder.addExtension(Extension.subjectAlternativeName, critical, subjectAltNames);
+        final boolean critical = subjectDN.getRDNs().length == 0;
+        certBuilder.addExtension(Extension.subjectAlternativeName, critical, subjectAlternativeNames);
 
         // add keyIdentifiers extensions
         JcaX509ExtensionUtils utils = new JcaX509ExtensionUtils();
@@ -532,4 +615,50 @@ public class CertificateManager {
             throw new GeneralSecurityException(e);
         }
     }
+
+    protected static GeneralNames getSubjectAlternativeNames( Set<String> sanDnsNames )
+    {
+        final ASN1EncodableVector subjectAlternativeNames = new ASN1EncodableVector();
+        if ( sanDnsNames != null )
+        {
+            for ( final String dnsNameValue : sanDnsNames )
+            {
+                subjectAlternativeNames.add(
+                    new GeneralName( GeneralName.dNSName, dnsNameValue )
+                );
+            }
+        }
+
+        return GeneralNames.getInstance(
+            new DERSequence( subjectAlternativeNames )
+        );
+    }
+
+    /**
+     * Finds all values that aught to be added as a Subject Alternate Name of the dnsName type to a certificate that
+     * identifies this XMPP server.
+     *
+     * @return A set of names, possibly empty, never null.
+     */
+    public static Set<String> determineSubjectAlternateNameDnsNameValues()
+    {
+        final HashSet<String> result = new HashSet<>();
+
+        // Add the XMPP domain name itself.
+        result.add( XMPPServer.getInstance().getServerInfo().getXMPPDomain() );
+
+        // The fully qualified domain name of the server
+        result.add( XMPPServer.getInstance().getServerInfo().getHostname() );
+
+        if ( XMPPServer.getInstance().getIQDiscoItemsHandler() != null ) // When we're not in setup any longer...
+        {
+            // Add the name of each of the domain level item nodes as reported by service discovery.
+            for ( final DiscoItem item : XMPPServer.getInstance().getIQDiscoItemsHandler().getServerItems() )
+            {
+                result.add( item.getJID().toBareJID() );
+            }
+        }
+
+        return result;
+    }
 }

src/web/security-keystore-signing-request.jsp

@@ -17,6 +17,8 @@
 <%@ page import="java.util.HashMap" %>
 <%@ page import="java.util.Map" %>
 <%@ page import="org.jivesoftware.openfire.spi.ConnectionType" %>
+<%@ page import="org.jivesoftware.openfire.keystore.CertificateUtils" %>
+<%@ page import="java.util.Set" %>
 <%@ taglib uri="admin" prefix="admin" %>
 <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
 <%@ taglib uri="http://java.sun.com/jsp/jstl/fmt" prefix="fmt" %>
@@ -114,7 +116,8 @@
                        int days = 60;
 
                        // Regenerate self-sign certs whose subjectDN matches the issuerDN and set the new issuerDN
-                       X509Certificate newCertificate = CertificateManager.createX509V3Certificate(new KeyPair(pubKey, privKey), days, builder, builder, domain, signAlgoritm);
+                       final Set<String> sanDnsNames = CertificateManager.determineSubjectAlternateNameDnsNameValues();
+                       X509Certificate newCertificate = CertificateManager.createX509V3Certificate(new KeyPair(pubKey, privKey), days, builder, builder, domain, signAlgoritm, sanDnsNames);
                        keyStore.setKeyEntry(alias, privKey, identityStore.getConfiguration().getPassword(), new X509Certificate[] { newCertificate });
                    }
                }

src/web/security-keystore.jsp

 <%@page import="org.jivesoftware.util.StringUtils"%>
-<%@page import="java.util.LinkedHashMap"%>
 <%@page import="java.security.PrivateKey"%>
 <%@page import="org.jivesoftware.util.CertificateManager"%>
 <%@ page import="org.jivesoftware.util.CookieUtils" %>
@@ -12,10 +11,7 @@
 <%@ page import="org.jivesoftware.openfire.spi.ConnectionType" %>
 <%@ page import="org.jivesoftware.util.ParamUtils" %>
 <%@ page import="java.security.cert.X509Certificate" %>
-<%@ page import="java.util.Collections" %>
-<%@ page import="java.util.HashMap" %>
-<%@ page import="java.util.Map" %>
-<%@ page import="java.util.Set" %>
+<%@ page import="java.util.*" %>
 
 <%@ taglib uri="admin" prefix="admin" %>
 <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
@@ -28,6 +24,7 @@
 
 <% // Get parameters:
     boolean generate          = ParamUtils.getBooleanParameter(request, "generate");
+    boolean generateFull      = ParamUtils.getBooleanParameter(request, "generateFull");
     boolean delete            = ParamUtils.getBooleanParameter(request, "delete");
     boolean importReply       = ParamUtils.getBooleanParameter(request, "importReply");
     final String alias              = ParamUtils.getParameter( request, "alias" );
@@ -37,9 +34,10 @@
     Cookie csrfCookie = CookieUtils.getCookie(request, "csrf");
     String csrfParam = ParamUtils.getParameter(request, "csrf");
 
-    if (generate | delete | importReply) {
+    if (generate | generateFull | delete | importReply) {
         if (csrfCookie == null || csrfParam == null || !csrfCookie.getValue().equals(csrfParam)) {
             generate = false;
+            generateFull = false;
             delete = false;
             importReply = false;
             errors.put("csrf", "CSRF Failure!");
@@ -78,6 +76,8 @@
 
         pageContext.setAttribute( "validRSACert", identityStore.containsDomainCertificate( "RSA" ) );
         pageContext.setAttribute( "validDSACert", identityStore.containsDomainCertificate( "DSA" ) );
+        pageContext.setAttribute( "allIDRSACert", identityStore.containsAllIdentityCertificate( "RSA" ) );
+        pageContext.setAttribute( "allIDDSACert", identityStore.containsAllIdentityCertificate( "DSA" ) );
 
         if ( delete )
         {
@@ -108,7 +108,6 @@
 
 
     if (generate) {
-        String domain = XMPPServer.getInstance().getServerInfo().getXMPPDomain();
         try {
             if (errors.containsKey("ioerror") || !identityStore.containsDomainCertificate("DSA")) {
                 identityStore.addSelfSignedDomainCertificate("DSA");
@@ -128,6 +127,25 @@
         }
     }
 
+    if (generateFull) {
+        try {
+            if (!identityStore.containsAllIdentityCertificate("DSA")) {
+                identityStore.addSelfSignedDomainCertificate("DSA");
+            }
+            if (!identityStore.containsAllIdentityCertificate("RSA")) {
+                identityStore.addSelfSignedDomainCertificate("RSA");
+            }
+            // Save new certificates into the key store
+            identityStore.persist();
+            // Log the event
+            webManager.logEvent("generated SSL self-signed certs", null);
+            response.sendRedirect("security-keystore.jsp?connectionType="+connectionType);
+            return;
+        } catch (Exception e) {
+            e.printStackTrace();
+            errors.put("generate", e.getMessage());
+        }
+    }
     if (importReply) {
         String reply = ParamUtils.getParameter(request, "reply");
         if (alias != null && reply != null && reply.trim().length() > 0) {
@@ -187,16 +205,29 @@
             </admin:infobox>
         </c:forEach>
 
-        <c:if test="${not validDSACert or not validRSACert}">
-            <admin:infobox type="warning">
-                <fmt:message key="ssl.certificates.keystore.no_installed">
-                    <fmt:param value="<a href='security-keystore.jsp?csrf=${csrf}&generate=true&connectionType=${connectionType}'>"/>
-                    <fmt:param value="</a>"/>
-                    <fmt:param value="<a href='import-keystore-certificate.jsp?connectionType=${connectionType}'>"/>
-                    <fmt:param value="</a>"/>
-                </fmt:message>
-            </admin:infobox>
-        </c:if>
+        <c:choose>
+            <c:when test="${not validDSACert or not validRSACert}">
+                <admin:infobox type="warning">
+                    <fmt:message key="ssl.certificates.keystore.no_installed">
+                        <fmt:param value="<a href='security-keystore.jsp?csrf=${csrf}&generate=true&connectionType=${connectionType}'>"/>
+                        <fmt:param value="</a>"/>
+                        <fmt:param value="<a href='import-keystore-certificate.jsp?connectionType=${connectionType}'>"/>
+                        <fmt:param value="</a>"/>
+                    </fmt:message>
+                </admin:infobox>
+            </c:when>
+            <c:when test="${not allIDDSACert or not allIDRSACert}">
+                <admin:infobox type="info">
+                    <fmt:message key="ssl.certificates.keystore.no_complete_installed">
+                        <fmt:param value="<a href='security-keystore.jsp?csrf=${csrf}&generateFull=true&connectionType=${connectionType}'>"/>
+                        <fmt:param value="</a>"/>
+                        <fmt:param value="<a href='import-keystore-certificate.jsp?connectionType=${connectionType}'>"/>
+                        <fmt:param value="</a>"/>
+                    </fmt:message>
+                </admin:infobox>
+            </c:when>
+
+        </c:choose>
 
         <c:if test="${param.addupdatesuccess}"><admin:infobox type="success"><fmt:message key="ssl.certificates.added_updated"/></admin:infobox></c:if>
         <c:if test="${param.generatesuccess}"><admin:infobox type="success"><fmt:message key="ssl.certificates.generated"/></admin:infobox></c:if>