[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/534 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user vtslab commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102313638
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
---
@@ -92,6 +102,13 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) {
try {
authenticator.authenticate(credentials);
ctx.fireChannelRead(request);
+
+// User name logged with the remote socket address and
authenticator classname for audit logging
+if (authenticationSettings.enableAuditLog) {
+String[] authClassParts =
authenticator.getClass().toString().split("[.]");
+auditLogger.info("User {} with address {}
authenticated by {}", credentials.get(PROPERTY_USERNAME),
+
ctx.channel().remoteAddress().toString().substring(1),
authClassParts[authClassParts.length - 1]);
--- End diff --
Thanks for clarifying, I will correct it. Tests are still running.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/534#discussion_r102104550 --- Diff: docs/src/reference/gremlin-applications.asciidoc --- @@ -1035,6 +1035,7 @@ The following table describes the various YAML configuration options that Gremli |= |Key |Description |Default |authentication.className |The fully qualified classname of an `Authenticator` implementation to use. If this setting is not present, then authentication is effectively disabled. |`AllowAllAuthenticator` +|authentication.enableAuditLog |The available authenticators can issue audit logging messages, binding the authenticated user to his remote socket address and binding requests with a gremlin query to the remote socket address. For privacy reasons, the default value of this setting is false. The audit logging messages are logged at the INFO level via the `audit.org.apache.tinkerpop.gremlin.server` logger, which can be configured using the log4j.properties file. |false --- End diff -- What I meant is that this one was not in alphabetical order and perhaps it should be. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102104360
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
---
@@ -92,6 +102,13 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) {
try {
authenticator.authenticate(credentials);
ctx.fireChannelRead(request);
+
+// User name logged with the remote socket address and
authenticator classname for audit logging
+if (authenticationSettings.enableAuditLog) {
+String[] authClassParts =
authenticator.getClass().toString().split("[.]");
+auditLogger.info("User {} with address {}
authenticated by {}", credentials.get(PROPERTY_USERNAME),
+
ctx.channel().remoteAddress().toString().substring(1),
authClassParts[authClassParts.length - 1]);
--- End diff --
Let me elaborate. substring(1) assumes the toString() always starts with
'/'. However, if the hostname were resolved, then it would be in the format of
"hostname/IP address:port". substring(1) would result in "ostname/IP
address:port". It might not ever happen, but wanted to point it out for
awareness.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user vtslab commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/534#discussion_r102095814 --- Diff: docs/src/reference/gremlin-applications.asciidoc --- @@ -1035,6 +1035,7 @@ The following table describes the various YAML configuration options that Gremli |= |Key |Description |Default |authentication.className |The fully qualified classname of an `Authenticator` implementation to use. If this setting is not present, then authentication is effectively disabled. |`AllowAllAuthenticator` +|authentication.enableAuditLog |The available authenticators can issue audit logging messages, binding the authenticated user to his remote socket address and binding requests with a gremlin query to the remote socket address. For privacy reasons, the default value of this setting is false. The audit logging messages are logged at the INFO level via the `audit.org.apache.tinkerpop.gremlin.server` logger, which can be configured using the log4j.properties file. |false --- End diff -- Answer to this riddle (needed some thought on my part): also other config items, like scriptEngines..config, are at the bottom of their section. Just because it looks more orderly in the config file. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user vtslab commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102094977
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
---
@@ -92,6 +102,13 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) {
try {
authenticator.authenticate(credentials);
ctx.fireChannelRead(request);
+
+// User name logged with the remote socket address and
authenticator classname for audit logging
+if (authenticationSettings.enableAuditLog) {
+String[] authClassParts =
authenticator.getClass().toString().split("[.]");
+auditLogger.info("User {} with address {}
authenticated by {}", credentials.get(PROPERTY_USERNAME),
+
ctx.channel().remoteAddress().toString().substring(1),
authClassParts[authClassParts.length - 1]);
--- End diff --
It looks suspect, indeed, thanks for the remark. remoteAddres is a
SocketAddress though, so it would not fail on address resolution. To keep
remoteAddress use consistent with gremlin-driver and be on the conservative
side, I do not mind putting the string operation sequence in a try{} block
(also for the other occurrences below).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102062737
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpBasicAuthenticationHandler.java
---
@@ -92,6 +102,13 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) {
try {
authenticator.authenticate(credentials);
ctx.fireChannelRead(request);
+
+// User name logged with the remote socket address and
authenticator classname for audit logging
+if (authenticationSettings.enableAuditLog) {
+String[] authClassParts =
authenticator.getClass().toString().split("[.]");
+auditLogger.info("User {} with address {}
authenticated by {}", credentials.get(PROPERTY_USERNAME),
+
ctx.channel().remoteAddress().toString().substring(1),
authClassParts[authClassParts.length - 1]);
--- End diff --
substring(1) assumes that the remoteAddress always has an unresolved
(reverse lookup) hostname. I don't know if this is always the case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102062848
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/SaslAuthenticationHandler.java
---
@@ -94,13 +99,17 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) throw
ctx.writeAndFlush(error);
return;
}
-
+
try {
final byte[] saslMessage =
negotiator.get().evaluateResponse(saslResponse);
if (negotiator.get().isComplete()) {
-// todo: do something with this user
final AuthenticatedUser user =
negotiator.get().getAuthenticatedUser();
-
+// User name logged with the remote socket
address and authenticator classname for audit logging
+if (authenticationSettings.enableAuditLog) {
+String[] authClassParts =
authenticator.getClass().toString().split("[.]");
+auditLogger.info("User {} with address {}
authenticated by {}", user.getName(),
+
ctx.channel().remoteAddress().toString().substring(1),
authClassParts[authClassParts.length - 1]);
--- End diff --
substring(1) again
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/534#discussion_r102058441 --- Diff: docs/src/reference/gremlin-applications.asciidoc --- @@ -1035,6 +1035,7 @@ The following table describes the various YAML configuration options that Gremli |= |Key |Description |Default |authentication.className |The fully qualified classname of an `Authenticator` implementation to use. If this setting is not present, then authentication is effectively disabled. |`AllowAllAuthenticator` +|authentication.enableAuditLog |The available authenticators can issue audit logging messages, binding the authenticated user to his remote socket address and binding requests with a gremlin query to the remote socket address. For privacy reasons, the default value of this setting is false. The audit logging messages are logged at the INFO level via the `audit.org.apache.tinkerpop.gremlin.server` logger, which can be configured using the log4j.properties file. |false --- End diff -- Should this be in alphabetical order? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/534#discussion_r102063183 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java --- @@ -243,6 +245,10 @@ protected void evalOpInternal(final Context context, final Supplier
[GitHub] tinkerpop pull request #534: TINKERPOP-1566 Kerberos authentication for grem...
Github user robertdale commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r102062745
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
---
@@ -188,6 +189,10 @@ public void channelRead(final ChannelHandlerContext
ctx, final Object msg) {
try {
logger.debug("Processing request containing script [{}]
and bindings of [{}] on {}",
requestArguments.getValue0(),
requestArguments.getValue1(), Thread.currentThread().getName());
+if (settings.authentication.enableAuditLog) {
+final String address =
ctx.channel().remoteAddress().toString().substring(1);
--- End diff --
substring(1) again
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #534: Tinkerpop 1566 Kerberos
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/534#discussion_r96403984
--- Diff:
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/AbstractGryoMessageSerializerV1d0.java
---
@@ -301,9 +301,12 @@ private Object serializeResultToString(final
ResponseMessage msg) {
if (msg.getResult().getData() == null) return "null";
// the IteratorHandler should return a collection so keep it as
such
+// Sasl authentication needs byte[] to pass unchanged
final Object o = msg.getResult().getData();
if (o instanceof Collection) {
return ((Collection) o).stream().map(d -> null == d ? "null" :
d.toString()).collect(Collectors.toList());
+} else if (o instanceof byte[]) {
--- End diff --
I think you should undo this special handling - based on my explanation on
#533
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
