Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEODE-4817: Add support for SSL to the experimental driver. #1621

Closed
wants to merge 78 commits into from

Conversation

PivotalSarge
Copy link
Contributor

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • [n/a] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

Copy link
Contributor

@jdeppe-pivotal jdeppe-pivotal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thought I'd take a look here since I've just been neck deep in TLS/SSL.

private final String SSL_PROTOCOLS = "any";
private final String SSL_CIPHERS = "any";
private static final String TEST_USERNAME = "cluster";
private static final String TEST_PASSWORD = TEST_USERNAME;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two fields appear unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I shall remove them.

Properties properties = new Properties();
properties.put(SSL_ENABLED_COMPONENTS, "server");
properties.put(ConfigurationProperties.SSL_PROTOCOLS, SSL_PROTOCOLS);
properties.put(ConfigurationProperties.SSL_CIPHERS, SSL_CIPHERS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe the code actually allows for setting specific ciphers and protocols. Perhaps I missed that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied that from an example. I shall remove that.

jinmeiliao and others added 24 commits March 16, 2018 08:09
…he-1.0.xsd

* add methods in ClusterConfigurationService to use POJOs to manipulate the cache xml content.
…gCommand

* updated the CacheConfig to hold only one JNDIBindingsType instead of a list
* make sure xml created by those POJOs can be used to start a server
* the service will keep a list of bind classes for clearer xml generation.
* the service will keep a reference to the marshaer/unmarsher for faster operation
If enabled, Regions and Transactions will serialize key and value before
updating the local cache. This prevents inconsistency between distributed
members caused by any failure to serialize the key or value.

This feature is disabled by default. To enable it, specify the system
property geode.earlyEntryEventSerialization=true.
Cleanup all test classes touched for this change.
        * Log level set to info when FilterProfile gets an exception while registering CQ
	* Before, when there is an exception while registering cq like while cache closing the cq's base region is null
	* There is an exception which is logged in debug level but execution continues and adds the cq to the cp map with base region set to null
	* This results in a NullPointerException while closing cq as methods are executed on null region
	* Now the operation to put the cq into the cq map is inside a if check for null cq base region.
jna >4.1 has some issues with process management on windows.  Revert
back to jna 4.1.0 until this is resolved.
Make DistributedRestoreSystemProperties delegate to RestoreSystemProperties
Any DUnit test that uses more than 4 VMs should always specify
the total VM count via the ClusterStartupRule constructor so
that "bundled" Rules such as DistributedRestoreSystemProperties
which has a before that needs to know about all the VMs can
be setup properly.
Cleanup some of these tests:
* change to use AssertJ
* change from using Thread sleeps to Awaitility
…tDistributedTest

* change category to DistributedTest
* change from using old and new JUnit Assert to AssertJ
…SystemProperties

Update TODO comment and minor cleanup.
… CacheConfig objects (apache#1627)

* GEODE-3875:  refactor create jndi binding
* GEODE-4384: refactor destroy jndi binding

* refactored xml manipulation using jaxb
* Eliminate the need for RegionElement
* rename the current ClusterConfigurationService to InternalClusterConfigurationService
* make a public interface and have the internal implementation implements it.
jinmeiliao and others added 22 commits March 21, 2018 06:18
fix issues with getRequest not properly returning null.

This reverts commit 7a3da49, which
originally reverted 6214a43.

This closes apache#1617
…ception occurs

* GEODE-4451: Changed sender startup to retry when a remote security exception occurs

* GEODE-4451: Prevented sender from being created when members aren't all current version

* GEODE-4451: Apply spotless

* GEODE-4451: Refactored test to use ConfigurationProperties
* GfshCommand class to return Cache instead of InternalCache
* Exposed authorize methods to delegate to underlying security service.

Signed-off-by: Jens Deppe <jdeppe@pivotal.io>
Change default the value of pulse-help-custom property in the default
gemfire.properties file to be a valid link.
Removing the property because Help URLs are now dynamic.
* LuceneServiceImpl changes to support AEQ addition to existing region (after index initialized)
* Testing with and without redundancy
…lures (apache#1664)

- Use Awaitility to poll for expected results.
…he#1660)

- Modify the class-and-method details to elide method
  byte codes.
- Update the sanctioned data serializables to no
  longer contain the actual method bytecodes.
* Added public GfshCommand abstract class
* InternalGfshCommand extends the public class
* CommandManager sets cache to GfshCommand

Signed-off-by: Jens Deppe <jdeppe@pivotal.io>
- Fixes formatting
- Removes extra @ (echo suppression)
* @param keyStorePath Path to the SSL key store.
* @return This driver factory.
*/
public DriverFactory setKeyStorePath(String keyStorePath) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really seems like there should just be a single function setSSLPaths(keyStorePath, trustStorePath), as it doesn't seem to be valid to just call one of these functions. That being said, that might not plug as nicely into the builder pattern that's used here, so...your call.

upthewaterspout and others added 4 commits March 26, 2018 11:28
The SSLTest is currently failing, it appears the server is not actually
using SSL for some reason or another. The SSL_ENABLED_COMPONENTS
settings doesn't seem to take...
In order to use a locator with SSL, the locator must trust itself.
Modifying the truststore and adding a test of shutting down a locator
with SSL.
The locator needs to trust itself. Fixing the truststore so that this
test can shutdown.
@PivotalSarge
Copy link
Contributor Author

Lots of weirdness in the git history. I'm closing, I'll clean that up, and then re-open a PR.

@PivotalSarge PivotalSarge deleted the feature/GEODE-4817 branch March 26, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet