I've just pushed a new commit that duplicates the Dockerfile

• 1 Dockerfile for everyone : it builds as the local user
• 1 Dockerfile dedicated to github Actions. It removes the USER instruction so that it can run in Github environment.

That's a pity I had to duplicate the docker files, but I couldn't find any other solution...

Thanks to @NeroBurner, we may have found a solution to the duplication of Dockerfile.

With this command:

docker run --rm -it -v ${PWD}:/sources -v /etc/passwd:/etc/passwd:ro -v /etc/group:/etc/group:ro -v /etc/shadow:/etc/shadow:ro -u 
$(id -u ${USER}):$(id -g ${USER})  jf002/infinitime-build-github

the "github" docker (the one that does not use the USER instruction builds the firmware as my local user, and the build file are created with the correct permission.

Do you think it's ok to add those -v and -u parameters to the command line?

Short answer: yes I think it is OK (although no docker expert, I just pieced this solution together to the point where it works)

-v does a bind mount, -u sets the user to use inside docker

the bind mounts of /etc/passwd, /etc/group and /etc/shadow are read only (because of the :ro flag). Those are needed for the -u flag to have the host-user and group available inside the container. Otherwise we'd have to create a user inside the container (in Dockerfile), and that user inside the container may have a different uuid than the user on the host machine. Which would again lead to files on the host with the wrong owner.

Long answer: a possible security implication against the mounting of /etc/{passwd,group,shadow} would be if we run an untrusted image, which could then collect the password-hash and send it off to somewhere. But then again this could happen with every untrusted docker container, as a malicious image could escape the container boundaries and access stuff on the root system (as far as I know).
We are running a trusted image, which we create. So I think it is OK to add those flags to the commandline

Great, thanks @NeroBurner !
So.. this might be the solution we need to avoid duplicating anything regarding the CI :)

you don't need to have a user and group for chown and similar things to work, you can use directly the id - and then the mounts should not be needed.
this works also when the user(and group) in question don't exist.

Also, even if you do need passwd and group, why would you need shadow?

for chown you still need to pass the user id and group id to the container via some means, probably set an environment variable and add another step inside the container to do the chown if the id isn't the one of root (=0 I think).

Yes, you're probably right about /etc/shadow not being necessary 🤔 I ran a user session inside the container with needing sudo to work. So trying without /etc/shadow would limit the potential attack surface even more

for chown you still need to pass the user id and group id

exactly - but you don't need to know the user:userid translations (passwd) or who is in each group (group), so long as you pass the correct ones.
and to switch to some user via sudo, normally[1] root can switch without any password to any other user - so you don't need shadow, which is also the worst surface. (and just because both the image, and what is run inside is known - does not mean security can be ignored. for example some stuff are installed, via pip and npm. those add outside control. I admit that it might make sense to ignore some precautions as a choice but not blindly)

and just as you can pass in the username, you can pass in the user. (for example: the environment variable UID)

needing sudo to work

it's better (I think) in this case to allow the docker "user" to use sudo without needing a password then letting it use your password. (this can be done with USERNAME ALL=(ALL:ALL) NOPASSWD: ALL this can also work with UID, with a User_Alias OURUSER = #1000 which if needed for arbitrary id's can complicate things, in which case it might make sense to create such a user, or even allow for ALL)

[1] sudoers tends to have root ALL=(ALL:ALL) ALL or some such. and not many want to remove it. (and this is done by default on almost any distro. (or maybe even all))

Actually, a simpler solution when shadow is really needed - create a new one. (which can even be a copy of one that worked - but not you passwords (hashes, bu nonetheless))

In this case, I don't think we need sudo. What I'm trying to fix here are the permissions on the file created by the container : they are owned by root and cannot be modified/deleted by the local user.

In this case, I don't think we need sudo. What I'm trying to fix here are the permissions on the file created by the container : they are owned by root and cannot be modified/deleted by the local user.

in that case, just used the uid and gid. (pass them in via env variable or some such[1], and set them to 0 (root) otherwise.)
(and chown 1000:1000 -R folder for example)

[1] for example, assume that the source files have the correct permissions, and use that as a reference.

If I understand correctly, this would be better to run the container as a user instead of root.
According to the documentation, the user can be specified when building the container using the USER instruction (which is not supported by github). It can also be overridden when running the container with the -u parameter. The doc also mentions

When passing a numeric ID, the user does not have to exist in the container.

So... it should just work without mounting /etc/passwd, /etc/group and /etc/shadow, right?

docker run --rm -it -v ${PWD}:/sources -u $(id -u ${USER}):$(id -g ${USER})  jf002/infinitime-build-github
...
... Building
...

ls -l                                                                                                                                                                             ✔ 
total 1704
-rw-r--r--  1 jf jf    3373 May  8 20:49 CMakeLists.txt
lrwxrwxrwx  1 jf jf      17 Mar  8 18:31 CONTRIBUTING.md -> doc/contribute.md
-rw-r--r--  1 jf jf   35148 Feb 14  2021 LICENSE
-rw-r--r--  1 jf jf    4251 May  8 13:37 README.md
drwxr-xr-x  3 jf jf    4096 Mar  8 18:31 bootloader
drwxr-xr-x  5 jf jf    4096 May 10 11:04 build
drwxr-xr-x  5 jf jf    4096 May  8 21:04 cmake-build-debug-nrf52
drwxr-xr-x  5 jf jf    4096 May  8 21:04 cmake-build-release-nrf52
drwxr-xr-x  3 jf jf    4096 Mar  8 18:31 cmake-nRF5x
drwxr-xr-x  8 jf jf    4096 May  8 20:49 doc
drwxr-xr-x  2 jf jf    4096 May 10 11:00 docker
-rw-r--r--  1 jf jf    3089 Mar  8 18:31 gcc_nrf52-mcuboot.ld
-rw-r--r--  1 jf jf    3090 Mar  8 18:31 gcc_nrf52.ld
drwxr-xr-x  2 jf jf    4096 Mar  8 18:31 hooks
drwxr-xr-x  6 jf jf    4096 Mar  8 18:31 images
-rw-r--r--  1 jf jf 1630719 Jul 25  2021 nrf52.svd
-rw-r--r--  1 jf jf    4475 May 30  2021 nrf_common.ld
drwxr-xr-x 12 jf jf    4096 May  8 20:49 src
drwxr-xr-x  3 jf jf    4096 Mar  8 18:31 tools

Notice that the build directly belongs to jf:jf, which is my local user/group.

Would this be an acceptable solution?

Would this be an acceptable solution?

not my PR, (or my project), so I really don't have any say in the matter - but security wise, it is.

not my PR, (or my project), so I really don't have any say in the matter - but security wise, it is.

But you're still free to provide your expertise 👍 Thanks!

With the last commit, the same Docker image can be used on my computer and on Github Action.
No specific parameter is given to Github. On my computer, I used this command line:

docker run --rm -it -v ${PWD}:/sources -u $(id -u ${USER}):$(id -g ${USER})  jf002/infinitime-build

I pushed an updated Docker image to DockerHub (https://hub.docker.com/repository/registry-1.docker.io/jf002/infinitime-build/tags). It's built for X86_64 and ARM64.

Sooo... Are we ready to switch the CI to Docker-inside-GitHubActions?

I think it's almost ready, but I've noticed that the actions run on this PR didn't upload a DFU file, it says it couldn't find it.

Maybe the location that it looks for the files needs to be changed, or the location the docker image puts its output need to be mounted on the "host"?

I think it's almost ready, but I've noticed that the actions run on this PR didn't upload a DFU file, it says it couldn't find it.

Maybe the location that it looks for the files needs to be changed, or the location the docker image puts its output need to be mounted on the "host"?

Thanks for checking! It looks like there's an issue with the artifacts, indeed. I'll have a look at this asap!

@FintasticMan It should be fixed, but I don't know why, the workflow was not triggered...

Aaaaah now it seems to work!

I had to workaround a new security fix that was added in git (since we upgraded to a new version of the ubuntu container). More info here and here.

Ok, I don't know what happened here, I've probably failed a merge commit or something, and this branch reverts changes that were made 2 months ago in nimble_port_freertos.c.
I've decided to create a new clean branch to ensure I won't break anything when merging it. The new PR is #1138. I'll keep this one to keep the history and the discussion about the integration of docker in the workflow.