IMPALA-11382: Produce log for unauthorized SELECT on non-existing table

This patch revised the logic of Ranger audit log generation such that
unauthorized SELECT operation on non-existing tables would be produced
as well. Note that after this change, in the case of an unauthorized
SELECT operation on an existing table, Impala will produce a table event
instead of the first failing column event because we do not filter out
the table event for an unauthorized SELECT operation like what we did
before.

In addition, this patch fixed a subtle bug where an authorized table
event could be produced even though the authorization failed due to a
deny policy on a column in the same table.

The code comment in RangerAuthorizationChecker#authorizeTableAccess()
was also updated to reflect Impala's current behavior with respect to
Ranger audit log generation.

Testing:
 - Added a test case to verify the log corresponding to an unauthorized
   SELECT operation on a non-existing table is produced.
 - Manually verified that an authorized table event won't be produced
   when the requesting user is granted the SELECT privilege on a table
   but is denied access to a column in the same table.

Change-Id: I92b2a6acc920de1d2d14b991c374a4550e742f7b
Reviewed-on: http://gerrit.cloudera.org:8080/18656
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>

Author: fangyurao

fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java

@@ -255,34 +255,49 @@ protected void authorizeTableAccess(AuthorizationContext authzCtx,
       throws AuthorizationException, InternalException {
     RangerAuthorizationContext originalCtx = (RangerAuthorizationContext) authzCtx;
     RangerBufferAuditHandler originalAuditHandler = originalCtx.getAuditHandler();
-    // case 1: table (select) OK --> add the table event
+    // case 1: table (select) OK, columns (select) OK --> add the table event,
+    //                                                    add the column events
     // case 2: table (non-select) ERROR --> add the table event
     // case 3: table (select) ERROR, columns (select) OK -> only add the column events
-    // case 4: table (select) ERROR, columns (select) ERROR --> only add the first column
-    //                                                          event
+    // case 4: table (select) ERROR, columns (select) ERROR --> add the table event
+    // case 5: table (select) ERROR --> add the table event
+    //         This could happen when the select request for a non-existing table fails
+    //         the authorization.
+    // case 6: table (select) OK, columns (select) ERROR --> add the first column event
+    //         This could happen when the requesting user is granted the select privilege
+    //         on the table but is denied access to a column in the same table in Ranger.
     RangerAuthorizationContext tmpCtx = new RangerAuthorizationContext(
         originalCtx.getSessionState(), originalCtx.getTimeline());
     tmpCtx.setAuditHandler(new RangerBufferAuditHandler(originalAuditHandler));
+    AuthorizationException authorizationException = null;
     try {
       super.authorizeTableAccess(tmpCtx, analysisResult, catalog, requests);
     } catch (AuthorizationException e) {
+      authorizationException = e;
       tmpCtx.getAuditHandler().getAuthzEvents().stream()
           .filter(evt ->
               // case 2: get the first failing non-select table
               (!"select".equalsIgnoreCase(evt.getAccessType()) &&
                   "@table".equals(evt.getResourceType())) ||
-              // case 4: get the first failing column
-              ("@column".equals(evt.getResourceType()) && evt.getAccessResult() == 0))
+              // case 4 & 5 & 6: get the table or a column event
+              (("@table".equals(evt.getResourceType()) ||
+                  "@column".equals(evt.getResourceType())) &&
+                  evt.getAccessResult() == 0))
           .findFirst()
           .ifPresent(evt -> originalCtx.getAuditHandler().getAuthzEvents().add(evt));
       throw e;
     } finally {
-      // case 1 & 4: we only add the successful events. The first table-level access
-      // check is only for the short-circuit, we don't want to add an event for that.
-      List<AuthzAuditEvent> events = tmpCtx.getAuditHandler().getAuthzEvents().stream()
-          .filter(evt -> evt.getAccessResult() != 0)
-          .collect(Collectors.toList());
-      originalCtx.getAuditHandler().getAuthzEvents().addAll(events);
+      // We should not add successful events when there was an AuthorizationException.
+      // Specifically, we should not add the successful table event in case 6.
+      if (authorizationException == null) {
+        // case 1 & 3: we only add the successful events.
+        // TODO(IMPALA-11381): Consider whether we should keep the table-level event which
+        // corresponds to a check only for short-circuiting authorization.
+        List<AuthzAuditEvent> events = tmpCtx.getAuditHandler().getAuthzEvents().stream()
+            .filter(evt -> evt.getAccessResult() != 0)
+            .collect(Collectors.toList());
+        originalCtx.getAuditHandler().getAuthzEvents().addAll(events);
+      }
     }
   }
 

fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java

@@ -317,14 +317,22 @@ public void testAuditLogFailure() throws ImpalaException {
         onDatabase("functional", TPrivilegeLevel.CREATE));
 
     authzError(events -> {
-      // Only log the first column event. We do not log the table access used for
-      // short-circuiting.
+      // Only log the table event.
       assertEquals(1, events.size());
-      assertEventEquals("@column", "select", "functional/alltypes/id", 0, events.get(0));
+      assertEventEquals("@table", "select", "functional/alltypes", 0, events.get(0));
       assertEquals("select id, string_col from functional.alltypes",
           events.get(0).getRequestData());
     }, "select id, string_col from functional.alltypes");
 
+    authzError(events -> {
+      // Log the table event even when the table does not exist.
+      assertEquals(1, events.size());
+      assertEventEquals("@table", "select", "functional/non_existing_tbl", 0,
+          events.get(0));
+      assertEquals("select * from functional.non_existing_tbl",
+          events.get(0).getRequestData());
+    }, "select * from functional.non_existing_tbl");
+
     authzError(events -> {
       // Show the actual view_metadata audit log.
       assertEquals(1, events.size());
@@ -496,15 +504,14 @@ public void testAuditsForColumnMasking() throws ImpalaException {
       // When the requesting user is not granted the necessary privileges, the audit log
       // entries corresponding to column masking are not generated. Notice that in the
       // case when the requesting user is not granted the necessary privileges, only the
-      // event of the first column that failed the authorization will be logged, i.e.,
-      // 'id'. Refer to RangerAuthorizationChecker#authorizeTableAccess() for further
-      // details.
+      // event of the table that failed the authorization will be logged. Refer to
+      // RangerAuthorizationChecker#authorizeTableAccess() for further details.
       authzError(events -> {
         assertEquals(1, events.size());
         assertEquals("with iv as (select id, bool_col, string_col from " +
                 "functional.alltypestiny) select * from iv",
             events.get(0).getRequestData());
-        assertEventEquals("@column", "select", "functional/alltypestiny/id", 0,
+        assertEventEquals("@table", "select", "functional/alltypestiny", 0,
             events.get(0));
       },"with iv as (select id, bool_col, string_col from functional.alltypestiny) " +
           "select * from iv", onTable("functional", "alltypestiny"));
@@ -714,13 +721,13 @@ public void testAuditsForRowFiltering() throws ImpalaException {
           TPrivilegeLevel.SELECT));
 
       // When fails with not enough privileges, no audit logs for row filtering is
-      // generated. Only the event of the first column (i.e. id) that failed the
-      // authorization will be logged.
+      // generated. Only the event of the table that failed the authorization will be
+      // logged.
       authzError(events -> {
         assertEquals(1, events.size());
         assertEquals("select * from functional.alltypestiny",
             events.get(0).getRequestData());
-        assertEventEquals("@column", "select", "functional/alltypestiny/id", 0,
+        assertEventEquals("@table", "select", "functional/alltypestiny", 0,
             events.get(0));
       },"select * from functional.alltypestiny", onTable("functional", "alltypestiny"));