[WIP] Adding allowed algorithms

[yannik1015]

I did a PoC implementation of how allowed algorithms could be added to the policies as mentioned in #251 As part of this I also changed the matching of the required algorithms from a complete string match such that it now checks each element/algorithm for existence. Maybe this is not wanted and the order and complete string matching as before is important.

Some questions still arise as what to do with the optional host keys field as in my opinion is now kinda useless.

Still missing:

• Tests
• What to do with the optional host keys field

[jtesta]

Thanks for putting the work into this PR!

I was just thinking about how to implement it, actually. I imagined adding an allow_algorithm_subset_and_reordering = true|false flag to control this behavior. When set to true, the policy evaluation will do something like:

for server_alg in server_algs:
    if server_alg not in allowed_algs:
       fail_audit()

Thus, if a policy states algorithms [alg1, alg2, alg3] all be included in that exact order, but the flag is set to true, then [alg2, alg1] would still pass. Is this the functionality that you had in mind?

The default should be false for all non-generic built-in policies, as well as false for new custom policies (this way, users won't be surprised by updates to the eval logic). Users can edit the custom policies they create and set it to true, if they so choose.

As for testing, I wouldn't mind handling this myself, but if you really wanted to give it a shot, there are the Tox and Docker tests to fill in (see https://github.com/jtesta/ssh-audit/blob/master/CONTRIBUTING.md).

An open question is whether one flag should control all categories (kex, ciphers, ...), or if one flag should control only one category (i.e.: kex_allow_algorithm_subset_and_reordering, etc).

[yannik1015]

Thus, if a policy states algorithms [alg1, alg2, alg3] all be included in that exact order, but the flag is set to true, then [alg2, alg1] would still pass. Is this the functionality that you had in mind?

Yeah that sounds like a sensible thing to do as it would not "break", i.e. change the behavior of existing policies. I think one flag for all categories should be enough.

[yannik1015]

I changed it now such that the allow_algorithm_subset_and_reordering now controls the behavior of the policies entries. If it is not set or set to False it will act like before but with it set to True we get the behavior of an allow list. Feel free to check and add it as you please. I am unsure if I have the time for implementing tests.

[jtesta]

@yannik1015 : thanks for working on this! After merging your PR, I checked in https://github.com/jtesta/ssh-audit/commit/3c31934ac7b5ab816bb31640cd34f57b1c2f9b00 to add tests and other cleanups.

I gave you partial credit in the change log (https://github.com/jtesta/ssh-audit/blob/3c31934ac7b5ab816bb31640cd34f57b1c2f9b00/README.md?plain=1#L193) and linked your Github username to your Github profile. If you'd rather use your real name, or would like the link to go to another personal site, please let me know!

Thanks again!