Final non-root changes to Python, Ruby and C# templates #91

ericstoekl · 2017-09-11T05:32:25Z

Dockerfile for python, ruby, and C# template should not use ROOT

Signed-off-by: Eric Stoekl ems5311@gmail.com

Description

Like #83, this change is to have template Dockerfiles establish the function in an environment not running as root.

Motivation and Context

I have raised an issue to propose this change (required)
Proposal: Alter templates to run as non-privileged user #81

How Has This Been Tested?

ad-hoc

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I've read the CONTRIBUTION guide
I have signed-off my commits with git commit -s
I have added tests to cover my changes.
All new and existing tests passed.

alexellis · 2017-09-12T14:08:35Z

This looks great @ems5311 - you are blazing a trail through these issues!

alexellis · 2017-09-13T17:25:39Z

template/csharp/Dockerfile

-WORKDIR /root/
-WORKDIR /root/src
+#WORKDIR /root/
+#WORKDIR /root/src


Can we remove the comments here?

no prob -- will be done this evening

alexellis · 2017-10-01T07:25:27Z

Derek add label: status/testing

alexellis · 2017-10-27T19:00:03Z

We should move the adduser/useradd up top.. @ericstoekl can you work on this or recruit some help from the community?

ericstoekl · 2017-10-27T19:22:43Z

Yeah, I can get some stuff done on this later tonight.

alexellis · 2017-10-27T19:48:02Z

The change for Node.js is already merged and released 👍

Lewiscowles1986 · 2017-10-27T20:01:13Z

all working xenial x64

Lewiscowles1986 · 2017-10-27T20:04:58Z

Btw is there a reason they should not use root?

At work I pass in UID and GID, that seems to stop effects of containers writing files as root or a user that doesn't exist. It's done once at container initial start

ericstoekl · 2017-10-29T20:55:45Z

@alexellis Just updated to move adduser to the top of the Dockerfiles. Also added python3. Is it ok to address the armhf and arm64 languages in a future pr?

@Lewiscowles1986, Thanks for testing! This change was proposed after Nic gave a presentation about the potential vulnerabilities of running the docker images as root.

nicholasjackson · 2017-11-04T13:05:32Z

template/python/Dockerfile

@@ -9,19 +11,26 @@ RUN apk --no-cache add curl \

 WORKDIR /root/


Is this line required?

Probably not, I think we can safely remove. Thanks Nic!

nicholasjackson · 2017-11-04T13:06:11Z

@ericstoekl @alexellis I have tested a python 2 function, including pip to install a package and it seems all ok. There is one line in the docker file: WORKDIR /root/. Which does not seem required any more?

https://github.com/openfaas/faas-cli/pull/91/files#diff-2c26f08f6712431fc39c186c9bf2117cL10

Temp file is also written

nicholasjackson · 2017-11-04T13:21:34Z

@ericstoekl @alexellis I have also tested python 3, including pip to install a package and all seems ok.

Temp file is also written:

open-derek · 2017-11-04T19:10:17Z

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off.
That's something we need before your Pull Request can be merged. Please see our contributing guide.

ericstoekl · 2017-11-04T20:07:12Z

So I've put together a script that lets anyone easily test this change. If you have faas-cli installed and faas deployed on docker swarm, you can run the following command to test this change. If you have it deployed with kubernetes, you will need to edit the script to include the --gateway flag pointing at your gateway with each faas-cli command.

curl -sSL https://gist.githubusercontent.com/ericstoekl/3d30624f9b7b78adcb1c2ca69d17c895/raw/cfe441221483213d1249c3f70c09b5db7a00bf6f/nonRootTemplateTest.bash | bash

It will download the templates from this PR and build a function in node, python, ruby, and csharp, then call each function.

Lewiscowles1986 · 2017-11-04T22:32:20Z

Tested, works.

C Sharp function reachable
Node function reachable
Ruby function reachable
Python function reachable
Python 3 function reachable

alexellis · 2017-11-05T20:30:36Z

This script is good for a cursory check but we have to ensure there is no breakage for getting/installing/accessing 3rd-party dependencies too. I also found some issues with the .lock file not being written properly to /tmp/. You can check on docker ps to see if the function is healthy (has a lock)

ericstoekl · 2017-11-06T16:48:16Z

To test lock file status, I created functions from the node, python, ruby, and csharp templates using the script posted above, and checked each deployed function with docker service logs:

$ docker service logs p1
p1.1.t8fxqzolchik@s-GA-78LMT-USB3-6-0    | 2017/11/06 16:45:58 Writing lock-file to: /tmp/.lock
$ docker service logs r1
r1.1.s7vatldn0qvu@s-GA-78LMT-USB3-6-0    | 2017/11/06 16:45:58 Writing lock-file to: /tmp/.lock
$ docker service logs c1
c1.1.xweirwsmxjwe@s-GA-78LMT-USB3-6-0    | 2017/11/06 16:46:00 Writing lock-file to: /tmp/.lock
$ docker service logs n1
n1.1.yunekpnk4r0a@s-GA-78LMT-USB3-6-0    | 2017/11/06 16:45:53 Writing lock-file to: /tmp/.lock

alexellis · 2017-11-06T17:27:53Z

template/python/Dockerfile

@@ -1,27 +1,34 @@
 FROM python:2.7-alpine

+RUN adduser app -D


Templates need to add a group too so they follow the pattern of the Node example.

RUN addgroup -S app && adduser -S -g app app

Then chown to app:app

Please also chmod /tmp to 777 for all templates to get around buildkit/containerd issue.

alexellis · 2017-11-06T17:28:04Z

template/python/Dockerfile

@@ -1,27 +1,34 @@
 FROM python:2.7-alpine

+RUN adduser app -D
+
 # Alternatively use ADD https:// (which will not be cached by Docker builder)
 RUN apk --no-cache add curl \ 
    && echo "Pulling watchdog binary from Github." \
    && curl -sSL https://github.com/openfaas/faas/releases/download/0.6.1/fwatchdog > /usr/bin/fwatchdog \


This version is now 0.6.9 please update.

alexellis · 2017-11-06T17:29:00Z

template/python/Dockerfile

 # Alternatively use ADD https:// (which will not be cached by Docker builder)
 RUN apk --no-cache add curl \ 
    && echo "Pulling watchdog binary from Github." \
    && curl -sSL https://github.com/openfaas/faas/releases/download/0.6.1/fwatchdog > /usr/bin/fwatchdog \
    && chmod +x /usr/bin/fwatchdog \
    && apk del curl --no-cache

-WORKDIR /root/
+RUN mkdir -p /home/app/function


What's going on with the ordering here? Does it optimize the build?

This is the same ordering used in the node template, which I am basing these templates off of.

Edit -- actually looks like this line is unnecessary. Removing.

alexellis

Really close, few minor changes needed.

alexellis · 2017-11-11T17:54:22Z

Derek add label: risk/high

alexellis · 2017-11-11T18:02:37Z

Review given. This is a high risk item so we can't afford for mistakes. Thank you for taking care with this.

johnmccabe · 2017-11-15T22:26:38Z

@ericstoekl can you update/respond to @alexellis' comments and we can get this closed out.

ericstoekl · 2017-11-24T22:20:23Z

@alexellis Updated to address comments from the review. Sorry for the delay, this one got lost in the shuffle somehow 😕

To test this out to verify it works, run the following:

curl -sSL https://gist.githubusercontent.com/ericstoekl/3d30624f9b7b78adcb1c2ca69d17c895/raw/de78492095c15c81682c4d3a05c5088880267f25/nonRootTemplateTest.bash | bash

When it's done running you should see this kind of output:

Lock file for nodefunction1: error is 0
app
Lock file for csharpfunction1: error is 0
app
Lock file for rubyfunction1: error is 0
app
Lock file for pythonfunction1: error is 0
app
Lock file for python3function1: error is 0

This shows that for each function template, the user is app and no error was encountered when accessing the /tmp/.lock file.

ericstoekl · 2017-11-25T22:20:05Z

Thanks to @austinfrey we found some issues in the current status of this PR -- the csharp function fails to build due to the adduser command not being correct. I'm working to resolve this.

Edit -- updated the PR with a squashed commit, addgroup and useradd now work on the csharp version.

Set up a new deployment (deploy_stack.sh in faas repo). Get the latest faas-cli version. Then test it out with this command: curl -sSL https://gist.githubusercontent.com/ericstoekl/3d30624f9b7b78adcb1c2ca69d17c895/raw/5995f8c1ad2b3cebd6e855770b75d08925d389ee/nonRootTemplateTest.bash | bash

austinfrey · 2017-11-26T03:20:47Z

lgtm

…ROOT Signed-off-by: Eric Stoekl <ems5311@gmail.com>

Signed-off-by: Eric Stoekl <ems5311@gmail.com>

alexellis · 2017-12-02T18:56:46Z

Please can we raise against this site too? https://github.com/openfaas/templates

Templates are graduating up.

alexellis · 2018-01-19T09:03:21Z

@ericstoekl templates are graduating to the new repo so I'm closing this PR. Please raise it against the new repo.

alexellis reviewed Sep 13, 2017

View reviewed changes

ericstoekl force-pushed the templateNonRoot branch from 46b41bc to 17cdcee Compare September 15, 2017 02:32

open-derek bot added the status/testing label Oct 1, 2017

austinfrey mentioned this pull request Oct 6, 2017

Add Go template capability for creating functions #145

Merged

11 tasks

alexellis changed the title ~~Dockerfile for python, ruby, and C# template should not use ROOT~~ Final non-root changes to Python, Ruby and C# templates Oct 27, 2017

ericstoekl force-pushed the templateNonRoot branch from 17cdcee to 4d7dd84 Compare October 29, 2017 20:48

alexellis force-pushed the master branch from da0b406 to 5cd3ea2 Compare November 3, 2017 13:05

nicholasjackson reviewed Nov 4, 2017

View reviewed changes

open-derek bot added the no-dco label Nov 4, 2017

ericstoekl force-pushed the templateNonRoot branch from 20504af to c94b058 Compare November 4, 2017 20:01

open-derek bot removed the no-dco label Nov 4, 2017

alexellis reviewed Nov 6, 2017

View reviewed changes

alexellis requested changes Nov 6, 2017

View reviewed changes

open-derek bot added the risk/high label Nov 11, 2017

ericstoekl force-pushed the templateNonRoot branch from c94b058 to 856bb7c Compare November 24, 2017 22:14

ericstoekl force-pushed the templateNonRoot branch from 856bb7c to 8146ee5 Compare November 25, 2017 22:42

ericstoekl added 2 commits November 27, 2017 08:11

Dockerfile for python, python3, ruby, and C# template should not use …

d905bda

…ROOT Signed-off-by: Eric Stoekl <ems5311@gmail.com>

Add user to groups, bump watchdog version

345fd09

Signed-off-by: Eric Stoekl <ems5311@gmail.com>

ericstoekl force-pushed the templateNonRoot branch from 8146ee5 to 345fd09 Compare November 27, 2017 16:21

alexellis closed this Jan 19, 2018

alexellis mentioned this pull request May 8, 2018

Update C# template to use non-root user openfaas/templates#24

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Final non-root changes to Python, Ruby and C# templates #91

Final non-root changes to Python, Ruby and C# templates #91

ericstoekl commented Sep 11, 2017

alexellis commented Sep 12, 2017

alexellis Sep 13, 2017

ericstoekl Sep 13, 2017

alexellis commented Oct 1, 2017

alexellis commented Oct 27, 2017

ericstoekl commented Oct 27, 2017

alexellis commented Oct 27, 2017

Lewiscowles1986 commented Oct 27, 2017

Lewiscowles1986 commented Oct 27, 2017

ericstoekl commented Oct 29, 2017

nicholasjackson Nov 4, 2017

ericstoekl Nov 4, 2017

nicholasjackson commented Nov 4, 2017 •

edited

Loading

nicholasjackson commented Nov 4, 2017

open-derek bot commented Nov 4, 2017

ericstoekl commented Nov 4, 2017 •

edited

Loading

Lewiscowles1986 commented Nov 4, 2017 •

edited

Loading

alexellis commented Nov 5, 2017

ericstoekl commented Nov 6, 2017

alexellis Nov 6, 2017

alexellis Nov 6, 2017

alexellis Nov 6, 2017

ericstoekl Nov 24, 2017 •

edited

Loading

alexellis left a comment

alexellis commented Nov 11, 2017

alexellis commented Nov 11, 2017

johnmccabe commented Nov 15, 2017

ericstoekl commented Nov 24, 2017

ericstoekl commented Nov 25, 2017 •

edited

Loading

austinfrey commented Nov 26, 2017

alexellis commented Dec 2, 2017

alexellis commented Jan 19, 2018

		@@ -9,19 +11,26 @@ RUN apk --no-cache add curl \

		WORKDIR /root/

Final non-root changes to Python, Ruby and C# templates #91

Final non-root changes to Python, Ruby and C# templates #91

Conversation

ericstoekl commented Sep 11, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

alexellis commented Sep 12, 2017

alexellis Sep 13, 2017

Choose a reason for hiding this comment

ericstoekl Sep 13, 2017

Choose a reason for hiding this comment

alexellis commented Oct 1, 2017

alexellis commented Oct 27, 2017

ericstoekl commented Oct 27, 2017

alexellis commented Oct 27, 2017

Lewiscowles1986 commented Oct 27, 2017

Lewiscowles1986 commented Oct 27, 2017

ericstoekl commented Oct 29, 2017

nicholasjackson Nov 4, 2017

Choose a reason for hiding this comment

ericstoekl Nov 4, 2017

Choose a reason for hiding this comment

nicholasjackson commented Nov 4, 2017 • edited Loading

nicholasjackson commented Nov 4, 2017

open-derek bot commented Nov 4, 2017

ericstoekl commented Nov 4, 2017 • edited Loading

Lewiscowles1986 commented Nov 4, 2017 • edited Loading

alexellis commented Nov 5, 2017

ericstoekl commented Nov 6, 2017

alexellis Nov 6, 2017

Choose a reason for hiding this comment

alexellis Nov 6, 2017

Choose a reason for hiding this comment

alexellis Nov 6, 2017

Choose a reason for hiding this comment

ericstoekl Nov 24, 2017 • edited Loading

Choose a reason for hiding this comment

alexellis left a comment

Choose a reason for hiding this comment

alexellis commented Nov 11, 2017

alexellis commented Nov 11, 2017

johnmccabe commented Nov 15, 2017

ericstoekl commented Nov 24, 2017

ericstoekl commented Nov 25, 2017 • edited Loading

austinfrey commented Nov 26, 2017

alexellis commented Dec 2, 2017

alexellis commented Jan 19, 2018

nicholasjackson commented Nov 4, 2017 •

edited

Loading

ericstoekl commented Nov 4, 2017 •

edited

Loading

Lewiscowles1986 commented Nov 4, 2017 •

edited

Loading

ericstoekl Nov 24, 2017 •

edited

Loading

ericstoekl commented Nov 25, 2017 •

edited

Loading