print config v2 Issue #191 #307

oam7575 · 2024-11-24T00:27:32Z

@jtesta @daniejstriata

printconfig script
test_printconfig for tox testing
update globals for GUIDES_UPDATED date value
update ssh_audit for print_config argument and checks

Quotes should no longer be needed.
usage : ssh-audit.py --print-config Ubuntu 2204 Server

- printconfig script - test_printconfig for tox testing - update globals for GUIDES_UPDATED date value - update ssh_audit for print_config argument and checks

jtesta

Thanks for submitting this PR! Here's some feedback on this draft.

jtesta · 2024-11-25T18:07:19Z

src/ssh_audit/printconfig.py

+
+from ssh_audit import exitcodes
+from ssh_audit.globals import VERSION
+from ssh_audit.globals import GUIDES_UPDATED


I think each platform should track an independent last-updated field. Just like the guides on the website, sometimes one platform needs an update, but the others don't.

Also, it would be a good idea to track a change log per platform (also like the online guides).

Perhaps the guides can be stored as a dictionary with "last_update", "change_log", and "instructions" fields. That would organize it in a cleaner way in the code, I think.

As an example of what I mean by using a dictionary, see here:

ssh-audit/src/ssh_audit/builtin_policies.py

Line 27 in a01baad

BUILTIN_POLICIES: Dict[str, Dict[str, Union[Optional[str], Optional[List[str]], bool, Dict[str, Any]]]] = {

jtesta · 2024-11-25T18:08:25Z

src/ssh_audit/printconfig.py

+from ssh_audit.globals import GUIDES_UPDATED
+
+
+class PrintConfig:


Perhaps this class could be called HardeningGuides.

src/ssh_audit/printconfig.py

jtesta · 2024-11-25T18:10:50Z

src/ssh_audit/printconfig.py

+            sys.exit(retval)
+        else:
+            print(" ")
+            print(f"\033[1mSSH-Audit Version : {VERSION}\033[0m")


If the class constructor accepts an OutputBuffer, then we can print using its good() method to handle colors for us.

Also, in the code, ssh-audit always refers to itself as "ssh-audit" (in lowercase).

src/ssh_audit/printconfig.py

jtesta · 2024-11-25T18:15:08Z

src/ssh_audit/ssh_audit.py

@@ -816,7 +817,10 @@ def process_commandline(out: OutputBuffer, args: List[str]) -> 'AuditConf':  # p
    parser.add_argument("--skip-rate-test", action="store_true", dest="skip_rate_test", default=False, help="skip the connection rate test during standard audits (used to safely infer whether the DHEat attack is viable)")
    parser.add_argument("--threads", action="store", dest="threads", metavar="N", type=int, default=32, help="number of threads to use when scanning multiple targets (-T/--targets) (default: %(default)s)")

-    # The mandatory target option.  Or rather, mandatory when -L, -T, or --lookup are not used.
+    # Print Suggested Configurations from : https://www.ssh-audit.com/hardening_guides.html
+    parser.add_argument("--print-config", nargs="*", action="append", metavar="OS Ver Client/Server", dest="print_configuration", type=str, default=None, help="print suggested server or client configurations. Usage Example : Ubuntu 2404 Server")


Perhaps this option could be called --get-hardening-guide?

Also, the user would want to see what hardening guides are available, so perhaps the supported arguments to --get-hardening-guide can be listed with --list-hardening-guides.

if the argument is called with out any further data ( server / version / edition ) or incorrectly formatted data it will print out the supported configurations.

With that said I have updated the configuration to print an example.

Would this be acceptable ?

python3 ssh-audit.py --get-hardening-guides ssh-audit Version : v3.4.0-dev Guides Last modified : 2024-10-01 Error unknown varient : OS Version Edition For current, community developed and legacy guides check the website : https://www.ssh-audit.com/hardening_guides.html Supported Server Configurations : Amazon 2023 Server Debian Bookworm Server Debian Bullseye Server Rocky 9 Server Ubuntu 2404 Server Ubuntu 2204 Server Ubuntu 2004 Server Supported Client Configurations : Amazon 2023 Client Debian Bookworm Client Mint 22 Client Mint 21 Client Mint 20 Client Rocky 9 Client Ubuntu 2404 Client Ubuntu 2204 Client Ubuntu 2004 Client Example Usage : python3 ssh-audit.py --get-hardening-guides Ubuntu 2404 Server

if the argument is called with out any further data ( server / version / edition ) or incorrectly formatted data it will print out the supported configurations.

Thinking from a user's perspective, usability would be better if we presented an obvious --list-hardening-guides option.

Would this be acceptable ?

Sure, that would work. My only feedback would be that having one last-modified field for all guides wouldn't be helpful to users; each platform should have its own history (date, integer version number, change log message, and hardening instructions).

Perhaps the current -L and -L -v behavior could serve as a model here. Notice how ssh-audit -L only shows the latest version of policies, but ssh-audit -L -v lists previous versions of the policies as well, including their change log messages. It would be nice if --list-hardening-guides did the same thing.

jtesta · 2024-11-25T18:17:24Z

test/test_printconfig.py

+
+
+# pylint: disable=attribute-defined-outside-init
+class TestAuditConf:


I suppose this class name should be updated to TestHardeningGuides.

oam7575 · 2024-11-26T06:36:13Z

Hi Joe,

As always, thanks for the review and feedback.
I'll look into the changes and get back to you.

Again I will try and update and attempt to squash everything, and again if it turns into a mess - I'll close this one of and create a clean PR when the code is a bit closer to a second review.

Cheers.

jtesta · 2024-11-26T22:54:19Z

I will try and update and attempt to squash everything, and again if it turns into a mess - I'll close this one of and create a clean PR

One way to do it is to make a working branch for your next revision, add as many commits as you'd like to that, then simply make one squash merge commit back into print_guide_v2.

For example:

$ git checkout print_guide_v2  # Start with the branch that corresponds to this PR
$ git checkout -b pr307_revision_2  # Copy this branch, and use it for experimentation
$ git add [...]; git commit [...]  # Make as many commits as you'd like to this pr307_revision_2 branch
$ git checkout print_guide_v2; git merge --squash pr307_revision_2 branch  # Squash all commits from the experimental branch into one commit in print_guide_v2.

Once you push the one squashed commit in print_guide_v2, this PR will be automatically updated with that one commit. Hope this helps!

Missed a TAB

oam7575 · 2024-12-23T07:10:55Z

Hi Joe,

As you will have seen I have made a few changes and pushed everything up again.

As you will have also seen - I still didn't get the squash merge correct. I can only apologies for that.

With that said; happy for another round of feed back when you have time to review after the festive season.

If this needs further work I will create a clean branch and try again.

print config v2

9814d18

- printconfig script - test_printconfig for tox testing - update globals for GUIDES_UPDATED date value - update ssh_audit for print_config argument and checks

oam7575 mentioned this pull request Nov 24, 2024

could ssh-audit print desired config or add Generic guide documentation #191

Open

jtesta requested changes Nov 25, 2024

View reviewed changes

oam7575 added 5 commits December 23, 2024 16:11

pr307 update 1

48b9ee7

pr307 update 2

231a479

pr307 - attempt 2

6b8b20a

Merge branch 'pr307_revision_2' into print_guide_v2

f15e53e

Update ssh_audit.py

62d41ea

Missed a TAB

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

print config v2 Issue #191 #307

print config v2 Issue #191 #307

oam7575 commented Nov 24, 2024

jtesta left a comment

jtesta Nov 25, 2024

jtesta Nov 25, 2024

jtesta Nov 25, 2024

jtesta Nov 25, 2024

jtesta Nov 25, 2024

oam7575 Nov 26, 2024

jtesta Nov 26, 2024

jtesta Nov 25, 2024

oam7575 commented Nov 26, 2024

jtesta commented Nov 26, 2024

oam7575 commented Dec 23, 2024

		from ssh_audit.globals import GUIDES_UPDATED


		class PrintConfig:



		# pylint: disable=attribute-defined-outside-init
		class TestAuditConf:

print config v2 Issue #191 #307

Are you sure you want to change the base?

print config v2 Issue #191 #307

Conversation

oam7575 commented Nov 24, 2024

jtesta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oam7575 commented Nov 26, 2024

jtesta commented Nov 26, 2024

oam7575 commented Dec 23, 2024