Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

profiles: gnome-keyring: harden & add gnome-keyring-daemon #6201

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

glitsj16
Copy link
Collaborator

@glitsj16 glitsj16 commented Feb 7, 2024

We were missing a profile for gnome-keyring-daemon. Let's bring that in.
Also slightly hardened gnome-keyring.profile.

@kmk3 kmk3 force-pushed the gnome-keyring-fixes branch from 6e4a4ae to 0605c04 Compare February 8, 2024 13:22
@kmk3 kmk3 changed the title gnome-keyring: hardening and introduce gnome-keyring-daemon.profile gnome-keyring: harden and add gnome-keyring-daemon.profile Feb 8, 2024
@kmk3
Copy link
Collaborator

kmk3 commented Feb 8, 2024

Rebased to make the changes clearer.

Please make sure to use a normal merge rather than squash to keep the hardening
and refactor commits separate.

@glitsj16
Copy link
Collaborator Author

glitsj16 commented Feb 8, 2024

@kmk3

Please make sure to use a normal merge rather than squash to keep the hardening
and refactor commits separate.

Didn't realize the default I have on GH is still 'squash'. Changing that now!

@glitsj16 glitsj16 merged commit eded5cc into netblue30:master Feb 8, 2024
8 checks passed
@glitsj16 glitsj16 deleted the gnome-keyring-fixes branch February 8, 2024 13:43
@kmk3
Copy link
Collaborator

kmk3 commented Feb 8, 2024

@glitsj16 on Feb 8:

Please make sure to use a normal merge rather than squash to keep the hardening
and refactor commits separate.

Didn't realize the default I have on GH is still 'squash'. Changing that now!

It depends, I'd avoid squashing if it looks like the author intended for the
commits to be separate, especially if there are multiple logical changes.

And I'd be inclined to organize the branch and/or squash it when there are
amend/fixup commits.

Also, when there is a single commit/single logical change it seems helpful to
squash to avoid the extra noise of a merge commit.

So it depends on whether the author already intentionally split them and
whether squashing would make the end result more or less clear.

@glitsj16
Copy link
Collaborator Author

glitsj16 commented Feb 8, 2024

Heh, the good old it depends... situation. Thanks again for explaining! I do realize my contributions via GH aren't always 'clean' (understating it). On a side-note I wish to convey my appreciation for the recent landlock work you've been doing. I can start to grasp it all now and am impressed by the speed of it. Interesting times ahead! Have fun.

@kmk3
Copy link
Collaborator

kmk3 commented Feb 9, 2024

@glitsj16 on Feb 8:

Heh, the good old it depends... situation. Thanks again for explaining! I
do realize my contributions via GH aren't always 'clean' (understating it).

It's alright, usually the changes are related and localized enough that
squashing is workable.

In the case of profiles, it's mostly when moving code/renaming files that
keeping such a change in its own separate commit makes any other changes much
clearer.

On a side-note I wish to convey my appreciation for the recent landlock work
you've been doing. I can start to grasp it all now and am impressed by the
speed of it. Interesting times ahead! Have fun.

Glad to hear it! I'm not sure how recent you mean, but the refactoring in
#6078 was indeed a bit mind-bending, as there was a lot of cleanup to do, the
changes had different authors and some changes were instructive enough to
warrant ensuring that they had their own commits.

For example, the following commits show how to add a linked list for a new
path-related command and how to process it all at once at a specific time
(instead of applying each command at parse time):

  • b94cc75 ("landlock: apply rules in sandbox before app start", 2023-10-26)
  • 520508d ("landlock: avoid parsing landlock commands twice", 2023-11-02)

Which is what made it easy to make the application of all landlock commands
conditional in #6125 (in sandbox.c):

  • 760f50f ("landlock: move commands into profile and add landlock.enforce",
    2023-11-17)

In #6200 I was just lucky that my first guess was correct :)

@kmk3 kmk3 changed the title gnome-keyring: harden and add gnome-keyring-daemon.profile profiles: gnome-keyring: harden and add gnome-keyring-daemon Sep 7, 2024
@kmk3 kmk3 changed the title profiles: gnome-keyring: harden and add gnome-keyring-daemon profiles: gnome-keyring: harden & add gnome-keyring-daemon Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants