Skip to content

Ensure proper auditd/logging permissions following reboot#60

Merged
AlexHearnNI merged 5 commits intoni:masterfrom
eli-engelhardt:fix_nilrt_verify_fail
Mar 6, 2025
Merged

Ensure proper auditd/logging permissions following reboot#60
AlexHearnNI merged 5 commits intoni:masterfrom
eli-engelhardt:fix_nilrt_verify_fail

Conversation

@eli-engelhardt
Copy link
Copy Markdown
Contributor

@eli-engelhardt eli-engelhardt commented Feb 27, 2025

Summary of Changes

  • Added permissions script to init.d in order to ensure permissions are set on symlink path
  • Moved initialization of audit conf file to ensure file exists
  • Added proper permissions for syslog-ng
  • Cleaned up unnecessary indentions in script files

Justification

Bug 3033025: nilrt-snac verify fails with error code 129. The verification steps that are failing after reboot:

 Stderr:
E           ( 2938) ERROR nilrt_snac.verify: MISSING: expected action_mail_acct value
E           ( 2939) ERROR nilrt_snac.verify: ERROR: /var/volatile/log is not owned by the 'adm' group.
E           ( 2939) ERROR nilrt_snac.verify: ERROR: /var/volatile/log does not have 770 permissions.
E           ( 2939) ERROR nilrt_snac.main: SNAC mode is not configured correctly.

Testing

Tested locally on clean reimaged VM.

Procedure

  • This PR: changes user-visible behavior, fixes a bug, or impacts the project's security profile; and so it includes a CHANGELOG note.
  • I certify that the contents of this pull request complies with the Developer Certificate of Origin.

Copy link
Copy Markdown
Collaborator

@amstewart amstewart left a comment

Choose a reason for hiding this comment

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

It's a little unclear to me what this commit is supposed to do. It seems like it's doing a few things, and the commit message isn't helpful.

Could you break this up into its component parts and make sure that all the necessary context about the change is in the commits? If part of this is a bug fix, the commit messages at least need to explain the bug and the fix.

- Added the `textwrap` module to clean up indentations in template scripts. This change improves the readability and consistency of the script formatting.

- Remove sudo group from ownership of a privileged file

Signed-off-by: Eli Engelhardt <eli.engelhardt@emerson.com>
Moved the initialization of `auditd_config_file` to occur after the potential installation of the auditd package. This change ensures that the auditd.conf is correctly populated. Previously, the file could be initialized as an empty string if it didn't exist, leading to the entire file being overwritten with an empty string.

Signed-off-by: Eli Engelhardt <eli.engelhardt@emerson.com>
Added a script `/etc/init.d/set_log_permissions.sh` to ensure that the permissions of the `/var/log` directory are correctly set and persist between system reboots. This change addresses the issue of incorrect permissions on the log directory after reboot.

Signed-off-by: Eli Engelhardt <eli.engelhardt@emerson.com>
The ownership for privileged files such as '/etc/syslog-ng/syslog-ng.conf' should stay as the default ownership of root:root.

Signed-off-by: Eli Engelhardt <eli.engelhardt@emerson.com>
@AlexHearnNI AlexHearnNI requested a review from a team March 3, 2025 16:46
Signed-off-by: Eli Engelhardt <eli.engelhardt@emerson.com>
@AlexHearnNI
Copy link
Copy Markdown
Collaborator

/azp run ni-nilrt-snac-pr

1 similar comment
@eli-engelhardt
Copy link
Copy Markdown
Contributor Author

/azp run ni-nilrt-snac-pr

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AlexHearnNI AlexHearnNI merged commit 712d10c into ni:master Mar 6, 2025
5 checks passed
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.

3 participants