New version of Radius2john.py

[k4amos]

radius2john.py didn't seem to be able to work properly :

• The attack method is described here. The previous version of radius2john.py implemented the 3.1 Response Authenticator Based Shared Secret Attack. The attacker can pre-compute the MD5 state for (Code+ID+Length+RequestAuth+Attributes). However, the value of the attributes was forgotten in the code, the calculated hash could not be broken if there were attributes.
• The 3.3 User-Password Attribute Based Shared Secret Attack was not implemented

For these two reasons, I propose a new version of radius2john.py much closer to the perl version radius2john.pl

[solardiz]

Thank you @k4amos! For reference, the previous version was contributed via #3621.

I'd appreciate review by @MaximeGoyette @exploide @kholia.

Is this revised script for Python 3 and/or 2? The current shebang lines implies it supports both, which is great if so, but I'm not sure is the case.

[k4amos]

Hello @solardiz,

I added the python3 shebang and a try / except block to display a message if scapy is not installed.

RADIUS accounting messages are on port 1813, I had forgotten to consider them, but I corrected that. The shared-secret can be cracked using these methods, as demonstrated by running the code on the first sample provided by Maxime Goyette https://github.com/openwall/john/pull/3621#issuecomment-458752330

I realise that the perl version has also forgotten to consider accounting messages, do I create an issue for the perl version ?

[solardiz]

I added the python3 shebang

In other words, you're saying the script is now Python 3 only? Was the previous one working with either Python version? If so, that's a minor regression (but not a blocker).

I realise that the perl version has also forgotten to consider accounting messages, do I create an issue for the perl version ?

Yes, or ideally a pull request right away. Thank you!

[solardiz]

Nothing in the john-samples repo unfortunately.

BTW, @k4amos @MaximeGoyette we welcome contributions via pull requests to https://github.com/openwall/john-samples so that it's easier and more reliable to regression-test future versions.

[k4amos]

Hi @solardiz and @solardiz, I hope I've corrected everything correctly. With the samples, you can test the code : the previous version of the code only worked with the first 2 samples, whereas this new version works with all 5 :

previous version of the code :

new version of the code : (by default, I no longer return a single hash)

You can test the 3.3 attack for example with the sample n°5 :

[k4amos]

The previous version of the code seems to work well with both python2 and python3 @solardiz , this is a regression on this point, I have no solution to keep this compatibility 🫤

[solardiz]

Hi @solardiz and @solardiz, I hope I've corrected everything correctly.

Hi @k4amos and @k4amos. Thank you. For fix-up commits within a PR, we prefer that the original commit be amended and force-pushed, instead of adding more commits. Also, the commit titles should start with radius2john.py:, although if the only commit is New version of radius2john.py that is also fine.

In this case, you may squash the commits, or you may just tell me to use GitHub's squash-and-merge (which we normally have disabled to avoid inadvertent use, but I can briefly re-enable it for merging this one PR).

I haven't reviewed your actual changes. I hope @exploide will.

Also thank you for the sample files and for the test examples. It would be even better if you post such examples as plain text copy-paste, rather than as screenshots, so they would also be in email.

[solardiz]

The previous version of the code seems to work well with both python2 and python3 @solardiz , this is a regression on this point, I have no solution to keep this compatibility 🫤

OK. Does the Perl script provide the same functionality? If so, I suppose it would be the one to use on older systems that don't have Python 3 installed.

Edit: Oh, I suppose this (from an earlier comment) implies the Perl version isn't yet as functional:

I realise that the perl version has also forgotten to consider accounting messages

[k4amos]

If it's possible to make a GitHub's squash-and-merge @solardiz instead of editing the commits already made, that would be perfect 😃

[solardiz]

If it's possible to make a GitHub's squash-and-merge @solardiz instead of editing the commits already made, that would be perfect 😃

OK, I'll plan on doing that. Does this look ready, @exploide?

[solardiz]

This is now merged. However, I made the mistake of not letting CI run on this before merging - my thinking was we don't test Python scripts anyway - but we do have the whitespace errors test for all text files, and this test failed. I've now made a follow-up commit fixing the whitespace errors.

@k4amos Please take a look at how this looks in the tree now. Thank you!