You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-issues@hadoop.apache.org by "Harsh J (JIRA)" <ji...@apache.org> on 2012/06/05 15:53:23 UTC

[jira] [Created] (MAPREDUCE-4317) Job view ACL checks are too permissive

Harsh J created MAPREDUCE-4317:
----------------------------------

             Summary: Job view ACL checks are too permissive
                 Key: MAPREDUCE-4317
                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
             Project: Hadoop Map/Reduce
          Issue Type: Bug
          Components: mrv1
    Affects Versions: 1.0.3
            Reporter: Harsh J


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

{code}private boolean isViewAllowed = true;{code}

Note that its true.

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

{code}
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;
{code}

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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400988#comment-13400988 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

Alejandro, 

The user string is read from the HTTPRequest. Both when the user == null and user == "null", the request reads it as "null". Hence, the check for both. And, I agree the user named "null" is forcibly not allowed.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401040#comment-13401040 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

Makes sense. Didn't realize it. I shall modify the test to not include a userName and see if I can omit the check for user.equals("null"). Thanks for the help.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401032#comment-13401032 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

In the following code snippet from TestWebUIAuthorization, new URL() takes in userName. When the userName is null, the URL sees it as "null". In checkAccessAndGetJob, we read the username from this string and get "null" and not null.

The test fails when I check only for user == null.

{code}
  static int getHttpStatusCode(String urlstring, String userName,
      String method) throws IOException {
    LOG.info("Accessing " + urlstring + " as user " + userName);
    URL url = new URL(urlstring + "&user.name=" + userName);
    HttpURLConnection connection = (HttpURLConnection)url.openConnection();
    connection.setRequestMethod(method);
    if (method.equals("POST")) {
      String encodedData = "action=kill&user.name=" + userName;      
      connection.setRequestProperty("Content-Type",
                                    "application/x-www-form-urlencoded");
      connection.setRequestProperty("Content-Length",
                                    Integer.toString(encodedData.length()));
      connection.setDoOutput(true);

      OutputStream os = connection.getOutputStream();
      os.write(encodedData.getBytes());
    }
    connection.connect();

    return connection.getResponseCode();
  }

{code}
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401036#comment-13401036 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

well, IMO, that test seems wrong. if you want to have an unauthenticated user then don't include '&user.name=...'. 
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alejandro Abdelnur updated MAPREDUCE-4317:
------------------------------------------

       Resolution: Fixed
    Fix Version/s: 1.2.0
     Hadoop Flags: Reviewed
           Status: Resolved  (was: Patch Available)

Thanks Karthik. Committed to branch-1.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>             Fix For: 1.2.0
>
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293269#comment-13293269 ] 

Hadoop QA commented on MAPREDUCE-4317:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12531738/MR-4317.patch
  against trunk revision .

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 1 new or modified test files.

    -1 patch.  The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2453//console

This message is automatically generated.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402607#comment-13402607 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

Karthik,

Why 'job ==null' ?

{code}
+    if (!jt.areACLsEnabled() || job == null) {
+      return myJob;
+    }
{code}

If job == null then myJob is also null (or even the call may fail)

Shouldn't we check for job == null before trying to the myJob?


                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402641#comment-13402641 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

Alejandro, 

The API (Javadoc below) mentions that the job will be null, if there doesn't exist a job with that JobID. The old API also has the same functionality.

{code}
  /**
   * Validates if current user can view the job.
   * If user is not authorized to view the job, this method will modify the
   * response and forwards to an error page and returns Job with
   * viewJobAccess flag set to false.
   * @return JobWithViewAccessCheck object(contains JobInProgress object and
   *         viewJobAccess flag). Callers of this method will check the flag
   *         and decide if view should be allowed or not. Job will be null if
   *         the job with given jobid doesnot exist at the JobTracker.
   */
{code}
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment: MR-4317.patch

Uploading a patch that checks access of a job to user in steps:
1. ACLs enabled/disabled
2. Job exists or not
3. User is valid - not null
4. ACL check to see if the user is allowed to access

Testing:

Unit test - TestWebUIAuthorization
- added a test for user == null
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment: MR-4317.patch

Cleaned the patch up to remove unused imports. Ready.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401103#comment-13401103 ] 

Hadoop QA commented on MAPREDUCE-4317:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12533419/MR-4317.patch
  against trunk revision .

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 1 new or modified test files.

    -1 patch.  The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2510//console

This message is automatically generated.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400977#comment-13400977 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

The corresponding code in MR2 (majorly refactored) - hadoop-mapreduce-client/ AMWebServices.hasAccess(job, request) - is not as permissive. This issue doesn't pertain to MR2.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400898#comment-13400898 ] 

Hadoop QA commented on MAPREDUCE-4317:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12533376/MR-4317.patch
  against trunk revision .

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 1 new or modified test files.

    -1 patch.  The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2509//console

This message is automatically generated.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment:     (was: MR-4317.patch.v1)
    
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment: MR-4317.patch.v1

Added v1 patch for this - the changes are fairly small and straight-forward.

1. I didn't see any tests checking TaskGraphServlet.
2. Do we need to add a test to verify this behavior? If so, can someone please point me to similar existing tests.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch.v1
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401015#comment-13401015 ] 

Karthik Kambatla commented on MAPREDUCE-4317:
---------------------------------------------

Let me explain in more detail:

JSPUtil.checkAccessAndGetJob() takes HTTPServletRequest as input. HTTPServletRequest.getRemoteUser() returns null if the user is not authenticated. For this case, check for user == null should suffice.

However, I have noticed while testing (TestWebUIAuthorization.validateTaskGraphServletAccess() in the patch) that when we build the HTTPServletRequest with user == null, the corresponding url captures the user as "null". For such cases, where the client mistakenly captures the user as "null", we need to check user.equals("null") as well. 

Do you think we should change the way client builds the HTTPServletRequest?

Many thanks.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment: MR-4317.patch

Updated the patch as per Alejandro's suggestions:

1. Corrected test for unauthenticated user.
2. Removed the check for user.equals("null").

Sorry for not realizing this earlier.

Thanks.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Comment Edited] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400986#comment-13400986 ] 

Alejandro Abdelnur edited comment on MAPREDUCE-4317 at 6/25/12 10:36 PM:
-------------------------------------------------------------------------

Karthik, why the check *if (user == null || user.equals("null")) {*, checking for *user == null* should be enough, no? Else you a voiding the user named '*null*'
                
      was (Author: tucu00):
    Karthik, why the check *if (user == null || user.equals("null")) {*, checking for *user == null* should be enough, no? Else you a voiding the user name *null*
                  
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Assigned] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla reassigned MAPREDUCE-4317:
-------------------------------------------

    Assignee: Karthik Kambatla
    
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400986#comment-13400986 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

Karthik, why the check *if (user == null || user.equals("null")) {*, checking for *user == null* should be enough, no? Else you a voiding the user name *null*
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Attachment:     (was: MR-4317.patch)
    
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401023#comment-13401023 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

what do you mean by +the client mistakenly captures the user as "null"+ ?
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Karthik Kambatla (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Karthik Kambatla updated MAPREDUCE-4317:
----------------------------------------

    Status: Patch Available  (was: Open)
    
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13407258#comment-13407258 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

+1. Thanks Karthik for following this offline with me and digging that making the changes I was suggesting would require changes in JSPs and thanks for verifying that if a null job is given to the JobWithViewAccessCheck constructor things still work as expected.
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch, MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-4317) Job view ACL checks are too permissive

Posted by "Alejandro Abdelnur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13401004#comment-13401004 ] 

Alejandro Abdelnur commented on MAPREDUCE-4317:
-----------------------------------------------

but the check for user '*null*' was not there, why are you introducing it? if the user is undefined it will be null, when do you expect the user to be the string '*null*'. Checking for *user == null* should be enough, no?
                
> Job view ACL checks are too permissive
> --------------------------------------
>
>                 Key: MAPREDUCE-4317
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4317
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv1
>    Affects Versions: 1.0.3
>            Reporter: Harsh J
>            Assignee: Karthik Kambatla
>         Attachments: MR-4317.patch
>
>
> The class that does view-based checks, JSPUtil.JobWithViewAccessCheck, has the following internal member:
> {code}private boolean isViewAllowed = true;{code}
> Note that its true.
> Now, in the method that sets proper view-allowed rights, has:
> {code}
> 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;
> {code}
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira