Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
OF-2611: Add unit tests for outbound server session (igniterealtime#2165
) * OF-2611: Refactor PKIX unit tests Adjusts unit tests and their utility methods that test functionality around TLS certificates: - Key size and algorithm, as well as the signature algorithm are now based on constants (allows for them to be changed faster) - Utilities that generate a certificate(chain) now return a holder object that returns both the certificate(chain) as well as the keypair that they were generated from. - Added more extensions to generated certifiates for them to represent certificates used in the wild better. - Additional methods to generate various certificate chains (eg: ones that will generate a chain with an expired intermediate certificate). These improvements facilitate future unit test development (which will be added in the next few commits) * OF-2611: (Draft) unit test for outbound server session * Workaround for OF-2592 * fix: remove wrong cert name, invalid intermediate and root certificates from tests and RemoteServerDummy * RemoteServerDummy config option: disable dialback feature The dummy class used to represent a remote server when testing outbound S2S connections can now be configured to avoid supporting the Dialback authentication mechanism. * RemoteServerDummy config option: disable TLS feature The dummy class used to represent a remote server when testing outbound S2S connections can now be configured to avoid supporting the TLS encryption and authentication mechanism. * RemoteServerDummy should offer Dialback when not authenticated Instead of offering Dialback when there's no TLS encryption, the RemoteServerDummy test tool should offer Dialback whenever the peer is not authenticated. * fix: remove self signed cert test * feat: add Junit 4 parameterised test * OF-2611: Add parameterized local outgoing server session test This commit takes the individual unit tests from the pre-exising LocalOutgoingServerSessionTest, and turns them into one parameterized test. Of this test, the server config (both from the local and remote server) are the arguments. * OF-2611: Modify LocalOutgoingServerSessionTest to use a locally invalid certificate To test outbound connections, the test has been modified to be able to send an invalid (expired) certificate. The dummy server responds to this by rejecting it. * OF-2611: LocalOutBoundServerSessionTest's dummy peer should support an optional TLS policy Previous to this commit, the dummy used for the test could support or disable TLS. For some tests, optional support is desirable. That's added by this commit. * OF-2611: RemoteServerDummy should not allow Dialback when TLS is required, but missing If TLS is required, but not established, the RemoteServerDummy should not allow Dialback authentication (as authentication must follow encryption). * OF-2611: LocalOutgoingServerSessionParameterizedTest's 'invalid' cert should really be invalid This bumps up a 'sleep' to help ensure that a recently generated cert is expired. I do not like this approach. It's based on timing, which does is a brittle (and slow) approach. * OF-2611: RemoteServerDummy should do basic SASL EXTERNAL checking When testing TLS authentication, RemoteServerDummy should do some basic checking of the provided certificates, instead of blindly accepting everything. * fix: add certificate validation to RemoteServerDummy * chore: delete Junit4 parameterised tests * test: add invalid certificate and private key to test fixtures * fix: remove possible leaking state between tests, change invalid local server certificate implementation * fix: increase numeric replication in parameterised tests * test: ignore original attempt at outgoing session tests * feat: add strictCertificateValidation option to admin console and connection settings * fix: fix 8 outgoing server tests that setup a plain dialback connection when it should make no connection * chore: delete old test file * fix: make checkbox render only on s2s page and not the c2s page * Allow Remote test dummy to deal with missing certs Relax client authentication rules in the Remote dummy to just indicate that we would like to authenticate the client, but if client certificates are self-signed or have no certificate chain then we are still good * test: WIP - add parameterised test for incoming server session * fix: log message more accurate when SSLHandshakeException thrown with strictCertificateValidation enabled * chore: add to javadocs * chore: add javadocs for strictCertificateValidation methods * chore: change names of test fixtures * OF-2611: Scaffolding for LocalIncomingServerSessionTest This refactors the existing _outgoing_ server session test, to reuse some of its implementation for an _inbound_ test. An initial inbound test class has been added, which is far from functional * OF-2611: Renamed LocalOutgoingServerSessionParameterizedTest -> LocalOutgoingServerSessionTest * OF-2611: Phase out ServerSettings.EncryptionPolicy EncryptionPolicy was created for unit tests. It duplicates Connection.TLSPolicy. Use the latter instead. * OF-2611: Re-enable all LocalIncomingServerSession unit tests This reverts a temporary limitation used during development. * OF-2611: LocalIncomingServerSession unit test should wait until 'done'. Prior to this commit, the unit test for an inbound server session waited for a certain period of time, assuming that the test had run by that time. In this commit, a structure is introduced that allows the test to explicitly flag the 'done' status. This should improve the time it takes to execute tests. * OF-2611: LocalIncomingServerSession unit test: define a session that's not authenticated as 'no connection' Due to the nature of this test, it's possible for Openfire to keep open a session, while the local test has deduced that it cannot continue. Although the session is not 'null', it still isn't properly set-up. This commit allows a session that is established, but NOT authenticated to pass the 'NO CONNECTION' definition. * OF-2611: Prevent NPE in LocalInboundServerSession unit test Prevent null pointers when interacting with a dialback handler that never was initialized. * OF-2611: LocalIncomingServerSession unit test should wait until 'done'. Removes a hack that was used to work-around the missing 'done' check. This commit can be fixed-up with the commit that introduces this behavior (~3 commits prior to this one). * OF-2611: Make unit test configuration repeat in the same order. * OF-2622: Do not accept inbound Server Dialback when disabled If the Server Dialback feature is disabled, Openfire should not allow peers to authenticate with that authentication mechanism. Additionally, Openfire should not define the corresponding XML namespace when the feature is disabled, as other servers might use that to determine support. * OF-2611: Improve XML parsing This change allows for a root element with child elements to be parsed. Note that an XML snippet that contains several elements (without a shared root) still can't be parsed. * OF-2611: Add TLS support to LocalIncomingServerSessionTest This adds support for encryption and SASL EXTERNAL to the incoming unit tests for S2S. With these changes, 4 of out of the 324 still fail. I'm unsure if this is caused by a faulty test, or bug in the system under test. * OF-2611: Speed up test execution by reducing SO_TIMEOUT By reducing the socket timeout, the S2S unit test execute a lot faster. There's likely a balance between a low timeout value, and introducing timing-related issues. This value might require some tweaking. * test : fix null pointer exception for missing certificate state * OF-2611: Refactor Incoming/Outgoing S2S unit tests for performance The Incoming- and OutgoingServerSessionTest implementations depend heavily on 'mock' server implementations. During the tests, these mocks act as the peer/remote XMPP domain. The test implementation is based on establishing TCP socket connections. As there are many tests that are being executed, the socket timeouts should be kept low. This improves the test execution time. This commit refactors how the dummy implementation works with socket timeouts. Notably: - improve explicit shutdown of sockets/executors to improve throughput - temporarily bump up the allowed timeouts when Server Dialback is used. Server Dialback depends on a second socket, during which interaction on the first socket is paused. * OF-2611: Reduce socket timeout for S2S unit tests By reducing the socket timeout, test execution duration improves. * OF-2611: Optimize S2S unit test for CPU usage Generating certificates is expensive. For performance, it's best to generate each set once, and then reuse those during the execution of the tests. This removes about 70% of the CPU usage during test execution. Locally, the duration of test execution dropped to about 60% of the original duration. * Github CI flow: expose junit reports * OF-2611: Tweak S2S unit test output As these unit tests are parameterized, it's not always straightforward what configuration was used in a failed test. This commit prints the configuration to std-out to make that more clear. * OF-2611: Refactor unit test helper method ServerSettings' constructor arguments should match the toString output for convenience. * OF-2626: Fix Server Dialback race condition Openfire should not report Server Dialback results back to the remote server, before the results have been locally stored. This prevents a race condition in which a remote server starts sending data, before the local server is aware that the remote has finished authentication. * OF-2611: Adjust S2S Unit tests for OF-2626 (Dialback race) * Fix references to RFC6120 * OF-2611: Ensure that strict-certificate setting always prevents dialback Depending on the exception that causes TLS to fail, dialback could still happen. With this change, Dialback won't happen if TLS failed. * OF-2611: Prevent NPEs when running tests that involves having no certs * OF-2611: Improved logging of S2S unit tests * OF-2611: More explicitly link 'strict cert verification' to cert status Previously, 'strict verification' would be applied to any TLS failure. It should be applied to certificate validation failures only. * OF-2611: Improve S2S unit test When the mock server doesn't have PKIX material, that shouldn't be reason to tell the peer that TLS (will) fail. Instead, the purpose of the test is to _see_ this fail. Thus, with this change, the peer is motivated to try (and fail). * OF-2611: S2S Outgoing Server Session unit test, add exemption In a very specific configuration of settings, a connection attempt must fail. However, the system under test can be expected to retry the connection immediately, with another configuration that's permissable under the unit test settings. * OF-2611: Remove unused defintion. * OF-2611: Introduce flag to disable logging to std-out * OF-2611: S2S unit test should print configuration Not all test-runners easily identify the parameters that are used to run each test iteration. Those that do not, typically show a number. By outputting the numbered arguments, they can be cross-referenced with any failed test case. * OF-2611: Add context to StreamError when no message is provided. * OF-2611: When Dialback fails, close the connection * OF-2611: Add copyright header * OF-2611: Modified copyright header This code was ported from a short-lived project in my personal repositories, hence the copyright definition. * OF-2611: Generically add references to specifications for ExpectedOutcome * OF-2611: S2S Unit test: clean up TODOs, add spec references This adds references to RFCs in the ExpectedOutcome calculation, and removes some of the TODO statements in that class. Most of the TODOs are 'resolved' by adding a 'strictCertificateValidation' setting in the calculation. This is used to choose between the multiple possible outcomes that were in the 'TODO'. Note taht the strictCertificateValidation setting is hardcoded in all tests. The current implementation makes it hard to configure different values for the initiating and receiving entities. Also, they would add to an already long list of tests. * OF-2611: Clean up test teardown * OF-2611: Additional null-check * Revert "Workaround for OF-2592" This reverts commit 0445be6. * OF-2611: Additional additional null-check --------- Co-authored-by: Alex Gidman <alex.gidman@surevine.com> Co-authored-by: Matthew Vivian <matthew.vivian@surevine.com> Co-authored-by: Dan Caseley <dan@caseley.me.uk>
- Loading branch information
4 people
committed
Jul 25, 2023
1 parent
e5c4a11
commit e5623ee