You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mc...@gmail.com> on 2015/08/05 12:00:50 UTC

Review Request 37125: Added 'Master::authorize' for Reserve/Unreserve.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37125/
-----------------------------------------------------------

Review request for mesos, Adam B and Jie Yu.


Bugs: MESOS-3062
    https://issues.apache.org/jira/browse/MESOS-3062


Repository: mesos


Description
-------

The `Master::authorize` function is overloaded for `Reserve` and `Unreserve`. This will be extended for `Create` and `Destroy` in the future. These are used for authorization of frameworks as well as master endpoints.


Diffs
-----

  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 

Diff: https://reviews.apache.org/r/37125/diff/


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 37125: Added 'Master::authorize' for Reserve/Unreserve.

Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37125/#review95020
-----------------------------------------------------------

Ship it!


LGTM modulo my (blocking) concern about allowing a malicious user to crash master on a malformed request.


src/master/master.cpp (line 2395)
<https://reviews.apache.org/r/37125/#comment149783>

    are you sure about this? this will cause master to abort on a malformed request - wouldn't it be best to take the 'safe route' and just deny authorization?
    
    security best practice is usually "just say no" when you don't understand :)
    (and don't provide even any feedback other than "Not Authorized" to a potential attacker).


- Marco Massenzio


On Aug. 5, 2015, 10 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37125/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 10 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-3062
>     https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `Master::authorize` function is overloaded for `Reserve` and `Unreserve`. This will be extended for `Create` and `Destroy` in the future. These are used for authorization of frameworks as well as master endpoints.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
> 
> Diff: https://reviews.apache.org/r/37125/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 37125: Added 'Master::authorize' for Reserve/Unreserve.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37125/#review98340
-----------------------------------------------------------


In the description of this patch, you mentioned that "These are used for authorization of frameworks as well as master endpoints.", not clear what does this mean, does it mean that the API in this patch can be also used to authorize framework? Can you please show more comments here? Thanks.


src/master/master.cpp (line 2354)
<https://reviews.apache.org/r/37125/#comment154753>

    What about s/authorize/authorizeReserveResource



src/master/master.cpp (line 2378)
<https://reviews.apache.org/r/37125/#comment154754>

    What about s/authorize/authorizeUnReserveResource


- Guangya Liu


On Aug. 5, 2015, 10 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37125/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 10 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-3062
>     https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `Master::authorize` function is overloaded for `Reserve` and `Unreserve`. This will be extended for `Create` and `Destroy` in the future. These are used for authorization of frameworks as well as master endpoints.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
> 
> Diff: https://reviews.apache.org/r/37125/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>