lldap / lldap

Light LDAP implementation
GNU General Public License v3.0
4.34k stars 201 forks source link

[FEATURE REQUEST] Build `rootless` container image #755

Closed onedr0p closed 9 months ago

onedr0p commented 10 months ago

Motivation

LLDAP cannot not run as nonroot with strict security constraints because it uses gosu and chowns data on start. As a tool that is built for security, best practices should be followed and gosu should be removed in favor of the supported method provided by the Docker runtime.

https://github.com/lldap/lldap/blob/70d85524db4d7070f1fda499702bbbd3ec2dd8b5/docker-entrypoint.sh#L24-L26

The error I get is when I set readOnlyRootFilesystem to true in Kubernetes securityContext:

k logs -n security lldap-6bf866cc88-lnqtv
Defaulted container "main" out of: main, init-db (init)
> Setup permissions..
chown: /app: Read-only file system

Describe the solution you'd like

Remove gosu and support the official method of using the user option in Docker Compose:

user Optional User and Group ID you want to run the container as. lldap will run using this UID:GID and any files it creates in your /config volume will also be owned by this user and group. The default for this, if not specified, is 1000:1000.

networks:
  lldap:
    name: lldap
    external: true

services:
  recyclarr:
    image: ghcr.io/lldap/lldap
    container_name: lldap
    user: 1000:1000
    networks: [lldap]
    volumes:
      - ./lldap_data:/data
    environment:
      - TZ=America/Santiago
onedr0p commented 10 months ago

I am willing to PR this change with the supporting docs around it.

martadinata666 commented 10 months ago

Initially LLDAP also adopt that conformity, v0.3 *cmiiw. The gosu was used to solve installation on synology, or NAS software alike, essentially when deployed LLDAP got permission error to read/write /data and /app even when specify user: xxxx:xxxx. Tbf I don't use syno, so I don't know how their mounting permission works. If this can be cleared up maybe we can adopt docker method.

onedr0p commented 10 months ago

This would be a breaking change for some users of the project, because they will need to adopt the official way to handle it. It's been years since I ran Syno but IIRC it is just docker-compose. So as long as the user sets that user var and chowns the data folder themselves prior to deploying lldap or uses the uid:gid that already is set on the lldap data folder they should not have an issue.

nitnelave commented 10 months ago

The current image uses gosu in order to solve common permission problems that made installation harder. I'd like to keep it that way, in part because of compatibility reasons.

However, I'm entirely open to also having a non root image that you can opt into, with a sightly different entry point. We can automate it in GitHub actions to have both images kept up to date.

Would that work for you?

On Sun, 10 Dec 2023, 16:21 Devin Buhl, @.***> wrote:

This would be a breaking change for some users of the project, because they will need to adopt the official way to handle it. It's been years since I ran Syno but IIRC it is just docker-compose. So as long as the user sets that user var and chowns the data folder themselves prior to deploying lldap or uses the uid:gid that already is set on the lldap folder they should not have an issue.

— Reply to this email directly, view it on GitHub https://github.com/lldap/lldap/issues/755#issuecomment-1848994473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGCPWNA7XGPFGNXJVFDTJTYIXHN3AVCNFSM6AAAAABAOTZXB2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHE4TINBXGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

onedr0p commented 10 months ago

That works, the way I've seen other projects do that is by adding a -non-root suffix to the tag. I have updated the issue title.

martadinata666 commented 9 months ago

Added support with rootless image. It expected run with user 1000 by default, override with user: xxxx:yyyy.

But I'm not really sure how it interacts with this The error I get is when I set readOnlyRootFilesystem to true in Kubernetes securityContext:

nitnelave commented 9 months ago

The new entrypoint doesn't try to chown/copy anything if the config file already exists, so it should be compatible with a read-only FS