element-web icon indicating copy to clipboard operation
element-web copied to clipboard

Specify a local address when exposing ports with Docker

Open gibson042 opened this issue 3 years ago • 8 comments

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:80 instead 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.

gibson042 avatar Feb 03 '22 15:02 gibson042

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:80 instead 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).

gibson042 avatar Feb 14 '22 15:02 gibson042

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).

novocaine avatar Feb 14 '22 16:02 novocaine

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.

turt2live avatar Feb 14 '22 16:02 turt2live

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.

gibson042 avatar Feb 15 '22 18:02 gibson042

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 avatar Feb 16 '22 09:02 novocaine

@novocaine Done.

gibson042 avatar Feb 17 '22 18:02 gibson042

CLA assistant check
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.

CLAassistant avatar Sep 06 '24 17:09 CLAassistant

CLA assistant check
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.

CLAassistant avatar Sep 06 '24 17:09 CLAassistant

@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 avatar Oct 31 '24 14:10 t3chguy

@t3chguy Done.

gibson042 avatar Oct 31 '24 18:10 gibson042