Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Debian packaging: explicitly allocate a group for the system user #13593

Merged
merged 7 commits into from
Aug 25, 2022

Conversation

behrmann
Copy link
Contributor

@behrmann behrmann commented Aug 23, 2022

This PR explicit system group for the system user, as currently the files owned by the matrix-synapse user will belong to the nobody group, which is not ideal, since that group will regularly be used for unrelated things, so having data owned by matrix-synapse may end up readable by other system users by accident. I don't know if there is some reasoning behind this, since the original repo containing Debian packaging doesn't seem to be available anymore, so I couldn't readily find it.

This is spun out off #13582.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

Otherwise the files of the synapse user are readable by the nobody user, which
is unsafe.

Signed-off-by: Jörg Behrmann <[email protected]>
@behrmann behrmann requested a review from a team as a code owner August 23, 2022 09:16
@behrmann
Copy link
Contributor Author

I left out the changelog entry and the Debian changelog entry so far, because I was unsure what version to add to the latter.

@behrmann behrmann mentioned this pull request Aug 23, 2022
4 tasks
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me — postgres and mysql both have their own groups.

@behrmann
Copy link
Contributor Author

While trying to answer the above question I noticed that dpkg-statoverride throws a warning about --force, which apparently is deprecated for a white bouquet of --force-foo options, with --force-statoverride-add seemingly being the one that's intended here. Would you like me to add this on top of this PR or as a separate PR?

@richvdh
Copy link
Member

richvdh commented Aug 23, 2022

since the original repo containing Debian packaging doesn't seem to be available anymore, so I couldn't readily find it.

I think you're looking for https://github.com/matrix-org/package-synapse-debian/blame/debian/0.33.9-1matrix1/debian/postinst? I don't think that helps though.

@richvdh
Copy link
Member

richvdh commented Aug 23, 2022

I left out the changelog entry and the Debian changelog entry so far, because I was unsure what version to add to the latter.

It doesn't matter - just follow the instructions in the contributing docs. It will add a temporary version which will be fixed up at release time.

Since this only affects the debian build, there is no need for a changelog in the changelog.d directory.

@richvdh
Copy link
Member

richvdh commented Aug 23, 2022

While trying to answer the above question I noticed that dpkg-statoverride throws a warning about --force, which apparently is deprecated for a white bouquet of --force-foo options, with --force-statoverride-add seemingly being the one that's intended here. Would you like me to add this on top of this PR or as a separate PR?

suggest a separate PR. I think there might be existing issues open around this.

@behrmann
Copy link
Contributor Author

It doesn't matter - just follow the instructions in the contributing docs. It will add a temporary version which will be fixed up at release time.

I test the build on a Debian machine, but my dev machine is not one and I don't have dch on it ;) I went with what I think dch would have created.

debian/changelog Outdated Show resolved Hide resolved
debian/changelog Outdated Show resolved Hide resolved
@richvdh richvdh mentioned this pull request Aug 25, 2022
4 tasks
@richvdh richvdh changed the title Explicitly allocate a group for the system user Debian packaging: explicitly allocate a group for the system user Aug 25, 2022
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@richvdh richvdh enabled auto-merge (squash) August 25, 2022 16:27
@richvdh richvdh merged commit 978666a into matrix-org:develop Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants