hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinayakumar B (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-9395) HDFS operations vary widely in which failures they put in the audit log and which they leave out
Date Mon, 08 Feb 2016 05:59:39 GMT

    [ https://issues.apache.org/jira/browse/HDFS-9395?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15136617#comment-15136617

Vinayakumar B commented on HDFS-9395:

Below are the places need fix

*Audit Logging In Finally block*

*Only Success audit logs ( Successfull Authorization )*

There might be some cases, where ACE will be thrown for Superuser check (admin commands).
Logging failure audit log for this can be discussed in other Jira (May be HDFS-5040, since
it is intended for that).
Other places needs to be fixed.

In Addition, 
{{FSN#renameTo(..)}}, is mixing rename result with permission check to log at last. This will
conflict with above scheme. So, instead only consider permission check.

Based on above changes, Some audit logs ( for non-ACE failures ) will go missing.
So this change needs to be marked as Incompatible, for heads-up.

Patch should cover all above methods. I see some are not covered in current patch.

Current changes in the patch looks good. 
Following are some nits.
+    } catch (AccessControlException ace) {
+      success = false;
+      logAuditEvent(success, "deleteSnapshot", rootPath, null, null);
+      throw ace;
+    } catch (AccessControlException ace) {
+      success = false;
+      logAuditEvent(success, "modifyCacheDirective", idStr,
+          directive.toString(), null);
+      throw ace;
     } finally {
{{success=false}} is not required. Its already false.

{code}       effectiveDirective = FSNDNCacheOp.addCacheDirective(this, cacheManager,
           directive, flags, logRetryCache);
+    } catch (AccessControlException ace) {
+      logAuditEvent(success, "addCacheDirective", null,
+          null, null);
+      throw ace;{code}
In {{FSNamesystem#addCacheDirective(..)}}, above code will not return null, it will either
return non-null value, or throw exception.
So, similar to other methods, {{success = true;}} can be inside try-block itself. And other
null checks may not be required for {{effectiveDirective}}

3. How about making the one cluster for all tests in @BeforeClass to speedup test?

> HDFS operations vary widely in which failures they put in the audit log and which they
leave out
> ------------------------------------------------------------------------------------------------
>                 Key: HDFS-9395
>                 URL: https://issues.apache.org/jira/browse/HDFS-9395
>             Project: Hadoop HDFS
>          Issue Type: Bug
>            Reporter: Kihwal Lee
>            Assignee: Kuhu Shukla
>         Attachments: HDFS-9395.001.patch, HDFS-9395.002.patch, HDFS-9395.003.patch, HDFS-9395.004.patch
> So, the big question here is what should go in the audit log? All failures, or just "permission
denied" failures? Or, to put it a different way, if someone attempts to do something and it
fails because a file doesn't exist, is that worth an audit log entry?
> We are currently inconsistent on this point. For example, concat, getContentSummary,
addCacheDirective, and setErasureEncodingPolicy create an audit log entry for all failures,
but setOwner, delete, and setAclEntries attempt to only create an entry for AccessControlException-based
failures. There are a few operations, like allowSnapshot, disallowSnapshot, and startRollingUpgrade
that never create audit log failure entries at all. They simply log nothing for any failure,
and log success for a successful operation.
> So to summarize, different HDFS operations currently fall into 3 categories:
> 1. audit-log all failures
> 2. audit-log only AccessControlException failures
> 3. never audit-log failures
> Which category is right?  And how can we fix the inconsistency

This message was sent by Atlassian JIRA

View raw message