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

[Guangya Liu]

---
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)


What about s/authorize/authorizeReserveResource



src/master/master.cpp (line 2378)


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
> 
>

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

[Marco Massenzio]

---
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)


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
> 
>

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

[Michael Park]

---
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