-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Execute ansible in a sandbox container #656
base: develop
Are you sure you want to change the base?
Execute ansible in a sandbox container #656
Conversation
Merge from yorc upstream to otc 01.06.2020
* this commit makes sure ansible and scripts are executed from inside a sandbox container in the following steps: * We first generate all configuration files for the execution in the ansible RecipePath on the host machine (i.e., ansible.cfg, hosts, run.ansible.yml, wrapper, .vault_pass). Then start the sandbox container and bind mount the ansibleRecipePath in the sandbox (at /home/ansible). * bind mount the ssh agent socket on the host machine to the sandbox (at /home/ssh-agent), and pass the path to the socket (i.e., SSH_AUTH_SOCK) as an environment variable to the sandbox container. * bind mount the overlay, which contains deployment artifacts in the sandbox (at /home/overlay). * after starting the sandbox, reuse the CMD implementation to execute ansible inside the sandbox container. Ansible then writes output logs back to the file system (in /home/ansible/*-out.csv) so that the output handler of the CMD can read the logs. * add container configs for security hardening the sandbox (e.g., specify a non-root user to run the container, do not allow new privileges escalation, remove ALL capabilities, add yorc config to limit cpu to 0.5 and memory to 256m to avoid DoS). * if OpenSSH is enabled, specify the location where ansible can write ControlPath in the container (at /home/ansible/cp) to avoid ansible to have a write permission denied. * add config to specify the location where ansible can write factCaches in the container (at /home/ansible/facts_cache). Each task should have its own facts_cache so that one task cannot overwrite caches from the other parallel ones unexpectedly.
@@ -65,10 +66,24 @@ type executionAnsible struct { | |||
isAlienAnsible bool | |||
} | |||
|
|||
func (e *executionAnsible) runAnsible(ctx context.Context, retry bool, currentInstance, ansibleRecipePath string) error { | |||
func (e *executionAnsible) generateRunAnsible(ctx context.Context, currentInstance, ansibleRecipePath string) (outputHandler, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method executionAnsible.generateRunAnsible
has 141 lines of code (exceeds 50 allowed). Consider refactoring.
var nanoCPUs opts.NanoCPUs | ||
var memoryInBytes opts.MemBytes | ||
|
||
func createSandbox(ctx context.Context, cli *client.Client, sandboxCfg *config.DockerSandbox, deploymentID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function createSandbox
has 93 lines of code (exceeds 50 allowed). Consider refactoring.
@@ -207,27 +208,32 @@ func getExecutionScriptTemplateFnMap(e *executionCommon, ansibleRecipePath strin | |||
} | |||
} | |||
|
|||
func (e *executionScript) runAnsible(ctx context.Context, retry bool, currentInstance, ansibleRecipePath string) error { | |||
func (e *executionScript) generateRunAnsible(ctx context.Context, currentInstance, ansibleRecipePath string) (outputHandler, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method executionScript.generateRunAnsible
has 64 lines of code (exceeds 50 allowed). Consider refactoring.
@@ -65,10 +66,24 @@ type executionAnsible struct { | |||
isAlienAnsible bool | |||
} | |||
|
|||
func (e *executionAnsible) runAnsible(ctx context.Context, retry bool, currentInstance, ansibleRecipePath string) error { | |||
func (e *executionAnsible) generateRunAnsible(ctx context.Context, currentInstance, ansibleRecipePath string) (outputHandler, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method executionAnsible.generateRunAnsible
has a Cognitive Complexity of 43 (exceeds 20 allowed). Consider refactoring.
|
||
// getNanoCPUs converts user defined cpus string to nano cpus presentation | ||
// defaults cpu to 0.5 cpu if not set | ||
func getNanoCPUs(c string) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
* Add bind-address for ssh-agent so that ssh-agent stores the agent sockets in the work directoy (work/ssh-agent) instead of the default /tmp folder. This allows the sandbox container to bind mount volume from inside the work dir but not the /tmp folder. * Add home environment for the sandbox container so that ansible resolves its default path "~/.ansible" to the workdir inside the sandbox correctly. Otherwise ansible resolves to /home/<yorc> on the host machine, where yorc is running.
var nanoCPUs opts.NanoCPUs | ||
var memoryInBytes opts.MemBytes | ||
|
||
func createSandbox(ctx context.Context, cli *client.Client, sandboxCfg *config.DockerSandbox, deploymentID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function createSandbox
has 96 lines of code (exceeds 50 allowed). Consider refactoring.
var nanoCPUs opts.NanoCPUs | ||
var memoryInBytes opts.MemBytes | ||
|
||
func createSandbox(ctx context.Context, cli *client.Client, sandboxCfg *config.DockerSandbox, deploymentID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function createSandbox
has a Cognitive Complexity of 21 (exceeds 20 allowed). Consider refactoring.
|
||
// getMemoryInBytes converts user defined memory string to memory in bytes presentation | ||
// defaults memory to 256m if not set | ||
func getMemoryInBytes(m string) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
* add new method signature NewSSHAgentWithSocket() * change ansible version in Dockerfile from 2.9.9 to 2.7.9 * remove package scoped variable in getNanoCPUs(), getMemoryInBytes()
* If an execution script is not a hosted operation, we can execute it directly on the host machine, where yorc is running. * add checkSandboxExecution() to check if the given execution requires sandboxing. Pull request ystiaGH-656
@@ -65,10 +66,24 @@ type executionAnsible struct { | |||
isAlienAnsible bool | |||
} | |||
|
|||
func (e *executionAnsible) runAnsible(ctx context.Context, retry bool, currentInstance, ansibleRecipePath string) error { | |||
func (e *executionAnsible) generateRunAnsible(ctx context.Context, currentInstance, ansibleRecipePath string) (outputHandler, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method executionAnsible.generateRunAnsible
has 138 lines of code (exceeds 50 allowed). Consider refactoring.
@@ -65,10 +66,24 @@ type executionAnsible struct { | |||
isAlienAnsible bool | |||
} | |||
|
|||
func (e *executionAnsible) runAnsible(ctx context.Context, retry bool, currentInstance, ansibleRecipePath string) error { | |||
func (e *executionAnsible) generateRunAnsible(ctx context.Context, currentInstance, ansibleRecipePath string) (outputHandler, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method executionAnsible.generateRunAnsible
has a Cognitive Complexity of 42 (exceeds 20 allowed). Consider refactoring.
Code Climate has analyzed commit f0aa1f8 and detected 11 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Pull Request description
Description of the change
In the current implementation, yorc executes an ansible-playbook on a target host with the command-line parameter
connection=ssh
from the host machine. However, because any playbook keyword will override any command-line option and any configuration setting [1], attackers can set the playbook keywordconnection: local
in the ansible plays explicitly. As a result, they can execute an ansible play on the host machine, where yorc is running (instead of the target host).In this PR we start a sandbox container first and execute the ansible from inside the container. The following changes keep the current implementation. It means we can choose, whether to execute ansible in a sandbox, or on the host machine for backward compatibility. If we provide the configuration for the sandbox image (in config.yorc.yaml), then ansible is executed in the sandbox. Otherwise, it is executed on the host machine.
The TOSCA's hosted operation is executed in the sandbox container the same way as the TOSCA's operation but with the command-line parameter set to
connection=local
.What I did
The ansible execution have 3 steps:
Step 1: generate all ansible configuration files in
deployments/<deployment-id>/ansible
.Step 2: start the container and bind mount the ansible configuration files and the overlay.
Step 3: execute
ansible-playbook
from inside the container.How I did it
Step 1:
In this step, all ansible configuration files for the execution (i.e.,
ansible.cfg
,hosts
,run.ansible.yml
,wrapper
,.vault_pass
) are generated in thedeployments/<deployment-id>/ansible
on the host machine as usual. The generation forhosts
,wrapper
, and.vault_pass
remains unchanged.For the generation of
ansible.cfg
, we added the following changes:By default, ansible writes its local temp files and the SSH ControlPath sockets (if OpenSSH is enabled) to
~/.ansible/tmp
and~/.ansible/cp
on the host machine, respectively. This may cause a permission denied error when ansible runs inside the container and has no write permission to the given paths. Therefore, we added two configs in the functiongenerateAnsibleConfigurationFile()
, which specify the location where ansible can write these files in the default workdir of the sandbox (i.e.,/work/ansible/tmp
and/work/ansible/cp
).Also, in the current implementation, the config
fact_caching_connection
specifies ansible to write the gathering fact caches of all yorc tasks at a common pathdeployments/<deployment-id>/facts_caches
. During our tests, we have noticed that each yorc task should have its own facts cache (i.e., different tasks should not share a common path for gathering facts). Otherwise, one task may overwrite the gathering facts of the other ones unexpectedly. For example, if users setbecome: true
in one task, it will overwrite the USER fact from ubuntu to root in another task. Therefore, we do not setup the containers to share a common path on the host machine for facts cache. Insteads, we added a config to specify the location where ansible can write fact caches in the workdir of the container (by default at/work/ansible/facts_cache
).For the generation of
run.ansible.yml
, theexecution_ansible.go
andexecution_scripts.go
is updated so that the path to theoverlay
and theDestFolder
is a location from within the container.Step 2:
In this step, we start the sandbox container and bind mount the
deployments/<deployment-id>/ansible
to the workdir of the container (/work/ansible
). Also, we bind mount thedeployments/<deployment-id>/overlay
, which contains deployment artifacts, in the sandbox (by default at/work/overlay
).In order for the ansible execution to access the target host, we also bind mount the ssh agent socket, which holds the private keys to access the target hosts, from the host machine to the sandbox (by default at
/work/ssh-agent
), and passSSH_AUTH_SOCK
to the sandbox as an environment variable of the starting container.In the current implementation, the sandbox container still miss the security hardening. Therefore, we added some configs for hardening the security of the container. For instance, we:
Step 3:
After the sandbox is started, we reuse the current implementation of CMD to execute ansible inside the sandbox container (i.e., docker exec ). Ansible then writes output logs back to the file system (by default at
/work/ansible/*-out.csv
) and the output handler of the CMD can read the logs. The implementation of the output handler for ansible and script remain unchanged.How to verify it
Step 1: Build the sandbox image
Go to
pkg/ansible
and build:Step 2: update config.yorc.yaml
Step 3: Start yorc.
Step 4: Tests
We already tested a topology with your provided python software component in [2] and the playbook mongodb in [3].
Description for the changelog
We will update the changelog and yorc documentation after you review the code changes and agree to it.
Applicable Issues
In the current implementation (i.e., before this PR), we have noticed that, when we enable OpenSSH in ansible, we got the following errors from SSH:
It means, SSH multiplexing does not work. For every command, it tries to reuse an opened SSH connection, but the master already closes it. We also tried to increase the ControlPersist from 60s to 10m but still got the same error. Even the ansible execution is succeed, but a new SSH connection is opened for every command reduces the performance significantly. Do you have the same observation?
References
[1] https://docs.ansible.com/ansible/latest/reference_appendices/general_precedence.html
[2] https://github.com/ystia/tosca-samples/tree/develop/org/ystia/yorc/samples/python
[3] https://github.com/ystia/forge/tree/develop/org/ystia/mongodb/linux/ansible