| Age | Commit message (Collapse) | Author |
|---|
|
When investigating problems on Solaris, attention focused on the
JavaConnection class, whose implementation appeared to be non-optimal.
The original implementation had a loop which operated on a
non-blocking socket, and looked something like this:
while (!disconnecting) {
while (something to write) {
write data to socket;
if write failed {
sleep(100); // and try again
}
}
try reading data from socket
if (any data was read) {
process data from socket;
}
sleep(100);
}
Because the socket is non-blocking, the reads/writes return straight
away. This means that even when no data is being transferred, the
loop is executing around ten times a second checking for any data to
read/write.
In one case (Solaris client talking to Solaris server on the same VM)
we were consistently able to get into a state where a write fails to
write any data, so that the "something to write" subloop never exits.
This in turn means that the "try reading data" section of the main
loop is never reached.
Investigation failed to uncover why this problem occurs. The
underlying socket appears to be returning EAGAIN (equivalent to
EWOULDBLOCK), suggesting that the write fails because the client's
local buffer is full. This in turn implies that the server isn't
reading data quickly enough, leading to the buffers on the client side
being full up. But this doesn't explain why, once things have got
into this state, they never free up.
At any rate, it was felt that the implementation above is not ideal
because it is relying on a polling mechanism that is not efficient,
rather than being event driven.
So this change re-implements JavaConnection to use a Selector, which
means that the main loop is event-driven. The new implementation
looks like this
while (!disconnected) {
wait for selector
if (disconnected) {
break;
}
if something to write {
try to write data;
}
if something to read {
try to read data;
}
if still something to write {
sleep(100);
post wake event; // so that next wait completes straight away
}
}
Test-information:
Testing appears to show that the problems we saw on Solaris are no
longer seen with this patch (Solaris tests still fail, but later on,
which appears to be due to a separate problem).
Testing shows that this leads to the thread spending much more time
idle, and only being active when data is being read/written (unlike
the original implementation which was looping ten times a second
regardless of whether any data was being read/written).
Testing using MLC seems to show the new implementation works OK.
I was unable to provoke the "write buffer not completely written"
case, so faked it by making the doWrite() method constrain its maximum
write size to 200 bytes. By doing this I verified that the "leftOver"
section of code was working properly (and incidentally fixed a problem
with the the initial implementation of the patch that had been passing
the wrong parameter to System.arrayCopy).
Change-Id: I5a6191567ba7e9afdb9a26febf00eae72b00f6eb
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
Making it Long allows it to hold an XML-unsignedLong value as well
as null values. Before this patch, it was an int and defaulted to 0.
This was not right as int is too small to hold number of seconds for
last activity time and primitive data types do not allow for null values.
Test-information:
tested using an XMPP client to query last IQ on MUC rooms
Change-Id: I6274403610bd60038fd7c235fad3bc2798f38e19
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
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>
|
|
Direct copy of current signal/slot implementation,
with 4 generic parameters.
Change-Id: I4b2cb37fd134e80e8481950030b6e8721f4f2854
|
|
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>
|
|
If since_ is null, calling clone on it was causing a NUll Pointer Exception.
Adding a check fixes it.
Test-information:
Tested by creating a room using an XMPP client - no exception seen after the fix
Change-Id: I25b151ac8e5b25562b8941eb5532fa9b9ea2de6f
|
|
Change-Id: I49cf4cba01452b291655dfccdc134180270c1ff3
|
|
Change-Id: I862e11dc293ce84e0311f1ad470293e07735aeaf
|
|
Change-Id: Ib02394df2c7bb818c2409b1d6f2fc3ad0d938224
|
|
Change-Id: Id2710c674abc19cdf2b37f97fe53288b86c7f367
|
|
Change-Id: Iab58df1cf6a3b8b9461b71fd3f27476214e07286
|
|
Change-Id: Iba3aeab8b0140c32f732ce01b1e2da243e7ec141
|
|
Change-Id: If5ef43f2d875f958cd8114b0b3246e6e6f03c95b
|
|
|
|
|
|
|
|
|
|
Makes ClientOptions do more.
|
|
|
|
|
|
|
|
The method was comparing the classes instead of instances as a result of which
equal JIDs but belonging to different derived class were always unequal.
Test-information:
Tested using an XMPP client which is using objects of class derived from JID
class to compare with raw JID objects
|
|
|
|
In order to make it available to clients.
Test-information:
tested using an XMPP Admin tool to display connection type error
|
|
In one of my testing scenario, socket was not getting closed. This was happening when
an XMPP client was connecting to a domain which was different from the domain in the
jabber ID of the connection user. Moving the close call to a finally block ensures that
socket gets closed in all scenarios.
Test-information:
I created an IM domain j.com on my XMPP Server and then added a user with that
domain (user@j.com). Then I tried connecting to my Primary domain using this
new user. After removing j.com, I could an increase in number of sockets after
every poll (coreclient.connect()) but not after this patch.
|
|
If I leave an Application using Stroke running for a few hours(making periodic
connection attempts), the JVM would throw an Exception saying "Too Many Open Files".
On doing an "lsof -p <pid_of_jvm>", I noticed that there were number of open
sockets in CLOSE_WAIT state and these went up after every attempt to do a
connect on CoreClient object.
Closing of DirContext object fixes this bug and the number of open sockets
does not increase.
Test-information:
Ran MLC and kept on monitoring the result of "lsof -p <pid>". It would not
increase after this patch.
|
|
|
|
|
|
|
|
Also adds a 'make test' target for the Makefile. Set the JUNIT environment variable to point to your jar if it doesn't find it.
|
|
This change ports the MUC Administration related classes from
Swiften to stroke. Also includes the MUC initialisation code in
the CoreClient.
Test-information:
tested the ported unit tests
|
|
This patch ports the classes for Storage, PrivateStorage and PrivateStorage
requests from Swiften to Stroke.
Test-information:
junit test for GetPrivateStorageRequestTest is also ported and tested
|
|
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 patch ports the MUC Payload parsers from swiften to stroke.
Test-information:
ported junits work fine
|
|
All the serializers for different kind of MUC payloads have
been ported from swiften to stroke.
Test-information:
There is a junit test that's ported which tests the admin payload serialiser.
Also executed the other MUC Junits.
|
|
This patch ports basic elements from swiftern to stroke.
This includes various types od MUC Payloads.
Test-information:
the junits for the parsers (still WIP) code works fine.
|
|
The porting includes Directed and Stanza Channel Presence senders.
Test-information:
tested with Work In Progress MUC Admin Port's Unit tests
|
|
MUC Admin requires Signals and Slots with 3 parameters so this patch adds
Signal/Slot classes which cater to 3 parameter values
Test-information:
tested with Work In Progress MUC Admin code
|
|
This patch adds a copy constructor to the Presence class(and hence base class
Staza as well). It also ports the compare method to JID class.
Also added javadocs to Presence and Stanza classes.
Test-information:
tested using the Work In Progress code that ports MUC Admin to stroke
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
Junit tests ported from Swiften to stroke.
Test-information:
ran the tests from Eclipse IDE
|
|
The javadoc for the method was not in line with its behaviour, so you
could get a NullPointerException if you asked for a session
certificate when the session wasn't TLS.
This patch makes the code do what the javadoc says (and what clients
most likely want)
Test-information:
Returns null rather than crashing when I ask for a certificate on a
non-TLS stream.
|
|
After this change, the error payload object should be populated in case of error.
The condtion, type and text field will be from the payload rather than Undefined,
Cancel and empty.
Test-information:
tested by executing adhoc-commands on an XMPP clinet in a way to result
in an error. I do see the error text and condition set as per the XMPP streams.
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
Corresponding with change in Swiften (assuming that is approved; it's
not yet been integrated at the time of writing)
Test-information:
Works as expected in my test applications
|
|
This change
- renames the "onError" signal to be "onDisconnected" (as per change
59be74ec6 in Swiften)
- adds "setCertificateTrustChecker()" method and uses the supplied checker
when configuring TLS
Test-information:
My applications still work.
When I configure my server with a certificate that doesn't correspond
to the requirements in RFC 6120, my CertificateTrustChecker gets
called, and the session is either dropped or maintained depending on
what my checker returns.
|
|
|
Swift