Fixed a bug in master and agent handler authorization logic. This patch fixes a bug where endpoint handlers would not correctly handle the case in which authorization is enabled when authentication is disabled. In this case, the handlers would send a default-constructed `authorization::Subject` to the authorizer, leading to an empty-string principal being evaluated as the subject.
This patch updates the handlers to correctly send `NONE` as the subject in this case. Review: https://reviews.apache.org/r/57054/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/30cbe95c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/30cbe95c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/30cbe95c Branch: refs/heads/master Commit: 30cbe95ca3739d88fd2b6e969b30ff7481452f20 Parents: 91d0ce4 Author: Greg Mann <[email protected]> Authored: Mon Mar 6 12:39:13 2017 -0800 Committer: Vinod Kone <[email protected]> Committed: Mon Mar 6 12:39:13 2017 -0800 ---------------------------------------------------------------------- src/master/http.cpp | 50 +++++++++++++++++++------------- src/slave/http.cpp | 75 +++++++++++++++++++++++++++++------------------- src/slave/slave.cpp | 6 ++-- 3 files changed, 78 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/30cbe95c/src/master/http.cpp ---------------------------------------------------------------------- diff --git a/src/master/http.cpp b/src/master/http.cpp index 37b96c0..3bc4f0b 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -664,9 +664,10 @@ Future<Response> Master::Http::subscribe( Future<Owned<ObjectApprover>> tasksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -1314,9 +1315,10 @@ Future<Response> Master::Http::frameworks( Future<Owned<ObjectApprover>> executorsApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -1478,9 +1480,10 @@ Future<Response> Master::Http::getFrameworks( Future<Owned<ObjectApprover>> frameworksApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -1544,9 +1547,10 @@ Future<Response> Master::Http::getExecutors( Future<Owned<ObjectApprover>> frameworksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -1675,9 +1679,10 @@ Future<Response> Master::Http::getState( Future<Owned<ObjectApprover>> tasksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -2582,9 +2587,10 @@ Future<Response> Master::Http::state( Future<Owned<ObjectApprover>> flagsApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -3071,9 +3077,10 @@ Future<Response> Master::Http::stateSummary( Future<Owned<ObjectApprover>> frameworksApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -3280,9 +3287,10 @@ Future<vector<string>> Master::Http::_roles( Future<Owned<ObjectApprover>> rolesApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } rolesApprover = master->authorizer.get()->getObjectApprover( @@ -3681,9 +3689,10 @@ Future<Response> Master::Http::tasks( Future<Owned<ObjectApprover>> frameworksApprover; Future<Owned<ObjectApprover>> tasksApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( @@ -3795,9 +3804,10 @@ Future<Response> Master::Http::getTasks( Future<Owned<ObjectApprover>> frameworksApprover; Future<Owned<ObjectApprover>> tasksApprover; if (master->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = master->authorizer.get()->getObjectApprover( http://git-wip-us.apache.org/repos/asf/mesos/blob/30cbe95c/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 94731ec..c904d89 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -836,9 +836,10 @@ Future<Response> Slave::Http::getFlags( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -976,9 +977,10 @@ Future<Response> Slave::Http::setLoggingLevel( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -1172,9 +1174,10 @@ Future<Response> Slave::Http::state( Future<Owned<ObjectApprover>> flagsApprover; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = slave->authorizer.get()->getObjectApprover( @@ -1355,9 +1358,10 @@ Future<Response> Slave::Http::getFrameworks( Future<Owned<ObjectApprover>> frameworksApprover; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = slave->authorizer.get()->getObjectApprover( @@ -1421,9 +1425,10 @@ Future<Response> Slave::Http::getExecutors( Future<Owned<ObjectApprover>> frameworksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = slave->authorizer.get()->getObjectApprover( @@ -1526,9 +1531,10 @@ Future<Response> Slave::Http::getTasks( Future<Owned<ObjectApprover>> tasksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = slave->authorizer.get()->getObjectApprover( @@ -1707,9 +1713,10 @@ Future<Response> Slave::Http::getState( Future<Owned<ObjectApprover>> tasksApprover; Future<Owned<ObjectApprover>> executorsApprover; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } frameworksApprover = slave->authorizer.get()->getObjectApprover( @@ -1963,9 +1970,10 @@ Future<Response> Slave::Http::getContainers( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2005,9 +2013,10 @@ Future<Response> Slave::Http::_containers( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2223,9 +2232,10 @@ Future<Response> Slave::Http::launchNestedContainer( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2331,9 +2341,10 @@ Future<Response> Slave::Http::waitNestedContainer( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2408,9 +2419,10 @@ Future<Response> Slave::Http::killNestedContainer( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2549,9 +2561,10 @@ Future<Response> Slave::Http::attachContainerInput( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2635,9 +2648,10 @@ Future<Response> Slave::Http::launchNestedContainerSession( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( @@ -2870,9 +2884,10 @@ Future<Response> Slave::Http::attachContainerOutput( Future<Owned<ObjectApprover>> approver; if (slave->authorizer.isSome()) { - authorization::Subject subject; + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } approver = slave->authorizer.get()->getObjectApprover( http://git-wip-us.apache.org/repos/asf/mesos/blob/30cbe95c/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 775f43b..c8f9bf6 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -6181,10 +6181,10 @@ Future<bool> Slave::authorizeSandboxAccess( } // Set authorization subject. - authorization::Subject subject; - + Option<authorization::Subject> subject; if (principal.isSome()) { - subject.set_value(principal.get()); + subject = authorization::Subject(); + subject->set_value(principal.get()); } Future<Owned<ObjectApprover>> sandboxApprover =
