[MAPREDUCE-4317] Job view ACL checks are too permissive

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.0.3
• Fix Version/s: 1.2.0
• Component/s: mrv1
• Hadoop Flags: Reviewed
• Assignee: Karthik Kambatla
• Reporter: Harsh J
• Created: 05/Jun/12 13:51
• Updated: 03/Nov/14 18:06
• Resolved: 05/Jul/12 16:51

Description

The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:

private boolean isViewAllowed = true;

Note that its true.

Now, in the method that sets proper view-allowed rights, has:

if (user != null && job != null && jt.areACLsEnabled()) {
      final UserGroupInformation ugi =
        UserGroupInformation.createRemoteUser(user);
      try {
        ugi.doAs(new PrivilegedExceptionAction<Void>() {
          public Void run() throws IOException, ServletException {

            // checks job view permission
            jt.getACLsManager().checkAccess(job, ugi,
                Operation.VIEW_JOB_DETAILS);
            return null;
          }
        });
      } catch (AccessControlException e) {
        String errMsg = "User " + ugi.getShortUserName() +
            " failed to view " + jobid + "!<br><br>" + e.getMessage() +
            "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
        JSPUtil.setErrorAndForward(errMsg, request, response);
        myJob.setViewAccess(false);
      } catch (InterruptedException e) {
        String errMsg = " Interrupted while trying to access " + jobid +
        "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
        JSPUtil.setErrorAndForward(errMsg, request, response);
        myJob.setViewAccess(false);
      }
    }
    return myJob;

In the above snippet, you can notice that if user==null, which can happen if user is not http-authenticated (as its got via request.getRemoteUser()), can lead to the view being visible since the default is true and we didn't toggle the view to false for user == null case.

Ideally the default of the view job ACL must be false, or we need an else clause that sets the view rights to false in case of a failure to find the user ID.