Add nio http transport to security plugin by Tim-Brooks · Pull Request #32018 · elastic/elasticsearch

Tim-Brooks · 2018-07-13T00:02:51Z

This is related to #27260. It adds the SecurityNioHttpServerTransport
to the security plugin. It randomly uses the nio http transport in
security integration tests.

elasticmachine · 2018-07-13T00:02:53Z

Pinging @elastic/es-security

jaymode

I left a couple of minor thing but otherwise LGTM

jaymode · 2018-07-13T15:57:27Z

.../plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

-                ServerTransportFilter.extractClientCertificates(logger, threadContext, handler.engine(), nettyChannel);
+                SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(httpChannel);
+
+                ServerTransportFilter.extractClientCertificates(logger, threadContext, sslEngine, httpChannel);


can we move extractClientCertificates into SSLEngineUtils and just have a single method call here?

jaymode · 2018-07-13T15:58:08Z

...plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java

+
+import javax.net.ssl.SSLEngine;
+
+public class SSLEngineUtils {


add a private constructor so no one instantiates this class

jaymode · 2018-07-13T15:58:34Z

...y/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java

+import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException;
+import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException;
+
+public class SecurityHttpExceptionHandler implements BiConsumer<HttpChannel, Exception> {


make it final

jaymode · 2018-07-13T15:59:50Z

...lugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java

+
+import java.util.function.Predicate;
+
+public class NioIPFilter implements Predicate<NioSocketChannel> {


make it final

Tim-Brooks · 2018-07-13T17:59:44Z

@jaymode Can you confirm those were the changes you wanted in regard to extractClientCertificates?

jaymode · 2018-07-13T18:24:06Z

@tbrooks8 that was exactly what I was looking for 👍

tylersmalley · 2018-07-14T04:58:52Z

Looks like this broke the Kibana CI, which builds from source. I expect we will also see this with the release manager tonight as well.

https://kibana-ci.elastic.co/job/elastic+kibana+master+multijob-x-pack/563/console

21:47:43    │ info  ./gradlew :distribution:archives:tar:assemble
21:47:43    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:06    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:07    │ERROR  WARNING: An illegal reflective access operation has occurred
21:48:07    │ERROR  WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/var/lib/jenkins/.gradle/wrapper/dists/gradle-4.8.1-all/6fmj4nezasjg1b7kkmy10xgo2/gradle-4.8.1/lib/groovy-all-2.4.12.jar) to method java.lang.Object.finalize()
21:48:07    │ERROR  WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
21:48:07    │ERROR  WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
21:48:07    │ERROR  WARNING: All illegal access operations will be denied in a future release
21:48:11    │ERROR  warning: [options] bootstrap class path not set in conjunction with -source 8
21:48:12    │ERROR  1 warning
21:48:19    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:19    │ERROR  WARNING: An illegal reflective access operation has occurred
21:48:19    │ERROR  WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/var/lib/jenkins/.gradle/wrapper/dists/gradle-4.8.1-all/6fmj4nezasjg1b7kkmy10xgo2/gradle-4.8.1/lib/groovy-all-2.4.12.jar) to method java.lang.Object.finalize()
21:48:19    │ERROR  WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
21:48:19    │ERROR  WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
21:48:19    │ERROR  WARNING: All illegal access operations will be denied in a future release
21:48:51    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java uses or overrides a deprecated API.
21:48:51    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:29    │ERROR  Note: Some input files use or override a deprecated API.
21:49:29    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:29    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:49:29    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:49:49    │ERROR  Note: Some input files use or override a deprecated API.
21:49:49    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:50    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java uses unchecked or unsafe operations.
21:49:50    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:49:50    │ERROR  Note: Some input files use or override a deprecated API.
21:49:50    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:51    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java uses or overrides a deprecated API.
21:49:51    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:54    │ERROR  Note: Some input files use or override a deprecated API.
21:49:54    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:56    │ERROR  Note: Some input files use or override a deprecated API.
21:49:56    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:58    │ERROR  Note: Some input files use or override a deprecated API.
21:49:58    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:04    │ERROR  Note: Some input files use or override a deprecated API.
21:50:04    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:04    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:04    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:10    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java uses or overrides a deprecated API.
21:50:10    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:17    │ERROR  Note: Some input files use or override a deprecated API.
21:50:17    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:17    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:17    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:23    │ERROR  Note: Some input files use or override a deprecated API.
21:50:23    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:24    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java uses or overrides a deprecated API.
21:50:24    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:28    │ERROR  /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java:62: error: method sslConfiguration in class SSLService cannot be applied to given types;
21:50:28    │ERROR              this.sslConfiguration = sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY);
21:50:28    │ERROR                                                ^
21:50:28    │ERROR    required: Settings
21:50:28    │ERROR    found: Settings,Settings
21:50:28    │ERROR    reason: actual and formal argument lists differ in length
21:50:28    │ERROR  /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java:62: error: getHttpTransportSSLSettings(Settings) has private access in SSLService
21:50:28    │ERROR              this.sslConfiguration = sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY);
21:50:28    │ERROR                                                                            ^
21:50:28    │ERROR  Note: Some input files use or override a deprecated API.
21:50:28    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:28    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:28    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:28    │ERROR  2 errors
21:50:28    │ERROR  
21:50:28    │ERROR  FAILURE: Build failed with an exception.
21:50:28    │ERROR  
21:50:28    │ERROR  * What went wrong:
21:50:28    │ERROR  Execution failed for task ':x-pack:plugin:security:compileJava'.
21:50:28    │ERROR  > Compilation failed; see the compiler error output for details.
21:50:28    │ERROR  
21:50:28    │ERROR  * Try:
21:50:28    │ERROR  Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
21:50:28    │ERROR  
21:50:28    │ERROR  * Get more help at https://help.gradle.org
21:50:28    │ERROR  
21:50:28    │ERROR  BUILD FAILED in 2m 44s
21:50:29 Error: unable to build ES

The build was broken due to some issues with the merging of elastic#32018. A method that was public went private before the PR was merged. That did not cause a merge conflict (so the PR was merged successfully). But it did cause the build to fail.

The build was broken due to some issues with the merging of #32018. A method that was public went private before the PR was merged. That did not cause a merge conflict (so the PR was merged successfully). But it did cause the build to fail.

Tim-Brooks added 4 commits July 12, 2018 15:03

WIP

26ea13e

Extract exception handler

3f0fee0

WIP

b8b84d7

Enable tests

9d5887a

Tim-Brooks added >non-issue review v7.0.0 :Security/TLS SSL/TLS, Certificates labels Jul 13, 2018

Tim-Brooks requested review from jasontedor, jaymode, s1monw and tvernum July 13, 2018 00:02

Tim-Brooks added 2 commits July 12, 2018 18:43

Fix checkstyle

9977af6

Fix forbidden

a0fbd07

jaymode approved these changes Jul 13, 2018

View reviewed changes

Issue from review

63ae74f

Tim-Brooks merged commit 305bfea into elastic:master Jul 13, 2018

Tim-Brooks mentioned this pull request Jul 14, 2018

Fix compile issues introduced by merge #32058

Merged

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Tim-Brooks deleted the nio_http_security branch December 18, 2019 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nio http transport to security plugin#32018

Add nio http transport to security plugin#32018
Tim-Brooks merged 7 commits intoelastic:masterfrom
Tim-Brooks:nio_http_security

Tim-Brooks commented Jul 13, 2018

Uh oh!

elasticmachine commented Jul 13, 2018

Uh oh!

jaymode left a comment

Uh oh!

jaymode Jul 13, 2018

Uh oh!

jaymode Jul 13, 2018

Uh oh!

jaymode Jul 13, 2018

Uh oh!

jaymode Jul 13, 2018

Uh oh!

Tim-Brooks commented Jul 13, 2018

Uh oh!

jaymode commented Jul 13, 2018

Uh oh!

tylersmalley commented Jul 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants


		import javax.net.ssl.SSLEngine;

		public class SSLEngineUtils {


		import java.util.function.Predicate;

		public class NioIPFilter implements Predicate<NioSocketChannel> {

Conversation

Tim-Brooks commented Jul 13, 2018

Uh oh!

elasticmachine commented Jul 13, 2018

Uh oh!

jaymode left a comment

Choose a reason for hiding this comment

Uh oh!

jaymode Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

Tim-Brooks commented Jul 13, 2018

Uh oh!

jaymode commented Jul 13, 2018

Uh oh!

tylersmalley commented Jul 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants