You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jojy Varghese <jo...@mesosphere.io> on 2015/11/13 09:12:36 UTC
Review Request 40284: Disabled SSL for launching mesos executor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/
-----------------------------------------------------------
Review request for mesos and Timothy Chen.
Repository: mesos
Description
-------
When SSL is enabled, the executor process will also expect SSL certs, keys etc.
which causes unnecessary dependencies. A simple case is when password is
required for the SSL key. In this case, the executor would launch and wait for
user on terminal to enter password.
Diffs
-----
src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
Diff: https://reviews.apache.org/r/40284/diff/
Testing
-------
Thanks,
Jojy Varghese
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106395
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40284]
All tests passed.
- Mesos ReviewBot
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
>
> Timothy Chen wrote:
> I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
> We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
>
> Jojy Varghese wrote:
> What we really want is to be able to pass in SSL keys, certs etc for the container from the command line. But that would be a new feature. By default I think we should disable SSL on the launched containers. Also this change is local to mesos containetizer.
>
> Timothy Chen wrote:
> However if you disable SSL for all tasks from mesos containerizer it's going to cause problems, as you remember there is a existing bug in Docker executor that it doesn't inherit SSL and users wasn't able to launch tasks with it enabled right?
>
> Jojy Varghese wrote:
> I dont have much details about the existing bug and not sure if the solution patch made it in. Logically, we should be passing container specific SSL parameters and not slave's SSL parameters to the container if container needs SSL based communication. Untill such feature is implemented, I think we should disable SSL for the container process.
We should distinguish between the communication that the tasks perform, and the communication between the containerizer and the agent.
If an operator enables SSL communication on an agent, they (currently) expect that all communication between it and other libprocess components (example executor) are encrypted.
To go and disable this is breaking that promise / assumption, and opening a security hole.
I agree that we might want to pass different parameters through something like executor / task Info to control the communication that tasks do with the rest of the world.
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
>
> Timothy Chen wrote:
> I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
> We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
What we really want is to be able to pass in SSL keys, certs etc for the container from the command line. But that would be a new feature. By default I think we should disable SSL on the launched containers. Also this change is local to mesos containetizer.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Timothy Chen <tn...@apache.org>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Timothy Chen <tn...@apache.org>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
>
> Timothy Chen wrote:
> I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
> We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
>
> Jojy Varghese wrote:
> What we really want is to be able to pass in SSL keys, certs etc for the container from the command line. But that would be a new feature. By default I think we should disable SSL on the launched containers. Also this change is local to mesos containetizer.
However if you disable SSL for all tasks from mesos containerizer it's going to cause problems, as you remember there is a existing bug in Docker executor that it doesn't inherit SSL and users wasn't able to launch tasks with it enabled right?
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
>
> Timothy Chen wrote:
> I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
> We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
>
> Jojy Varghese wrote:
> What we really want is to be able to pass in SSL keys, certs etc for the container from the command line. But that would be a new feature. By default I think we should disable SSL on the launched containers. Also this change is local to mesos containetizer.
>
> Timothy Chen wrote:
> However if you disable SSL for all tasks from mesos containerizer it's going to cause problems, as you remember there is a existing bug in Docker executor that it doesn't inherit SSL and users wasn't able to launch tasks with it enabled right?
I dont have much details about the existing bug and not sure if the solution patch made it in. Logically, we should be passing container specific SSL parameters and not slave's SSL parameters to the container if container needs SSL based communication. Untill such feature is implemented, I think we should disable SSL for the container process.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 13, 2015, 9:59 a.m., Joris Van Remoortere wrote:
> > Can we fix the underlying problem, as opposed to disabling SSL?
>
> Jojy Varghese wrote:
> Not sure if we should enforce SSL sockets in an executor unless being explicitly asked for (say using a flag)
>
> Timothy Chen wrote:
> I don't think hard coding this is the right way to fix this, and this is going to cause more problems as it affects all executors.
> We should investigate how to get SSL to work when the environment is present, and also have a better way to control all components either SSL enabled or not.
>
> Jojy Varghese wrote:
> What we really want is to be able to pass in SSL keys, certs etc for the container from the command line. But that would be a new feature. By default I think we should disable SSL on the launched containers. Also this change is local to mesos containetizer.
>
> Timothy Chen wrote:
> However if you disable SSL for all tasks from mesos containerizer it's going to cause problems, as you remember there is a existing bug in Docker executor that it doesn't inherit SSL and users wasn't able to launch tasks with it enabled right?
>
> Jojy Varghese wrote:
> I dont have much details about the existing bug and not sure if the solution patch made it in. Logically, we should be passing container specific SSL parameters and not slave's SSL parameters to the container if container needs SSL based communication. Untill such feature is implemented, I think we should disable SSL for the container process.
>
> Joris Van Remoortere wrote:
> We should distinguish between the communication that the tasks perform, and the communication between the containerizer and the agent.
> If an operator enables SSL communication on an agent, they (currently) expect that all communication between it and other libprocess components (example executor) are encrypted.
> To go and disable this is breaking that promise / assumption, and opening a security hole.
> I agree that we might want to pass different parameters through something like executor / task Info to control the communication that tasks do with the rest of the world.
Sure that makes sense. We enabling SSL for processes inside container with SSL keys of agent is also security hole. From containerizer perpective, we need to separate the SSL concerns of agent and container processes. For example, Docker uses SSL to pull images from registry. But the processes running inside a docker container is unaware of that. And thats exactly the case here. We need to decouple SSL inside the container with the SSL outside it (agent). I am envisoning a new feature (say, enable SSL for mesos containers) that will have tasks to pass in the SSL information inside the container. Untill then, I am not sure if we should allow inheritence of SSL environment inside containers from agent.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40284: Disabled SSL for launching mesos executor.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40284/#review106388
-----------------------------------------------------------
Can we fix the underlying problem, as opposed to disabling SSL?
- Joris Van Remoortere
On Nov. 13, 2015, 8:12 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40284/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2015, 8:12 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When SSL is enabled, the executor process will also expect SSL certs, keys etc.
> which causes unnecessary dependencies. A simple case is when password is
> required for the SSL key. In this case, the executor would launch and wait for
> user on terminal to enter password.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 08243b61c1c277da7609bc910323cc1e27ff5cd4
>
> Diff: https://reviews.apache.org/r/40284/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jojy Varghese
>
>