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

Setup skeleton audit log #310

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Conversation

corbin-phipps
Copy link
Contributor

Type

  • Bug fix
  • Feature addition
  • Feature update
  • Documentation
  • Build Infrastructure

Side Effects

  • Breaking change
  • Non-functional change

Goals

This PR sets up the foundation for the Wi-Fi state audit log.

Technical Details

  • Added a PLOG RollingFileAppender separate from the existing logging to be used for writing to the audit log file.
  • Added audit log macros in the form of AUDIT_LOG*.
  • Ensured that log files are actually present by specifying the path to /usr/local/.

Test Results

Verified that the AUDIT_LOGN line added to Main shows up in /usr/local/202407-LogNetRemote-audit.txt.

Reviewer Focus

I'm not sure if /usr/local is the best place, but omitting that and just using what is returned by GetLogName doesn't seem to create a log file anywhere.

Future Work

Write Wi-Fi state and events to the audit log.

Checklist

  • Build target all compiles cleanly.
  • clang-format and clang-tidy deltas produced no new output.
  • Newly added functions include doxygen-style comment block.

@corbin-phipps corbin-phipps requested a review from a team as a code owner July 11, 2024 01:46
Copy link
Contributor

@abeltrano abeltrano left a comment

Choose a reason for hiding this comment

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

I'm not sure if /usr/local is the best place, but omitting that and just using what is returned by GetLogName doesn't seem to create a log file anywhere.

The log file as currently used, will be created in the current working directory of the binary. This is configurable using the WorkingDirectory= configuration directive in [Service] section of the netremote-server systemd unit file. I'd guess the binary doesn't have permission to write to that directory (/usr/bin or /usr/local/bin) since the service is started as a non-root user. So, the fix should be to set WorkingDirectory to something a regular user can access.

src/common/shared/logging/include/logging/LogUtils.hxx Outdated Show resolved Hide resolved
src/linux/server/Main.cxx Outdated Show resolved Hide resolved
Copy link
Contributor

@abeltrano abeltrano left a comment

Choose a reason for hiding this comment

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

I'm not sure if /usr/local is the best place, but omitting that and just using what is returned by GetLogName doesn't seem to create a log file anywhere.

The log file as currently used, will be created in the current working directory of the binary. This is configurable using the WorkingDirectory= configuration directive in [Service] section of the netremote-server systemd unit file. I'd guess the binary doesn't have permission to write to that directory (/usr/bin or /usr/local/bin) since the service is started as a non-root user. So, the fix should be to set WorkingDirectory to something a regular user can access, like /var/log.

@abeltrano abeltrano merged commit a0c7540 into develop Jul 15, 2024
1 of 4 checks passed
@abeltrano abeltrano deleted the user/corbinphipps/setup-audit-log branch July 15, 2024 22:35
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