I've finished dockerizing fbctf both in dev and prod mode :D #36
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
I've alredy signed it. Thanks. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
lol I was about to do this! thanks!! |
|
I also started something, but I took a different path, I ended up not using I also think it would be better to split the processes into multiple containers (MySQL, memcached, hhvm/nginx). Having multiple processes per container is a bit against the docker philosophy isn't it ? :) |
|
@AlexGaspar I agree with you, there are lots of stuffs that docker actually doesn't need them to be installed. Right now, I'm just making an attempt to modify less changes as possible as I can to let docker run successfully. Maybe we could try to add another provision script to handle the processing of dockerizing eventually. Spliting processes into multiple containers is indeed a better way, we can do this after things mentioned above have been resolved. Or do you have any progress on it? Thanks for your suggestions btw :) |
|
why? |
|
Thanks for the PR! We'll review it over the next few days. I definitely agree some tools aren't necessary for prod mode (i.e. emacs, htop, mycli, unison). We'll modify the provision script to install these only in dev mode. We're fine with either the single container or multiple container approach. |
|
@qazbnm456 Yep I started this : https://github.com/AlexGaspar/docker-fbctf if you find value in what I've done we could perhaps merge our 2 things? I've spltited the resources into multi containers, but I didn't include the SSL termination, I don't think it should be the responsibility of the fbctf container. In the other hand, fbctf force the session's cookie to |
|
@AlexGaspar i agree to both, it should be splitted and ssl termination should be the job of the orchestration platform which often comes with it's own loadbalancer. in my case i want to make fbctf ready for sloppy.io but right now it's not possible. i changed the nginx conf to http but login is not working. |
|
hey guys i am new in this and i am arabian so can you give me some help |
|
@AlexGaspar Neat, I'll look into it when I'm available, many thanks :) @AlexGaspar @MikeMichel Due to this line of code, I'll reserve my approach about ssl termination included container, which may help those who are not familiar with CTF get involved more easily. Thanks for your suggestions, I'm really appreciate. :) |
|
@RW34 What kind of help do you need? :) |
|
While I completely understand why people would want to have their own control over the way SSL is implemented, we wanted to make the platform as simple as possible for someone to setup and use. We assume that people who want more advanced setups can tweak the platform as necessary, while people with less experience can use it as is and still have a secure setup. |
|
YOUR DOCKER CONTAINER ACTUALLY HAS FLAWS IN IT. It would not build, and hangs up at the running MYSQL phase of the end of the process |
|
@Jovonni Do you mean you get stuck in your building process on |
|
@qazbnm456 No, I actually let it run for almost an hour. I also tried it on two different machines running docker, it still hangs on the same part. I don't know what it is. Have you not encountered this? |
|
@Jovonni Weird, I've not encountered this... Following is my recording of building process: By the way, could you show me your recording as well which can possibly let me dive into solving potential problems. Thank you for your feedback. |
| COPY extra/service_startup.sh /etc/my_init.d/02_service_startup.sh | ||
| COPY extra/import_empty_db.sh /etc/my_init.d/03_import_empty_db.sh | ||
| RUN [ "$MODE" = "dev" ] && rm /etc/my_init.d/01_setup_lets_encrypt.sh || true | ||
| RUN [ "$MODE" = "prod" ] && sed -i 's/domain.com/'$DOMAIN'/g' /etc/my_init.d/01_setup_lets_encrypt.sh || true |
There was a problem hiding this comment.
I think many people will be terminating TLS at their reverse proxy. Is it possible to skip all of the TLS stuff inside of the container?
There was a problem hiding this comment.
As @gsingh93 said, this Dockerfile aims to be friendly to newbies, which means advanced hackers are supposed to tweak the platform as necessary. Thus, no modification will be made currently.
|
I'll be looking at this and testing it locally once I get #105 merged, at which point this should be rebased on top of that. Overall it looks good other than the minor things I already commented on. I'd also like to not duplicate the We're going to have to host our own version of your |
|
@gsingh93 Okay, I'll await your work and rebase this PR while yours get merged. Then, I'll do further modification if it need to. |
|
@gsingh93 I've made some changes, and it works on both dev and prod mode. Does it look good right now? |
| @@ -167,7 +177,10 @@ function install_nginx() { | |||
| own_cert "$__cert" "$__key" | |||
| ;; | |||
| certbot) | |||
| letsencrypt_cert "$__cert" "$__key" "$__email" "$__domain" | |||
| if [[ $__docker = true ]]; then | |||
There was a problem hiding this comment.
Why do we need to do this?
There was a problem hiding this comment.
I've explained it in detail below.
|
Looking much better. I'm assuming you're creating certbot.sh to renew the cert on load it expires? Or is it for a different reason? |
|
certbot.sh is actually doing the same thing. What resides in certbot.sh is something like: #!/bin/bash
/usr/bin/certbot-auto certonly -n --agree-tos --standalone --standalone-supported-challenges tls-sni-01 -m "xxx@xxx.com" -d "xxx.com"
sudo ln -sf "/etc/letsencrypt/live/xxx.com/cert.pem" "/etc/nginx/certs/fbctf.crt"
sudo ln -sf "/etc/letsencrypt/live/xxx.com/privkey.pem" "/etc/nginx/certs/fbctf.key"The reason for doing this is while we are building the fbctf image, there will no available internet connections for certbot-auto to get our certs. Thus, I've added the docker flag as a hint to tell provision.sh to postpone the process of getting certs, and "writing those scripts into another certbot.sh" is the countermeasure I choose to implement. As you can see that in service_startup.sh, it will check if there is a certbot.sh in ~/tmp/. If there is one, it will start the script to get required certs. |
|
Ok, that makes sense, but the only thing I'm worried about now is that if the cert is created when you run the docker container, running the container 5 times will generate 5 different certs for the same domain and then you'll hit the letsencrypt rate limit. Any thoughts on that? |
ghost
mentioned this pull request
|
There are actually many ways users can do to prevent recreating certs. All they must to do is copy those certs in container to host, then they can choose to rebuild image with type=own , or overwrite the CMD and mount their certs into container. Therefore, I think it depends on how users interact to their containers. Docker is just another easy way for beginners to run an instance, and they can tune it by themselves. But a line of notice is probably needed :) How do you think, @gsingh93 ? |
| set -e | ||
|
|
||
| if [[ -e /root/tmp/certbot.sh ]]; then | ||
| /bin/bash /root/tmp/certbot.sh |
There was a problem hiding this comment.
Once we execute this, we can clear the file, so there is no risk on generating a new certificate again.
There was a problem hiding this comment.
Nevermind my comment, I didn't realize the file will still be there inside the container. The only way to handle this without taking all the certificates will be to handle that outside of the container and just copy the files...
|
Can we mount /etc/letsencrypt as a volume, so the data there persists? |
|
@gsingh93 Yes, we can. However, I think I can make |
|
@javuto @gsingh93 Okay, I've made some changes. This time, If we mount /etc/letsencrypt as a volume, it will call How do you think? Is it much better now? |
|
Other than my comment about the heredoc above, this looks good now. |
| ADD . $HOME | ||
| RUN chown www-data:www-data $HOME | ||
| RUN ./extra/provision.sh -m $MODE -c $TYPE -k $KEY -C $CRT -D $DOMAIN -e $EMAIL -s `pwd` --docker | ||
| RUN apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* |
There was a problem hiding this comment.
I'm no docker expert, but I've heard it's better practice to put lines like this in the same RUN command as the ones where the files were added (the apt-get update command). The reason is even though you delete them before the final image is created, docker has already created a FS layer that includes the files, so the image size still increases.
|
Also, maybe the README should contain an example |
|
Kickass PR @qazbnm456, @gsingh93 merge at will! |
|
Thanks @qazbnm456! |
I've added some files in extra/ to make dockerizing more easier, and updated README as well.
Would you please have a look at it? Thanks!