You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/06/17 23:15:33 UTC
[GitHub] [ozone] duongnguyen0 opened a new pull request, #3531: HDDS-6904. Cleanup customer related events in S3G logs
duongnguyen0 opened a new pull request, #3531:
URL: https://github.com/apache/ozone/pull/3531
## What changes were proposed in this pull request?
Customer related events, including validation errors, should be logged in Audi logs only. The goal is to avoid a bad client to over load the s3g log and keep s3g log purely for internal issues.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6904
## How was this patch tested?
Manual test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3531:
URL: https://github.com/apache/ozone/pull/3531
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongnguyen0 commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
duongnguyen0 commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r924013257
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AWSSignatureProcessor.java:
##########
@@ -67,7 +78,13 @@ public SignatureInfo parseSignature() throws OS3Exception {
SignatureInfo signatureInfo = null;
for (SignatureParser parser : signatureParsers) {
- signatureInfo = parser.parseSignature();
+ try {
+ signatureInfo = parser.parseSignature();
+ } catch (MalformedResourceException e) {
+ AuditMessage message = buildAuthFailureMessage(e);
+ AUDIT.logAuthFailure(message);
+ throw S3ErrorTable.newError(MALFORMED_HEADER, e.getResource());
+ }
if (signatureInfo != null) {
break;
}
Review Comment:
Thanks for looking at the logs. Actually, the audit log was there but the action is not properly named. I've just submit an update to fixed this.
The warning logs is another problem. It's from the fact that S3G parses and links S3 signature to Endpoints using dependency injection. i.e.
```
@RequestScoped
public class OzoneClientProducer {
@Produces
public S3Auth getSignature() {}
}
```
```
public abstract class EndpointBase implements Auditor {
@Inject
private OzoneClient client;
```
The resolution of `S3Auth` is done by HK2 (Jersey lightweight IoC framework). When HK2 tries to create an `S3Auth` instance and gets exceptions for invalid/malformed signatures, it always logs the exceptions as warnings before passing to Jersey to resolve the exceptions to 4xx responses. I found no way to override this behavior.
To avoid the warning logs for those user event exceptions, we can either refactor to avoid using DI for creating and injecting `S3Auth`, or just disable the those warning logs using log config (note that Jersey just doesn't use log4j but Java Util Logging, so this may be some complication). Think I should log this in another JIRA.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongnguyen0 commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
duongnguyen0 commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r904445419
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java:
##########
@@ -308,4 +308,14 @@ protected Map<String, String> getAuditParameters() {
}
return res;
}
+
+ protected void auditWriteFailure(AuditAction action, Throwable ex) {
+ AUDIT.logWriteFailure(
+ buildAuditMessageForFailure(action, getAuditParameters(), ex));
+ }
+
+ protected void auditReadFailure(AuditAction action, Exception ex) {
+ AUDIT.logReadFailure(
+ buildAuditMessageForFailure(action, getAuditParameters(), ex));
+ }
Review Comment:
Yeah, I intended to keep the scope of this PR for the error logging. But actually we can extend it a little bit to success audits to make the code consistent. Good catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongnguyen0 commented on pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
duongnguyen0 commented on PR #3531:
URL: https://github.com/apache/ozone/pull/3531#issuecomment-1181102580
> There is one complication when adding a separate auth failure audit log. S3 gateway only validates if the auth signature is well-formed. The actual check for credentials occurs at OM. We would need to peek into the exception to know if this is related to auth or a different issue if we want to log it as an AUTH exception entry in the audit log.
Thanks for the clarification. I introduced the new AUTH marker just because at the time of validating signature, we don't know if the operation is either `READ` or `WRITE` yet. These action/classification is determined manually on-by-one at the endpoint handlers, [e.g.](https://github.com/duongnguyen0/ozone/blob/fcf8aae1fc510e399ffde7013d6649712bc06424/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L250-L250).
Yet, as you mentioned, if we're to do that, need to do that consistently for the actual credentials check as well.
If we don't do that, we can probably infer READ/WRITE base on the web context (e.g. if method is GET/HEAD then READ). However, this is also prone for inconsistence.
Also, the logged action is another complication is the logged action, which is also [manually put at the endpoint handler](https://github.com/duongnguyen0/ozone/blob/fcf8aae1fc510e399ffde7013d6649712bc06424/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L246-L246). So, if the adit log is issued at in the handler, it appears as
```
INFO | S3GAudit | op=CREATE_BUCKET {bucket=myBucket} | Some failure inside the handler
```
But at the time of validating signature, the best we can do is:
```
INFO | S3GAudit | op=PUT /bucket/myBucket {bucket=myBucket} | Error parsing signature...
```
I guess it depends on how consistent we want to introduce in the AUDIT log. Another refactor would be needed to make audit logging a consistent mechanism in S3G.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r924098722
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AWSSignatureProcessor.java:
##########
@@ -67,7 +78,13 @@ public SignatureInfo parseSignature() throws OS3Exception {
SignatureInfo signatureInfo = null;
for (SignatureParser parser : signatureParsers) {
- signatureInfo = parser.parseSignature();
+ try {
+ signatureInfo = parser.parseSignature();
+ } catch (MalformedResourceException e) {
+ AuditMessage message = buildAuthFailureMessage(e);
+ AUDIT.logAuthFailure(message);
+ throw S3ErrorTable.newError(MALFORMED_HEADER, e.getResource());
+ }
if (signatureInfo != null) {
break;
}
Review Comment:
Thanks @duongnguyen0 for digging into the exception problem.
> I should log this in another JIRA.
:+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3531:
URL: https://github.com/apache/ozone/pull/3531#issuecomment-1180924024
There is one complication when adding a separate auth failure audit log. S3 gateway only validates if the auth signature is well-formed. The actual check for credentials occurs at OM. We would need to peek into the exception to know if this is related to auth or a different issue if we want to log it as an AUTH exception entry in the audit log.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3531:
URL: https://github.com/apache/ozone/pull/3531#issuecomment-1183885971
@adoroszlai do you want to give one last look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongnguyen0 commented on pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
duongnguyen0 commented on PR #3531:
URL: https://github.com/apache/ozone/pull/3531#issuecomment-1163830859
> Thanks @duongnguyen0 for working on this. The change looks good, but I have two questions on scope.
>
> I see one more exception stack trace (see below) related to invalid auth headers. Is [HDDS-6905](https://issues.apache.org/jira/browse/HDDS-6905) intended to address it, or should it be fixed here?
Thanks for the detailed review, @adoroszlai.
The following log line, as well as others in `AuthorizationV4HeaderParser` should be addressed in this CR. It's great that you pointed out. I'll be sending a new version for this.
```
s3g_1 | 2022-06-18 00:28:29,054 [qtp1305004711-24] ERROR signature.AuthorizationV4HeaderParser: AWS access id shouldn't be empty. credential:/20220618/us-west-1/s3/aws4_request
```
And the other looks like a mishandling of exception and that results in an unhandled exception logging, it should be a part of [HDDS-6905](https://issues.apache.org/jira/browse/HDDS-6905).
```
Jun 18, 2022 12:28:29 AM org.glassfish.jersey.internal.Errors logErrors
s3g_1 | WARNING: The following warnings have been detected: WARNING: Unknown HK2 failure detected:
s3g_1 | MultiException stack 1 of 1
s3g_1 | javax.ws.rs.WebApplicationException: The authorization header you provided is invalid.
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.wrapOS3Exception(OzoneClientProducer.java:140)
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.getSignature(OzoneClientProducer.java:101)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r901632745
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java:
##########
@@ -308,4 +308,14 @@ protected Map<String, String> getAuditParameters() {
}
return res;
}
+
+ protected void auditWriteFailure(AuditAction action, Throwable ex) {
+ AUDIT.logWriteFailure(
+ buildAuditMessageForFailure(action, getAuditParameters(), ex));
+ }
+
+ protected void auditReadFailure(AuditAction action, Exception ex) {
+ AUDIT.logReadFailure(
+ buildAuditMessageForFailure(action, getAuditParameters(), ex));
+ }
Review Comment:
Extracting these is a good idea. Did you intend to change only parts of the code which were touched anyway? Would you be converting other call places in a future task? Also, what about `audit(Read|Write)Success`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r920954407
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AWSSignatureProcessor.java:
##########
@@ -67,7 +78,13 @@ public SignatureInfo parseSignature() throws OS3Exception {
SignatureInfo signatureInfo = null;
for (SignatureParser parser : signatureParsers) {
- signatureInfo = parser.parseSignature();
+ try {
+ signatureInfo = parser.parseSignature();
+ } catch (MalformedResourceException e) {
+ AuditMessage message = buildAuthFailureMessage(e);
+ AUDIT.logAuthFailure(message);
+ throw S3ErrorTable.newError(MALFORMED_HEADER, e.getResource());
+ }
if (signatureInfo != null) {
break;
}
Review Comment:
I see this exception in the [log](https://github.com/apache/ozone/suites/7311716169/artifacts/295519364):
```
s3g_1 | Jul 11, 2022 10:46:07 PM org.glassfish.jersey.internal.Errors logErrors
s3g_1 | WARNING: The following warnings have been detected: WARNING: Unknown HK2 failure detected:
s3g_1 | MultiException stack 1 of 1
s3g_1 | javax.ws.rs.WebApplicationException: The authorization header you provided is invalid.
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.wrapOS3Exception(OzoneClientProducer.java:141)
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.getSignature(OzoneClientProducer.java:102)
...
s3g_1 | Caused by: org.apache.hadoop.ozone.s3.exception.OS3Exception
s3g_1 | at org.apache.hadoop.ozone.s3.exception.S3ErrorTable.newError(S3ErrorTable.java:139)
s3g_1 | at org.apache.hadoop.ozone.s3.exception.S3ErrorTable.newError(S3ErrorTable.java:126)
s3g_1 | at org.apache.hadoop.ozone.s3.signature.AWSSignatureProcessor.parseSignature(AWSSignatureProcessor.java:86)
```
instead of the `AUTH` failure in audit.
Triggered by:
https://github.com/apache/ozone/blob/843fac2fb646eecfc33103fdb16eaf77a66ca062/hadoop-ozone/dist/src/main/smoketest/s3/bucketlist.robot#L43-L46
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3531:
URL: https://github.com/apache/ozone/pull/3531#discussion_r920954407
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AWSSignatureProcessor.java:
##########
@@ -67,7 +78,13 @@ public SignatureInfo parseSignature() throws OS3Exception {
SignatureInfo signatureInfo = null;
for (SignatureParser parser : signatureParsers) {
- signatureInfo = parser.parseSignature();
+ try {
+ signatureInfo = parser.parseSignature();
+ } catch (MalformedResourceException e) {
+ AuditMessage message = buildAuthFailureMessage(e);
+ AUDIT.logAuthFailure(message);
+ throw S3ErrorTable.newError(MALFORMED_HEADER, e.getResource());
+ }
if (signatureInfo != null) {
break;
}
Review Comment:
I see this exception in the [log](https://github.com/apache/ozone/suites/7311716169/artifacts/295519364):
```
s3g_1 | Jul 11, 2022 10:46:07 PM org.glassfish.jersey.internal.Errors logErrors
s3g_1 | WARNING: The following warnings have been detected: WARNING: Unknown HK2 failure detected:
s3g_1 | MultiException stack 1 of 1
s3g_1 | javax.ws.rs.WebApplicationException: The authorization header you provided is invalid.
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.wrapOS3Exception(OzoneClientProducer.java:141)
s3g_1 | at org.apache.hadoop.ozone.s3.OzoneClientProducer.getSignature(OzoneClientProducer.java:102)
...
s3g_1 | Caused by: org.apache.hadoop.ozone.s3.exception.OS3Exception
s3g_1 | at org.apache.hadoop.ozone.s3.exception.S3ErrorTable.newError(S3ErrorTable.java:139)
s3g_1 | at org.apache.hadoop.ozone.s3.exception.S3ErrorTable.newError(S3ErrorTable.java:126)
s3g_1 | at org.apache.hadoop.ozone.s3.signature.AWSSignatureProcessor.parseSignature(AWSSignatureProcessor.java:86)
```
instead of the `AUTH` failure in audit.
Triggered by:
https://github.com/apache/ozone/blob/843fac2fb646eecfc33103fdb16eaf77a66ca062/hadoop-ozone/dist/src/main/smoketest/s3/bucketlist.robot#L43-L46
I'm not sure the exception belongs to HDDS-6905, since it is not an unhandled `OMException`, rather an S3-specific one thrown by S3 Gateway. But the main problem is that it is missing from audit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #3531: HDDS-6904. Cleanup customer related events in S3G logs
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3531:
URL: https://github.com/apache/ozone/pull/3531#issuecomment-1181366640
> > There is one complication when adding a separate auth failure audit log. S3 gateway only validates if the auth signature is well-formed. The actual check for credentials occurs at OM. We would need to peek into the exception to know if this is related to auth or a different issue if we want to log it as an AUTH exception entry in the audit log.
>
> Thanks for the clarification. I introduced the new AUTH marker just because at the time of validating signature, we don't know if the operation is either `READ` or `WRITE` yet. These action/classification is determined manually on-by-one at the endpoint handlers, [e.g.](https://github.com/duongnguyen0/ozone/blob/fcf8aae1fc510e399ffde7013d6649712bc06424/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L250-L250).
>
> Yet, as you mentioned, if we're to do that, need to do that consistently for the actual credentials check as well.
>
> If we don't do that, we can probably infer READ/WRITE base on the web context (e.g. if method is GET/HEAD then READ). However, this is also prone for inconsistence.
>
> Also, the logged action is another complication is the logged action, which is also [manually put at the endpoint handler](https://github.com/duongnguyen0/ozone/blob/fcf8aae1fc510e399ffde7013d6649712bc06424/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L246-L246). So, if the adit log is issued at in the handler, it appears as
>
> ```
> INFO | S3GAudit | op=CREATE_BUCKET {bucket=myBucket} | Some failure inside the handler
> ```
>
> But at the time of validating signature, the best we can do is:
>
> ```
> INFO | S3GAudit | op=PUT /bucket/myBucket {bucket=myBucket} | Error parsing signature...
> ```
>
> I guess it depends on how consistent we want to introduce in the AUDIT log. Another refactor would be needed to make audit logging a consistent mechanism in S3G.
I agree, I think it would be best to not expand the current PR to fix all the issue but to do incrementally and keep the parent Jira open and add tasks as identified. For now, the scope of the changes here look good and it is more important to get the customer initiated logging off the S3G logs first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org