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

selinux: implemented remaining selinux functions #2850

Merged
merged 11 commits into from
Aug 16, 2024

Conversation

Gekko0114
Copy link
Contributor

@Gekko0114 Gekko0114 commented Jul 14, 2024

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
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
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
@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?

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
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
@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 😍

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? 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
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?

Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
@utam0k
Copy link
Member

utam0k commented Aug 3, 2024

Is this PR ready for re-reviewing?

Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
@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

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

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 containers:main Aug 16, 2024
28 checks passed
@github-actions github-actions bot mentioned this pull request Aug 16, 2024
@Gekko0114 Gekko0114 deleted the selinux branch August 17, 2024 14:57
@github-actions github-actions bot mentioned this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/experimental `/experimental`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants