| Age | Commit message (Collapse) | Author |
|---|
|
Add missing methods and classes in the TLS and Base packages where
possible. In the case of TLSContextFactory the methods could not be
implemented in java so added a not saying they are not supported in
java.
Test-information:
Unit tests still build and run ok.
Change-Id: I9be2035f092875fcdc02644a3c0082739f26949a
|
|
Remove SHA1 and HMACSHA1 from StringCodecs as they are already provided by CryptoProvider,
and is equivalent to Swiften.
License:
This patch is BSD-licensed, see Documentation/Licenses/BSD-simplified.txt for details.
Test-Information:
Add tests for Base64 and PBKDF2, which passes.
Change-Id: Ife05f185a10a79c9d69a12235f1b0397d022d992
|
|
Adds TLSError and TLSOptions.
Updates BasicSessionStream, SessionStream and Session.
Updates Client and Components to accomodate changes in TLS.
Also completes TLSLayer in StreamStack which was pending due to TLS port.
License:
This patch is BSD-licensed, see Documentation/Licenses/BSD-simplified.txt for details.
Test-Information:
Tests added for Certificate and ServerIdentityVerifier.
Test updated for ComponentSession.
All tests pass.
Change-Id: I34a8fe068c1e8af5348cc4ab49d3d1ed118ae833
|
|
TLSLayer could not be updated because it requires TLS to be ported first.
Updates other classes, only for having compatibility with SafeByteArray because of updates in Stream Stack.
License:
This patch is BSD-licensed, see Documentation/Licenses/BSD-simplified.txt for details.
Test-Information:
Tests added for StreamStack and XMPPLayer, which passes.
Change-Id: I8707fc1f16d622d2a90f6f39f671b7e7c46aa170
|
|
The code is currently doing
bytesConsumed += lastConsumed;
inside one of the clauses in a while loop, and then the same operation
again when the while loop exits. So there is a risk that the
"bytesConsumed" value will be too large.
In fact the only place the value of "bytesConsumed" is used is by some
code that checks whether it's greater than zero, so it would not be
problematic if the value were too large.
It seems worth fixing this in case future changes rely on the
"bytesConsumed" value being accurate.
Test-information:
inspection only
Change-Id: Ibd6fd01417afc4c4e030a5173bfba9a02980a757
signed-off-by:robert.williams@isode.com
|
|
Android Lollipop implementation reports this
innaccurately during handshaking, which was
causing SSL setup to fail. Now derives the
information directly from ByteBuffer positions.
This change is based on a similar change for Jetty at
https://gist.github.com/tavianator/96e9daab0fd1a45f11f2/8cbbfe028c97cac63d3b39f575b2c317d7a174c2
Test-information:
Tested Harrier with IMAPS and STARTTLS against MBox on Android
4.4 and 5.0 emulators, all connect and authenticate OK.
Tested MLC on Linux, with extra debugging added - everything works,
and the values returned for "lastConsumed" are the same as those
returned by "bytesConsumed()" so the change appears to make no
difference to behaviour for non-Android
Ran unit tests - no failures.
Change-Id: I54b3850136b35535918b0eb303409232d64a60b5
Reviewer: Nick Hudson <nick.hudson@isode.com>
|
|
The POODLE vulnerability means that using SSLv3 is insecure. So this
change removes it from the list of protocols that JSSEContext may use.
Oracle's "Java Cryptography Architecture Standard Algorithm
Name Documentation"
http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html
Lists the "standard names" that can be used in this context:
SSLv2
SSLv3
TLSv1
TLSv1.1
TLSv1.2
SSLv2Hello
After this patch, only the three "TLS" protocols will be allowed.
Test-information:
Tested using JRE6 and JRE7; viewing the SSL handshake indicates that
the protocol being requested is being used when the handshake occurs
Change-Id: I99710a72a4b8567226b1205fdf64c6c67ccc2a9a
|
|
Until now, Stroke would not do trust anchor checking because there was
no suitable way to getting to a default trust store.
This patch makes stroke use JDK's default trust store for looking up
trust anchors. If it can find the trust anchor in JDK's store, it
proceeds to do validy check. If any check fails, an error is set
and it is upto the client to decide if client is happy with certificate.
Test-information:
I tested with with an XMPP client MLC.
I got prompted with cert for server whose CA was not in Java Trust Store.
After adding the CA to JDK trust store, no prompt was seen
I then renewed the certificte with validity = 2 minutes.
On doing a connection, MLC prompted me because the certificate was expired
even though the CA was in the trust store.
Change-Id: Id3fc86d85641f07814ff8621b8bf038cde406063
Reviewer: Nick Hudson <nick.hudson@isode.com>
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
Since the initial Stroke TLS implementation was done, some changes
were made in Swiften, starting with
"Show Certificate dialog from certificate error window."
159e773b156f531575d0d7e241e2d20c85ee6d7cA
which mean that certificate verification uses the peer's certificate
chain, and not just the peer's EE certificate.
This change updates Stroke so that its API now more closely matches
what Swiften does.
Note that any current Stroke clients that implement the
"CertificateTrustChecker" interface will break, as this patch makes an
incompatible change to that interface, requiring implementing classes
to handle a certificate chain rather than a single certificate.
Isode copyright notices are updated; Remko copyright notices are
updated to reflect the current copyright notices in any equivalent
Swiften source files.
Test-information:
Used MLC (after having patched it for CertificateTrustChecker changes)
and verified that it sees the entire certificate chain coming back.
Ran self-tests for Stroke and saw no junit failures
Change-Id: I3d863f929bfed3324446cadf3bb4d6b9ff916660
|
|
Old code was casting Object[] to String[], which may be safe,
but is dependant on the Set's internal implementation of
toArray, and may lead to ClassCastExceptions. We now
preallocate a String[] to avoid the cast and force type
safety for any implementation.
Test-information:
Was crashing when enabling restricted ciphers on Android. Now
works OK.
Change-Id: I759a369449296f1819e91a25aa123b083ec280c9
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
If a TLS connection results in the server choosing an anonymous cipher
suite, then no server certificate will be returned by the server.
This ought not to happen, since XMPP clients are expected only to
propose non-anonymous cipher suites, but it could be that a client is
coded to propose anonymous suites, or that a bug in the server means
that it fails to return a server certificate.
This change updates the ServerIdentityVerifier to make it resilient
against these situations, treating this situation as equivalent to
"certificate presented by server does not verify".
Test-information:
In my testing, I was deliberately using anonymous ciphers and getting
Stroke crashes. After this patch, I don't get Stroke crashes any more
(but the connection fails because the certificate verification fails).
Change-Id: Ia7b9b8dad7a054ff266a78ef33a56157320654c8
|
|
It was noticed that in certain cases, Stroke got stuck when connecting
to a server over TLS, and the server closed down the connection.
Investigation showed that this appears to be caused by the JSSEContext
code not properly coping with a "close notify" SSL message. What
happens in this case is that the SSLEngine generates a response to the
server's "close notify", and expects this to be sent back over the
network.
The original JSSEContext code saw the "CLOSED" status from the
SSLEngine.unwrap(), but assumed that no more data would be generated
by the engine (which was wrong, because the engine wants to send a close
response back), and so got stuck in a loop.
This patch therefore fixes the JSSEContext code to deal properly when
it sees a CLOSED from unwrap.
After the close has been received, an error will be emitted by
JSSEContext so that the application knows that the SSLEngine can no
longer be used (in practice we have always seen the socket closing,
which generates its own error to the application, but it was
recommended that we should have this check in case a server sends a
close notify and does NOT close the socket as well).
It appears that many servers don't actually send the "close notify",
and just drop the connection, which is (presumably) why we'd not seen
this behaviour before.
Test-information:
Tested by connecting to the aforementioned server. This time, when
the connection times out (and the closenotify is sent), we no longer
see a loop, but the application realises what's happens and attempts
to reconnect.
I have been running with this patch in my copy of MLC for two weeks
and have noticed no difference in behaviour - so far as I can tell the
code is not exercised when talking to M-Link but at any rate the patch
isn't causing anything to break.
Change-Id: Id007c923c510ef1b4ce53192105b00296c65c757
|
|
Some implementations of SSLEngine (notably Apache harmony used in
Android) never return the FINSHED status from calls to wrap or unwrap,
causing the TLSLayer to never emit its completed signal.
With this change, we treat a return of NOT_HANDSHAKING as equivalent
to FINISHED. The NOT_HANDSHAKING will never happen before handshaking
has finished, because the status during handshaking should always be
NEED_WRAP, NEED_UNWRAP, or NEED_TASK.
Test-information:
Tested with OracleJDK and OpenJDK using Isode M-Link Console to ensure
that the behaviour when negotiating TLS is unchanged (debugging shows
that in these cases it always sees the FINISHED status).
Tested on Android. Without this patch TLS handshakes don't complete;
with the patch, they do.
Change-Id: Ied2989cb2a3458dc6b1d2584dcc6c722d18e1355
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
By default, when a TLS connection is established, the SSLContext will
enable all available ciphersuites. This may not be appropriate in
situations where export restrictions apply and higher grade
ciphersuites are prohibitied.
This change allows a caller to configure a restricted set of
ciphersuites to be used when establishing TLS connections.
Callers use the JSSEContextFactory.setRestrictedCipherSuites() method
to configure a list of ciphersuites. Any ciphersuites which are not
included in the list will be excluded in subsequent TLS connections.
If the JSSEContextFactory.setRestrictedCipherSuites() is never called,
or called with a null parameter, then no restriction will apply.
Test-information:
Validated that by calling the new method to restrict the available
ciphers, TLS connections initiated by Stroke only propose ciphersuites
in the restricted list, and connections fail when the server fails to
find an acceptable cipher.
Change-Id: Id0b4b19553a6f386cda27a71f0172410d899218e
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
This patch adds a new "CAPICertificate" class, which can be used to
configure TLS connections that use a client certificate from a Windows
CAPI keystore, including certificates on smart cards.
The JSSEContext class is updated so that "setClientCertificate()"
checks to see whether the CertificateWithKey object that it's been
given is a PKCS12Certificate or a CAPICertificate, and initializes the
appropriate type of KeyStore.
Note that the default behaviour of the KeyStore returned by SunMSCAPI
when choosing a client certificate for TLS authentication is for it to
choose the "most suitable" certificate it finds.
This "most suitable" certificate may not be the one that the user has
chosen, and in fact various certificates in CAPI are not considered by
SunMSCAPI in this case - for example, certificates issued by CAs who
don't appear in the list of acceptable CAs in the server's
CertificateRequest (RFC5246 7.4.4).
The CAPIKeyManager class provided here allows a caller to override the
default behaviour, and force the use of a specific client certificate
(whether it's "suitable" or not) based on the value specified by the
caller when the CAPICertificate object was created.
This also means that it is possible for a user to specify a particular
certificate and use that, even if SunMSCAPI would have thought a "more
suitable" one was found in CAPI.
Test-information:
Tested that P12 based TLS still works
Tested on Windows that I can specify a "CAPICertificate" which is a
reference to a certificate in the Windows keystore whose private key
is held on a smartcard, and that I am prompted to insert the card (if
necessary() and enter the PIN before the TLS handshake proceeds.
Tested on Windows that I can specify a "CAPICertificate" which is a
reference to an imported P12 file where certificate and key are in
CAPI, and the TLS handshake proceeds without asking me for a PIN
Tested that the "CAPIKeyManager" class is correctly forcing use of the
certificate specified by the user, rather than the one which would be
returned by the default SunMSCAPI implementation.
Tested that I can still use "PKCS12Certificate"s to authenticate
Tested that if I try and use a CAPICertificate on a non-Windows
platform, then I can't authenticate, and get errors emitted from Stroke
complaining of "no such provider: SunMSCAPI"
Change-Id: Iff38e459f60c0806755820f6989c516be37cbf08
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
Two things
- the implementation of JavaTrustManager was attempting to instantiate a
TrustManagerFactory with a hard-coded name of "PKIX", which doesn't
work on Android. So instead of that, we ask for the
TrustManagerFactory's default algorithm - which for the standard JRE
still appears to be "PKIX", but which for Android may be something
else.
- the "hack" which had been in place to force the SSLEngine to
perform a TLS handshake has been removed.
Calling "SSLEngine.beginHandshake()" is not guaranteed to make the
SSLEngine perform the TLS handshake, which it typically only does when
it is told to wrap some data from the client. The earlier version of
JSSEContext provoked this by asking it to send a "<" character, and
then removing the leading "<" from whatever Stroke happened to send next.
It turns out that you can force the handshake to start by telling the
SSLEngine to wrap 0 bytes of data from the client, and so this change
removes the hack, and instead calls "wrapAndSendData()" with an empty
buffer as soon as the SSLEngine has been created.
Test-information:
Ran XMPP client that uses TLS and verified that everything still works
as expected.
Change-Id: Ie08d76bd2f5a743320a59bad62a09c1f215c48d6
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
The original implementation of JSSEContext allocated buffers for the
SSL data which started off at a size determined using information from
the SSLSession, and which were allowed to grow to up to ten times
their original size.
When testing with jabber.org it was fairly easy to exceed this size
(e.g. requiring a buffer of ~650K where the maximum value had been
around 160K), meaning that applications would fail.
This change removes the upper limit altogether. Now, the buffer will
grow to whatever is required, so long as free memory is available.
I also renamed "enlargeBuffer" in response to a previous review comment
Test-information:
No longer see problems when talking to jabber.org over
SSL. Instrumentation shows that buffer is growing as expected.
|
|
This change provides the functionality to allow clients to specify a
PKCS#12 file containing client certificate/key for use when starting
TLS sessions.
The PKCS12Certificate class now subclasses "CertificateWithKey"
(matching the Swiften implementation).
Swiften also has "CAPICertificate", which is another subclass of
CertificateWithKey. This has not been provided in this patch.
From a client's point of view, all that's necessary to specify a
certificate to be used for TLS is to do something like
CertificateWithKey myCert = new PKCS12Certificate(
"/home/fred/myp12file.p12",
"secret".toCharArray());
coreClient.setCertificate(myCert);
before calling "CoreClient.connect".
Matching the Swiften functionality, constructing a new
PKCS12Certificate does not actually perform validation of the P12
file/passphrase; that takes place when the p12 file is used.
There is limited scope for returning to the caller errors describing
possible problems, but JSSEContext uses the "emitError" method which
does maintain error information, which is available in a debugger, or
from the JSSEContext.toString() method.
Test-information:
Set up an M-Link server with TLS verified that
- when I specify a client certificate with suitable SAN, the client
sends it and the server reports authentication using the certificate
- when I specify a client certificate without a suitable SAN, the
client sends it but the server rejects it
|
|
The nascent support for TLS is now enabled by the uncommenting of a
line in "PlatformTLSFactories" which means that Stroke will now try
and negotiate TLS when connecting to a server that offers it.
Note that further changes will be required to allow configuring of
client certificate and trust anchors.
In performing testing, a couple of problems were found and have been
fixed by this patch:
- The "hack" field inside JSSEContext, which keeps track of whether
the fake "<" character used to provoke an SSL handshake has been
sent was mistakenly declared static, which meant that if you tried
using TLS on more than one session, things didn't work
properly. This has been fixed.
- The buffer used for incoming encrypted data for the SSLEngine in
JSSEContext is created with a size that matches "the largest
SSL/TLS packet that is expected". But it turns out not to be big
enough to cope with all the data that the JavaConnection class
might provide when calling "handleDataRead()".
So the "handleDataFromNetwork" method is changed to break this data
into chunks that will fit into the buffer. The same technique is
used in "handleDataFromApplication" for cases where the application
provides more data than is will fit in a buffer.
- All of the "ByteBuffer" values are initialised with a size as
recommended by the Sun documentation, although in some cases it
appears that these sizes may not be enough (you are cautioned to be
able to cope with the buffers overflowing)
So all of the ByteBuffers are able to grow, up to a maximum of ten times
there initial size, using the "enlargeBuffer()" method.
Note that in most cases, I could only provoke buffer overflows in
my tests by deliberately starting off with buffers that are too
small.
- When testing with JRE7, it became apparent that the behaviour of
the SSLEngine and SSLContext classes had changed, which initially
resulted in "hangs" being seen as the SSLEngine did not appear to
decrypt data being fed to it until subsequent SSL messages arrived
and appeared and to prod it into life.
This behaviour is influenced by the version of TLS handshake being
used, which made it awkward to debug, since some versions of TLS handshake
worked fine for JRE6 but not JRE7 and vice versa; also different servers
would negotiate different with different handshakes.
Eventually this turned out to be a pre-existing bug in the initial
JSSEContext implementation: specifically the "unwrapPendingData()"
method had been assuming that a call to SSLEngine.unwrap() would
consume all pending data (which is the case for in all scenarios
using JRE6, and is often, but not always, the case for JRE7).
So the fix for the problem is to loop inside "unwrapPendingData" until
calls to unwrap() don't consume any more data.
- I also added some logging to JSSEContext - warnings when an error
is emitted, and a "fine" message when buffer sizes have to be
increased.
- Also, double-slash comments are replaced by /*..*/ style in JSSEContext
Test-information:
Before this patch, TLS wasn't starting. Now it does.
Before the bug fixes, concurrent TLS connections to more than one
server resulted in "corruption" of the streams, with errors being
generated relating to XML parsing errors at both client/server.
Before the bug fixes, large messages from the server (~36K) would
cause "BufferOverflow" exceptions and connections to drop.
After the bug fixes, these problems are no longer seen.
Before the bug fixes, TLS sessions would sometimes (depending on what
version of TLS the server negotiated, and what version of JRE you were
using) appear to "hang". Now they don't.
I also tested creating artificially small buffers to make sure that
the various "buffer overflow" situations are handled properly. I
wasn't able to provoke all of these problems in a real
configuration, so I suspect that the "enlargeBuffer" stuff may not
actually get used much, but it has been tested.
All tested with JRE6 and JRE7
|
|
The JavaCertificate class functionality copies that found in Swiften's
OpenSSLCertificate class. One of the things that the class needs to
do is make available lists of SRV and XMPP names which are contained
in the certificate's subjectAltName extensions.
However, unlike OpenSSL, the standard Java classes don't provide
support for parsing the different types of subjectAltNames that we
want - what Java provides is a method to return the DER encoded
values, and so this change adds functions to the JavaCertificate class
to parse these values to extract Strings corresponding to either
XMPP or SRV subjectAltNames.
Also addressed a couple of comments from a previous patch.
Test-information:
Tested with certificate that has subjectAltNames for
- Service Name _xmpp-client.funky.isode.net
- Service Name _xmpp-server.funky.isode.net
- XMPP address funky.isode.net
- DNS Name funky.isode.net
And verified that all of these are parsed and made available with the
relevant methods.
Verified that a certificate which does NOT contain any of these
subjectAltNames is parsed with no errors (and the code reports no
matching subjectAltNames)
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
Hopefully the changes speak for themselves.
Some feedback relating to JavaCertificate has not been addressed; that
will be done in a separate patch
Test-information:
Can still establish sessions with / without TLS
|
|
Note that TLS won't be enabled with this patch unless you uncomment the
change in PlatformTLSFactories. With that comment removed, then a new
CoreClient session will attempt to negotiate TLS if the server supports
it.
Further changes are required to support this properly, as there
appears not to be comprehensive support in the CoreClient class for
dealing with situations when the server's certificate is not acceptable.
There's also no support yet for setting up client certificates.
Further changes will also be needed (see below) to support full
parsing of subjectAltNames from server certificates.
Significant changes are as follows
- TLSProceed - FIXME comments removed
- JavaConnection - changed so that it reads bytes from the socket's
InputStream, rather than reading chars and then constructing a
String out of them from which a byte array is then extracted.
While this seemed to work for non-binary data (e.g. non-encrypted
XMPP sessions), it breaks when you start sending binary (i.e. TLS)
data.
- JavaTLSConnectionFactory - implemented
- PlatformTLSFactories - By having this return a JSSEContextFactory, then
this will cause the client to try TLS if possible. But because other
changes are needed to make this work properly, the current code still
returns null.
- JSSEContext - new class which uses an SSLEngine to handle TLS handshake
and subsequent encryption/decryption. This is the main substance of
the SSL implementation
Note the "hack" in here to cope with SSLEngine requiring that some data
be sent from the application before it will do a TLS handshake
- JSSEContextFactory - just creates JSSEContexts
- JavaCertificate - this wraps an X509Certificate and does *some* of the
parsing of a certificate to look for stuff that is expected when
verifying an XMPP server certificate (RFC 6120 and RFC 6125). Note that
the JDK classes for parsing certificates don't provide an easy way
to decode "OTHER" subjectAltNames, and so this implementation does
not find XMPP or SRV subjectaltnames from the server certificate. This
will need extra work.
- JavaTrustManager - obtains the server certificate from the TLS handshake
and verifies it. Currently the only verification done is to check that
it's in date. More work will be needed to perform proper validation
- Where necessary, Remko's copyright comments were changed from GNU to
"All rights reserved". Isode copyright notices updated to "2012"
Test-information:
Set up XMPP server with its own certificate, and checked that TLS gets
negotiated and starts OK (provided the server cert contains e.g. a DNS
subjectAltName matching its own name). Subsequent operation appears
to be as expected.
|
|
|
|
|
Swift