element-web
element-web copied to clipboard
Specify a local address when exposing ports with Docker
cf. https://docs.docker.com/engine/reference/commandline/run/#publish-or-expose-port--p---expose
This corrects what looks like accidental creation of potentially attackable network exposure. From the linked Docker documentation:
Note that ports which are not bound to the host (i.e.,
-p 80:80instead of-p 127.0.0.1:80:80) will be accessible from the outside.
The local development server should instead be confined only to the local host unless there is specific reason to make it network available (which is what this PR addresses).
This PR currently has no changelog labels, so will not be included in changelogs.
A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.
This corrects what looks like accidental creation of potentially attackable network exposure. From the linked Docker documentation:
Note that ports which are not bound to the host (i.e.,
-p 80:80instead of-p 127.0.0.1:80:80) will be accessible from the outside.
The local development server should instead be confined only to the local host unless there is specific reason to make it network available (which is what this PR addresses).
The command is suggested to serve element-web as a web server, with all the use cases that entails.
I think you might be assuming that only a development or local use-case exists, but there is also the use case of serving it to other clients on the network (e.g. run your own app.element.io with your own customisations, as many people do).
fwiw, the documentation in this area was written more as a point of interest rather than something to copy/paste. It's fairly rare that folks use bare docker commands these days, so the important aspect becomes the ports and container name.
I think you might be assuming that only a development or local use-case exists, but there is also the use case of serving it to other clients on the network (e.g. run your own app.element.io with your own customisations, as many people do).
That use case is certainly valid, but I believe running a server that supports it should be intentional rather than accidental—it's generally bad form to encourage creation of unnecessary attack surface area.
fwiw, the documentation in this area was written more as a point of interest rather than something to copy/paste. It's fairly rare that folks use bare docker commands these days, so the important aspect becomes the ports and container name.
Acknowledged. Unless you see harm in these changes, though, I still consider it valuable to default to restricted access.
I think you might be assuming that only a development or local use-case exists, but there is also the use case of serving it to other clients on the network (e.g. run your own app.element.io with your own customisations, as many people do).
That use case is certainly valid, but I believe running a server that supports it should be intentional rather than accidental—it's generally bad form to encourage creation of unnecessary attack surface area.
My suggestion is to list the different commands for the different use cases noting the implications.
@novocaine Done.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
@gibson042 sorry this slipped through the cracks but if you'd be willing to resolve the conflicts & sign the CLA we can get this merged
@t3chguy Done.