Dconf improvements#93
Conversation
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
Signed-off-by: Will <will@dower.dev>
* update chrony ctrl and inspec.yml * remove explicit space char in split call Was making the linter sad --------- Co-authored-by: wdower <57142072+wdower@users.noreply.github.com>
* Update inspec.yml * Update SV-258131.rb * cookstyle --------- Co-authored-by: Jon Metzger <jmetzger@mitre.org>
taken from rhel8
…sed (presumably due to it having been duplicated as the system_inactivity_timeout which I have subsequently changed, added an input called screensaver_lock_delay to be used in 258025, fixed a bug in 258023, implemented 258025 Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
…o be checking for the value of the key and then i also added an input so that you could change the picture to being an actual picture instead of just blank if you wanted to Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
…ment due to there only being the one pool directive Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
There was a problem hiding this comment.
Discuss if we should add a matching input to gui_automount_required called gui_automount_writable_required similar to the guidance in 258017 for making an ISSO exception
There was a problem hiding this comment.
Agreement to add flag
There was a problem hiding this comment.
Discuss if we should add a requirement for locking this key even though there isn't such a requirement in the guidance despite most near all of the other security related keys getting locked.
There was a problem hiding this comment.
Submit a bug to say that they forgot the locked requirement, but in the meantime we should add the locked? checks to the test
There was a problem hiding this comment.
Discuss how this one is not locked either.
There was a problem hiding this comment.
Same as before - bug report + locked
|
I'll post/explain the gui logic stuff tomorrow cause it was not straightforward even for me haha. I'll also fix the linting then. For testing purposes, I ran it against a Fedora 38 vm (so had to make runtime mods to get rid of the supports declarations). See: https://heimdall-demo.mitre.org/compare/2159,2109,2160. The file marked as 'hdf.json' is the run against the vm. |
|
Forgot to do it last night: comparison between the original profile and my improvements: https://heimdall-demo.mitre.org/compare/2160,2161 |
…an treat them as blocks Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
|
Ok, here we go: https://heimdall-demo.mitre.org/compare/2163,2109,2165,2161 - hdf3 is the one with the bug fixes from above |
Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
Signed-off-by: Amndeep Singh Mann <amann@mitre.org>
|
The linter didn't change anything but in case you wanted to confirm: https://heimdall-demo.mitre.org/results/2165,2168 |
|
Review from Will + Aaron
Future iterations
|
… controls This commit implements comprehensive GUI resources following InSpec best practices and transforms 25 STIG controls from complex nested logic to clean declarative testing. New Resources: - gui: Cross-platform GUI detection with inheritance pattern - gnome_settings: Schema-based GNOME settings with 5 interface patterns - dconf: Policy validation and administrative lock management Key Improvements: - 90% code reduction across GUI controls - Natural language testing with proper grammar - Ruby best practices throughout (guard clauses, memoization, keyword args) - Professional InSpec patterns replacing hacky nested conditionals - Comprehensive error handling and validation Controls Transformed (25 total): - SV-258012-SV-258033: GUI controls with dramatic simplification - SV-257945: Fixed chrony pool array handling bug - SV-258068: Improved shell timeout validation Input Improvements: - Added gui_session_timeout (900s) for graphical sessions - Added shell_session_timeout (600s) for command line sessions - Replaced overly verbose input names with clear, concise alternatives This approach achieves all PR #93 goals while following established InSpec and Ruby best practices, resulting in maintainable, professional code. Authored by: Aaron Lippold<lippold@gmail.com>
… controls This commit implements comprehensive GUI resources following InSpec best practices and transforms 25 STIG controls from complex nested logic to clean declarative testing. New Resources: - gui: Cross-platform GUI detection with inheritance pattern - gnome_settings: Schema-based GNOME settings with 5 interface patterns - dconf: Policy validation and administrative lock management Key Improvements: - 90% code reduction across GUI controls - Natural language testing with proper grammar - Ruby best practices throughout (guard clauses, memoization, keyword args) - Professional InSpec patterns replacing hacky nested conditionals - Comprehensive error handling and validation Controls Transformed (25 total): - SV-258012-SV-258033: GUI controls with dramatic simplification - SV-257945: Fixed chrony pool array handling bug - SV-258068: Improved shell timeout validation Input Improvements: - Added gui_session_timeout (900s) for graphical sessions - Added shell_session_timeout (600s) for command line sessions - Replaced overly verbose input names with clear, concise alternatives This approach achieves all PR #93 goals while following established InSpec and Ruby best practices, resulting in maintainable, professional code. Authored by: Aaron Lippold<lippold@gmail.com>
There was a problem hiding this comment.
Add another envvar to list desktops packages that might be false positives. Look into xorg-x11-xinit-session to see if it is actually a full gui or what.
There was a problem hiding this comment.
The xorg-x11-xinit-session package might not be installed even when xorg-x11-xinit is.
Goes and tries to run the first session it finds in ~/.xession or ~/.Xclients. If neither is available, then it goes to /etc/X11/xinit/XClients which is a script that tries to start up one of the default desktops (gnome, mate, kde, lxde). If none of those exist, then it will try to start one of the scripts in /etc/X11/xinit/Xclients.d. If that directory is empty, then as a final fallback it will try to start a few applications if they're installed (xclock, xterm, firefox) and a window (not desktop!) manager (twm).
After discussion with Aaron, there are valid reasons to have an xserver installed on a machine that would not result in a full desktop experience. A little bit more discussion is required to determine if "any graphical user interface" also includes that final fallback.
However, discussion with Aaron and some further research is making me think we haven't actually fully resolved the problem. Some of the requirements are for behavior on a system that you've already logged into (ex. the automount requirements) which the way we have implemented is fine (since that behavior is often handled by the desktop environment or systemd). However, the various requirements for the banner are for it to be shown before or as you are logging into the gui. So what actually matters is what desktop manager you have installed and if you have configured it correctly since that is what is going to be showing that banner to you. Yes, you can start a desktop environment after having logged into the machine without a gui but that means that you will then have had to satisfy the other requirements about showing the banner when trying to log in that way. Consequently, the particular desktop environment/gui you have doesn't matter, it's how do you get access to that environment which is by the desktop manager. This means that some of the controls (esp the banner ones) will need to be tweaked to remove the "which DE" logic and instead have "which DM" logic.
There was a problem hiding this comment.
this one is somehow allowing passes to go through even though it is doing the skipped for manual review. the fail is for the directory existing, but the passes are for all the key and lockfiles being updated, and then lastly there's a skip for the manual review.
thinking: is it allowing the passes to go through because of the one fail and then once that fail is resolved it would go back to being strictly the manual review skip?
There was a problem hiding this comment.
the only if does a check for gdm and cancels the requirement if it is not there, but that sidesteps the gui check stuff we have in the rest of the codebase and is also just wrong because the requirement still applies even if not gdm
5 participants
No description provided.