Skip to content
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

Add some verifications on deployment script #1

Open
renatomassaro opened this issue Mar 22, 2017 · 1 comment
Open

Add some verifications on deployment script #1

renatomassaro opened this issue Mar 22, 2017 · 1 comment
Assignees

Comments

@renatomassaro
Copy link
Member

For security reasons we have a proxy node where all deployment must go through. This reduces the attack surface, so even if our build servers get owned, the attacker still won't be able to access production servers, data or keys.

Part of this safety guarantees relies on the fact that the build server has limited access on the deployer server (which does have access to production servers). This limited access is based on a custom shell and a custom deploy script.

As it is right now, it's lacking proper command checks to make sure someone can't bypass the shell or deploy script. An attacker could use "deploy ; " to run any command on the deployer host. The saner approach here is to allow only azAZ09 characters, ensuring it won't be able to bypass the shell and run multiple commands on the remote host.

@renatomassaro
Copy link
Member Author

To further extend on this: any variable which the user can set and is blindly executed by the script is able to bypass the secure shell. As an example, take the command below:

ansible-playbook \
    "$DEPLOY_SOFTWARE".yml \
    -i environments/"$DEPLOY_ENV" \
    --extra-vars "deploy=1 branch=$DEPLOY_BRANCH version=$DEPLOY_VERSION" \
    -u deployer \
    --private-key ~/.ssh/deployer_production

A well-crafted $DEPLOY_BRANCH value could escape this command and run a different one. As such, the regex verifications above must apply to these variables too. The most dangerous character here is ";", which could be used to dynamically finish the current command and run a different one.

@renatomassaro renatomassaro self-assigned this Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant