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.

Sign up for GitHub

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

Jump to bottom

selinux: implemented remaining selinux functions #2850

Merged
merged 11 commits into from
Aug 16, 2024
Merged

selinux: implemented remaining selinux functions #2850

merged 11 commits into from
Aug 16, 2024

Conversation

Gekko0114
Copy link
Contributor

@Gekko0114 Gekko0114 commented Jul 14, 2024 •
edited
Loading

This is an experimental create. This PR is for selinux crate written in Rust.
In this PR, most of remaining selinux functions are implemented.

ref: #2718 #2800 #2825

@YJDoc2 YJDoc2 added the kind/experimental `/experimental` label Jul 15, 2024
@Gekko0114 Gekko0114 force-pushed the selinux branch 12 times, most recently from 474bb5f to 005b243 Compare July 19, 2024 09:23
@Gekko0114 Gekko0114 changed the title added selinux functions added remaining selinux functions Jul 19, 2024
@Gekko0114 Gekko0114 changed the title added remaining selinux functions selinux: added remaining selinux functions Jul 19, 2024
@Gekko0114 Gekko0114 changed the title selinux: added remaining selinux functions selinux: implemented remaining selinux functions Jul 19, 2024
@Gekko0114 Gekko0114 marked this pull request as ready for review July 19, 2024 10:33
@Gekko0114
Copy link
Contributor Author

@utam0k @YJDoc2
Could you review this PR? I think most of remaining selinux functions are covered in this PR.
Though there are several remaining functions, I am not sure all of them are necessary.

@utam0k utam0k self-requested a review July 21, 2024 03:49
utam0k
utam0k requested changes Jul 21, 2024
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

I'll still review this PR because the changes are significant. But I've done the first my review.

experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
@Gekko0114
added selinux functions
3c8d871 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
not use arc
e5952e4 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
follow reviewer comment
d02d4d8 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
Copy link
Contributor Author

Hi @utam0k

Thank you so much for your review. I've fixed or replied to your comments.
Could you take a look when you have time?

utam0k
utam0k reviewed Jul 27, 2024
View reviewed changes
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux_label.rs Outdated Show resolved Hide resolved
@Gekko0114
fix
54d8408 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
fix
d534bfe 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
fix
5a07279 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
use SELinuxLabel struct
ef12153 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
Copy link
Contributor Author

Hi @utam0k , @YJDoc2
Thanks for your review. I've updated my PR. Could you take a look?

@utam0k
Copy link
Member

utam0k commented Jul 30, 2024

I'll review it when I have more time to review. But the structure itself seemed very readable 😍

utam0k
utam0k approved these changes Aug 1, 2024
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The structure itself looks good to me +1. May I ask you to add main.rs to show an example of how to use this crate? I was mistaken.

experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/selinux.rs Show resolved Hide resolved
experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
@utam0k utam0k self-requested a review August 1, 2024 11:54
utam0k
utam0k requested changes Aug 1, 2024
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

The structure itself looks good to me +1. May I ask you to add main.rs to show an example of how to use this crate?

@Gekko0114
use pointer instead of clone
2b82887 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
not loop
f0da9cb 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@utam0k
Copy link
Member

utam0k commented Aug 3, 2024

Is this PR ready for re-reviewing?

@Gekko0114
add main.rs
6e4fbc7 
Signed-off-by: Hiroyuki Moriya <[email protected]>
@Gekko0114
Copy link
Contributor Author

@utam0k
Yes, I've added main.rs.
Please take a look when you have time :)

@utam0k
Copy link
Member

utam0k commented Aug 5, 2024

@utam0k Yes, I've added main.rs. Please take a look when you have time :)

Awesome! Does cargo run need root permission, right?

root ➜ /workspaces/youki/experiment/selinux (selinux) $ ls -Z test_file.txt 
unconfined_u:object_r:public_content_t:s1 test_file.txt

@Gekko0114
Copy link
Contributor Author

Gekko0114 commented Aug 5, 2024 •
edited
Loading

Does cargo run need root permission, right?

Ah I hadn't tried this as a root on workspace, but seems you're right (I've tried it just now).

Seems that whether main.rs can be executed successfully depends on development environment.
I wrote this code on my ec2 amazon linux because selinux is enabled.
However, on my ec2 linux env, "set_file_label" is failed because of amazon linux's setting (though I executed it as a root user).

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Aug 8, 2024

Hey @Gekko0114 , do you think adding a vagrant file (or making changes in existing ones) to enable selinux in the vagrant box would be a good/easy idea? That way we can have a common dev env to write and test these things. As you said depending on how selinux is setup on machine, the main file can succeed or fail, I think it is a good idea to have a standard env to test in. wdyt?

@Gekko0114
Copy link
Contributor Author

@YJDoc2 @utam0k
Agree, I think it is good preparing vagrant file for selinux.
Though I am not very familiar with vagrant, I can work on it.
I will take that work after completing this PR. wdyt?

@utam0k
Copy link
Member

utam0k commented Aug 16, 2024

I will take that work after completing this PR. wdyt?

+1

utam0k
utam0k approved these changes Aug 16, 2024
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

Thanks!

@utam0k utam0k merged commit c7d5992 into youki-dev:main Aug 16, 2024
28 checks passed
@github-actions github-actions bot mentioned this pull request Aug 16, 2024
Merged
@Gekko0114 Gekko0114 deleted the selinux branch August 17, 2024 14:57
@github-actions github-actions bot mentioned this pull request Aug 23, 2024
Closed
@Gekko0114 Gekko0114 mentioned this pull request Sep 1, 2024
Merged
@Gekko0114 Gekko0114 mentioned this pull request Sep 29, 2024
Merged
sat0ken pushed a commit to sat0ken/youki that referenced this pull request Oct 4, 2024
@Gekko0114 @sat0ken
selinux: implemented remaining selinux functions (youki-dev#2850)
c8eb148 
* added selinux functions

Signed-off-by: Hiroyuki Moriya <[email protected]>

* not use arc

Signed-off-by: Hiroyuki Moriya <[email protected]>

* follow reviewer comment

Signed-off-by: Hiroyuki Moriya <[email protected]>

* divided selinux impl into two files

Signed-off-by: Hiroyuki Moriya <[email protected]>

* fix

Signed-off-by: Hiroyuki Moriya <[email protected]>

* fix

Signed-off-by: Hiroyuki Moriya <[email protected]>

* fix

Signed-off-by: Hiroyuki Moriya <[email protected]>

* use SELinuxLabel struct

Signed-off-by: Hiroyuki Moriya <[email protected]>

* use pointer instead of clone

Signed-off-by: Hiroyuki Moriya <[email protected]>

* not loop

Signed-off-by: Hiroyuki Moriya <[email protected]>

* add main.rs

Signed-off-by: Hiroyuki Moriya <[email protected]>

---------

Signed-off-by: Hiroyuki Moriya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YJDoc2 YJDoc2 YJDoc2 left review comments

@utam0k utam0k utam0k approved these changes

Assignees
No one assigned
Labels
kind/experimental `/experimental`
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Gekko0114 @utam0k @YJDoc2