mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 58428: Added tests for failed executor authorization.
Date Wed, 19 Apr 2017 00:33:56 GMT

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


Fix it, then Ship it!




While these tests are good I'm wondering if they are realistic, because the assumption is
that someone knows the agent's secret key but doesn't know the container id of the executor
they want to attack. In reality it's the opposite; they know the container id of the executor
they want to attack but not the agent's key. Just a thought.


src/tests/slave_authorization_tests.cpp
Lines 28-30 (original), 30-34 (patched)
<https://reviews.apache.org/r/58428/#comment245396>

    order alphabetically.



src/tests/slave_authorization_tests.cpp
Line 32 (original), 36-37 (patched)
<https://reviews.apache.org/r/58428/#comment245397>

    order alphabetically.



src/tests/slave_authorization_tests.cpp
Lines 739 (patched)
<https://reviews.apache.org/r/58428/#comment245399>

    s/badRequest/error/



src/tests/slave_authorization_tests.cpp
Lines 1070 (patched)
<https://reviews.apache.org/r/58428/#comment245400>

    don't need terminate/wait slave here like the above test?


- Vinod Kone


On April 14, 2017, 9:18 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58428/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 9:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7339
>     https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds new tests to verify that HTTP executors cannot
> subscribe or launch nested containers when HTTP executor
> authentication is enabled, authorization is enabled, and they
> do not provide a valid executor authentication token
> 
> 
> Diffs
> -----
> 
>   src/tests/slave_authorization_tests.cpp 3657e0a3d19d75cef92e5bf90b65ef00c291b032 
> 
> 
> Diff: https://reviews.apache.org/r/58428/diff/3/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message