From 4622d00d9e6e1c1c93ec42a0741e291ddf7542b7 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 17 Feb 2022 11:58:02 +0100 Subject: [PATCH 1/3] [tcp] Unravel SSLSocketFactory.createSocket() invocation --- .../java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index c1dc8b7ee..e8de5e131 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -48,6 +48,7 @@ import javax.net.SocketFactory; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; import org.jivesoftware.smack.AbstractXMPPConnection; import org.jivesoftware.smack.ConnectionConfiguration; @@ -713,9 +714,11 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { SmackTlsContext smackTlsContext = getSmackTlsContext(); Socket plain = socket; + int port = plain.getPort(); + String xmppServiceDomainString = config.getXMPPServiceDomain().toString(); + SSLSocketFactory sslSocketFactory = smackTlsContext.sslContext.getSocketFactory(); // Secure the plain connection - socket = smackTlsContext.sslContext.getSocketFactory().createSocket(plain, - config.getXMPPServiceDomain().toString(), plain.getPort(), true); + socket = sslSocketFactory.createSocket(plain, xmppServiceDomainString, port, true); final SSLSocket sslSocket = (SSLSocket) socket; // Immediately set the enabled SSL protocols and ciphers. See SMACK-712 why this is From ba2e36dbc3ac75cf8e5e6c2962481e105dd3b81a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 17 Feb 2022 11:59:03 +0100 Subject: [PATCH 2/3] [core] Deprecate some SSLContext config options and add KeyManager option Smack historically provided fine-grained options for the SSLContext. This is however not flexible enough, as an option to specifiy the KeyManager(s) was missing. This deprecated the options for keystore path, keystore type, and PKCS#11 library, in favor of an option to set the KeyManager, which could be aware of the keystore path and type, and the PKCS#11 library. At some point, Smack may provide some high-level methods to create a KeyManager from provided keystore path, keytsore type and PKCS#11 library. --- .../smack/ConnectionConfiguration.java | 213 ++++++++++++------ 1 file changed, 145 insertions(+), 68 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/ConnectionConfiguration.java b/smack-core/src/main/java/org/jivesoftware/smack/ConnectionConfiguration.java index 2a0165009..e8689d24f 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/ConnectionConfiguration.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/ConnectionConfiguration.java @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software, 2017-2020 Florian Schmaus. + * Copyright 2003-2007 Jive Software, 2017-2022 Florian Schmaus. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.Provider; +import java.security.SecureRandom; import java.security.Security; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; @@ -189,7 +190,7 @@ public abstract class ConnectionConfiguration { protected ConnectionConfiguration(Builder builder) { try { smackTlsContext = getSmackTlsContext(builder.dnssecMode, builder.sslContextFactory, - builder.customX509TrustManager, builder.keystoreType, builder.keystorePath, + builder.customX509TrustManager, builder.keyManagers, builder.sslContextSecureRandom, builder.keystoreType, builder.keystorePath, builder.callbackHandler, builder.pkcs11Library); } catch (UnrecoverableKeyException | KeyManagementException | NoSuchAlgorithmException | CertificateException | KeyStoreException | NoSuchProviderException | IOException | NoSuchMethodException @@ -252,7 +253,7 @@ public abstract class ConnectionConfiguration { } private static SmackTlsContext getSmackTlsContext(DnssecMode dnssecMode, SslContextFactory sslContextFactory, - X509TrustManager trustManager, String keystoreType, String keystorePath, + X509TrustManager trustManager, KeyManager[] keyManagers, SecureRandom secureRandom, String keystoreType, String keystorePath, CallbackHandler callbackHandler, String pkcs11Library) throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException, NoSuchProviderException, UnrecoverableKeyException, KeyManagementException, UnsupportedCallbackException, @@ -266,69 +267,10 @@ public abstract class ConnectionConfiguration { context = SSLContext.getInstance("TLS"); } - KeyStore ks = null; - PasswordCallback pcb = null; - KeyManager[] kms = null; - - if ("PKCS11".equals(keystoreType)) { - Constructor c = Class.forName("sun.security.pkcs11.SunPKCS11").getConstructor(InputStream.class); - String pkcs11Config = "name = SmartCard\nlibrary = " + pkcs11Library; - ByteArrayInputStream config = new ByteArrayInputStream(pkcs11Config.getBytes(StandardCharsets.UTF_8)); - Provider p = (Provider) c.newInstance(config); - Security.addProvider(p); - ks = KeyStore.getInstance("PKCS11", p); - pcb = new PasswordCallback("PKCS11 Password: ", false); - callbackHandler.handle(new Callback[] { pcb }); - ks.load(null, pcb.getPassword()); - } else if ("Apple".equals(keystoreType)) { - ks = KeyStore.getInstance("KeychainStore", "Apple"); - ks.load(null, null); - // pcb = new PasswordCallback("Apple Keychain",false); - // pcb.setPassword(null); - } else if (keystoreType != null) { - ks = KeyStore.getInstance(keystoreType); - if (callbackHandler != null && StringUtils.isNotEmpty(keystorePath)) { - pcb = new PasswordCallback("Keystore Password: ", false); - callbackHandler.handle(new Callback[] { pcb }); - ks.load(new FileInputStream(keystorePath), pcb.getPassword()); - } else { - InputStream stream = TLSUtils.getDefaultTruststoreStreamIfPossible(); - try { - // Note that PKCS12 keystores need a password one some Java platforms. Hence we try the famous - // 'changeit' here. See https://bugs.openjdk.java.net/browse/JDK-8194702 - char[] password = "changeit".toCharArray(); - try { - ks.load(stream, password); - } finally { - CloseableUtil.maybeClose(stream); - } - } catch (IOException e) { - LOGGER.log(Level.FINE, "KeyStore load() threw, attempting 'jks' fallback", e); - - ks = KeyStore.getInstance("jks"); - // Open the stream again, so that we read it from the beginning. - stream = TLSUtils.getDefaultTruststoreStreamIfPossible(); - try { - ks.load(stream, null); - } finally { - CloseableUtil.maybeClose(stream); - } - } - } - } - - if (ks != null) { - String keyManagerFactoryAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); - KeyManagerFactory kmf = KeyManagerFactory.getInstance(keyManagerFactoryAlgorithm); - if (kmf != null) { - if (pcb == null) { - kmf.init(ks, null); - } else { - kmf.init(ks, pcb.getPassword()); - pcb.clearPassword(); - } - kms = kmf.getKeyManagers(); - } + // TODO: Remove the block below once we removed setKeystorePath(), setKeystoreType(), setCallbackHanlder() and + // setPKCS11Library() in the builder, and all related fields and the parameters of this function. + if (keyManagers == null) { + keyManagers = Builder.getKeyManagersFrom(keystoreType, keystorePath, callbackHandler, pkcs11Library); } SmackDaneVerifier daneVerifier = null; @@ -343,7 +285,7 @@ public abstract class ConnectionConfiguration { } // User requested DANE verification. - daneVerifier.init(context, kms, trustManager, null); + daneVerifier.init(context, keyManagers, trustManager, secureRandom); } else { final TrustManager[] trustManagers; if (trustManager != null) { @@ -354,7 +296,7 @@ public abstract class ConnectionConfiguration { trustManagers = null; } - context.init(kms, trustManagers, null); + context.init(keyManagers, trustManagers, secureRandom); } return new SmackTlsContext(context, daneVerifier); @@ -688,6 +630,8 @@ public abstract class ConnectionConfiguration { public abstract static class Builder, C extends ConnectionConfiguration> { private SecurityMode securityMode = SecurityMode.required; private DnssecMode dnssecMode = DnssecMode.disabled; + private KeyManager[] keyManagers; + private SecureRandom sslContextSecureRandom; private String keystorePath; private String keystoreType; private String pkcs11Library = "pkcs11.config"; @@ -942,7 +886,12 @@ public abstract class ConnectionConfiguration { * @param callbackHandler to obtain information, such as the password or * principal information during the SASL authentication. * @return a reference to this builder. + * @deprecated set a callback-handler aware {@link KeyManager} via {@link #setKeyManager(KeyManager)} or + * {@link #setKeyManagers(KeyManager[])}, created by + * {@link #getKeyManagersFrom(String, String, CallbackHandler, String)}, instead. */ + // TODO: Remove in Smack 4.6. + @Deprecated public B setCallbackHandler(CallbackHandler callbackHandler) { this.callbackHandler = callbackHandler; return getThis(); @@ -970,6 +919,47 @@ public abstract class ConnectionConfiguration { return getThis(); } + /** + * Set the {@link KeyManager}s to initialize the {@link SSLContext} used by Smack to establish the XMPP connection. + * + * @param keyManagers an array of {@link KeyManager}s to initialize the {@link SSLContext} with. + * @return a reference to this builder. + * @since 4.4.5 + */ + public B setKeyManagers(KeyManager[] keyManagers) { + this.keyManagers = keyManagers; + return getThis(); + } + + /** + * Set the {@link KeyManager}s to initialize the {@link SSLContext} used by Smack to establish the XMPP connection. + * + * @param keyManager the {@link KeyManager}s to initialize the {@link SSLContext} with. + * @return a reference to this builder. + * @see #setKeyManagers(KeyManager[]) + * @since 4.4.5 + */ + public B setKeyManager(KeyManager keyManager) { + KeyManager[] keyManagers = new KeyManager[] { keyManager }; + return setKeyManagers(keyManagers); + } + + /** + * Set the {@link SecureRandom} used to initialize the {@link SSLContext} used by Smack to establish the XMPP + * connection. Note that you usually do not need (nor want) to set this. Because if the {@link SecureRandom} is + * not explicitly set, Smack will initialize the {@link SSLContext} with null as + * {@link SecureRandom} argument. And all sane {@link SSLContext} implementations will then select a safe secure + * random source by default. + * + * @param secureRandom the {@link SecureRandom} to initialize the {@link SSLContext} with. + * @return a reference to this builder. + * @since 4.4.5 + */ + public B setSslContextSecureRandom(SecureRandom secureRandom) { + this.sslContextSecureRandom = secureRandom; + return getThis(); + } + /** * Sets the path to the keystore file. The key store file contains the * certificates that may be used to authenticate the client to the server, @@ -977,7 +967,12 @@ public abstract class ConnectionConfiguration { * * @param keystorePath the path to the keystore file. * @return a reference to this builder. + * @deprecated set a keystore-path aware {@link KeyManager} via {@link #setKeyManager(KeyManager)} or + * {@link #setKeyManagers(KeyManager[])}, created by + * {@link #getKeyManagersFrom(String, String, CallbackHandler, String)}, instead. */ + // TODO: Remove in Smack 4.6. + @Deprecated public B setKeystorePath(String keystorePath) { this.keystorePath = keystorePath; return getThis(); @@ -988,7 +983,12 @@ public abstract class ConnectionConfiguration { * * @param keystoreType the keystore type. * @return a reference to this builder. + * @deprecated set a key-type aware {@link KeyManager} via {@link #setKeyManager(KeyManager)} or + * {@link #setKeyManagers(KeyManager[])}, created by + * {@link #getKeyManagersFrom(String, String, CallbackHandler, String)}, instead. */ + // TODO: Remove in Smack 4.6. + @Deprecated public B setKeystoreType(String keystoreType) { this.keystoreType = keystoreType; return getThis(); @@ -1000,7 +1000,12 @@ public abstract class ConnectionConfiguration { * * @param pkcs11Library the path to the PKCS11 library file. * @return a reference to this builder. + * @deprecated set a PKCS11-library aware {@link KeyManager} via {@link #setKeyManager(KeyManager)} or + * {@link #setKeyManagers(KeyManager[])}, created by + * {@link #getKeyManagersFrom(String, String, CallbackHandler, String)}, instead. */ + // TODO: Remove in Smack 4.6. + @Deprecated public B setPKCS11Library(String pkcs11Library) { this.pkcs11Library = pkcs11Library; return getThis(); @@ -1276,5 +1281,77 @@ public abstract class ConnectionConfiguration { public abstract C build(); protected abstract B getThis(); + + public static KeyManager[] getKeyManagersFrom(String keystoreType, String keystorePath, + CallbackHandler callbackHandler, String pkcs11Library) + throws NoSuchMethodException, SecurityException, ClassNotFoundException, KeyStoreException, + NoSuchProviderException, NoSuchAlgorithmException, CertificateException, IOException, InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, UnsupportedCallbackException, UnrecoverableKeyException { + KeyManager[] keyManagers = null; + KeyStore ks = null; + PasswordCallback pcb = null; + + if ("PKCS11".equals(keystoreType)) { + Constructor c = Class.forName("sun.security.pkcs11.SunPKCS11").getConstructor(InputStream.class); + String pkcs11Config = "name = SmartCard\nlibrary = " + pkcs11Library; + ByteArrayInputStream config = new ByteArrayInputStream(pkcs11Config.getBytes(StandardCharsets.UTF_8)); + Provider p = (Provider) c.newInstance(config); + Security.addProvider(p); + ks = KeyStore.getInstance("PKCS11", p); + pcb = new PasswordCallback("PKCS11 Password: ", false); + callbackHandler.handle(new Callback[] { pcb }); + ks.load(null, pcb.getPassword()); + } else if ("Apple".equals(keystoreType)) { + ks = KeyStore.getInstance("KeychainStore", "Apple"); + ks.load(null, null); + // pcb = new PasswordCallback("Apple Keychain",false); + // pcb.setPassword(null); + } else if (keystoreType != null) { + ks = KeyStore.getInstance(keystoreType); + if (callbackHandler != null && StringUtils.isNotEmpty(keystorePath)) { + pcb = new PasswordCallback("Keystore Password: ", false); + callbackHandler.handle(new Callback[] { pcb }); + ks.load(new FileInputStream(keystorePath), pcb.getPassword()); + } else { + InputStream stream = TLSUtils.getDefaultTruststoreStreamIfPossible(); + try { + // Note that PKCS12 keystores need a password one some Java platforms. Hence we try the famous + // 'changeit' here. See https://bugs.openjdk.java.net/browse/JDK-8194702 + char[] password = "changeit".toCharArray(); + try { + ks.load(stream, password); + } finally { + CloseableUtil.maybeClose(stream); + } + } catch (IOException e) { + LOGGER.log(Level.FINE, "KeyStore load() threw, attempting 'jks' fallback", e); + + ks = KeyStore.getInstance("jks"); + // Open the stream again, so that we read it from the beginning. + stream = TLSUtils.getDefaultTruststoreStreamIfPossible(); + try { + ks.load(stream, null); + } finally { + CloseableUtil.maybeClose(stream); + } + } + } + } + + if (ks != null) { + String keyManagerFactoryAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); + KeyManagerFactory kmf = KeyManagerFactory.getInstance(keyManagerFactoryAlgorithm); + if (kmf != null) { + if (pcb == null) { + kmf.init(ks, null); + } else { + kmf.init(ks, pcb.getPassword()); + pcb.clearPassword(); + } + keyManagers = kmf.getKeyManagers(); + } + } + + return keyManagers; + } } } From 4d026d8ae84819184a8cad93f65630fe6e2dca8c Mon Sep 17 00:00:00 2001 From: cmeng-git Date: Sun, 13 Feb 2022 15:58:37 +0800 Subject: [PATCH 3/3] [jingle] Add element and text to JingleReason's XML --- .../org/jivesoftware/smackx/jingle/element/JingleReason.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleReason.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleReason.java index 116c8375d..32a9e7d0f 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleReason.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleReason.java @@ -35,6 +35,7 @@ public class JingleReason implements FullyQualifiedElement { public static final String ELEMENT = "reason"; public static final String NAMESPACE = Jingle.NAMESPACE; + public static final String TEXT_ELEMENT = "text"; public static AlternativeSession AlternativeSession(String sessionId) { return new AlternativeSession(sessionId); @@ -142,7 +143,7 @@ public class JingleReason implements FullyQualifiedElement { /** * An optional element that provides more detailed machine-readable information about the reason for the action. * - * @return an elemnet with machine-readable information about this reason or null. + * @return an element with machine-readable information about this reason or null. * @since 4.4.5 */ public ExtensionElement getElement() { @@ -155,6 +156,8 @@ public class JingleReason implements FullyQualifiedElement { xml.rightAngleBracket(); xml.emptyElement(reason); + xml.optElement(TEXT_ELEMENT, text); + xml.optAppend(element); xml.closeElement(this); return xml;