jtesta / ssh-audit

SSH server & client security auditing (banner, key exchange, encryption, mac, compression, compatibility, security, etc)
MIT License
3.4k stars 176 forks source link

ecdsa-sha2-nistp<256/384/521> #107

Open thecliguy opened 3 years ago

thecliguy commented 3 years ago

@jtesta , ssh-audit 2.4.0 returns the following for host key algorithms ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 and ecdsa-sha2-nistp521:

# host-key algorithms
(key) ecdsa-sha2-nistp521 -- [fail] using weak elliptic curves
                          `- [warn] using weak random number generator could reveal the key
                          `- [info] available since OpenSSH 5.7, Dropbear SSH 2013.62
(key) ecdsa-sha2-nistp384 -- [fail] using weak elliptic curves
                          `- [warn] using weak random number generator could reveal the key
                          `- [info] available since OpenSSH 5.7, Dropbear SSH 2013.62
(key) ecdsa-sha2-nistp256 -- [fail] using weak elliptic curves
                          `- [warn] using weak random number generator could reveal the key
                          `- [info] available since OpenSSH 5.7, Dropbear SSH 2013.62

Would it be possible to update the [fail] message to include a citation to a credible source that backs up the claim of using weak elliptic curves?

I've seen that you presented on the topic of Problems With Elliptic Curves In TLS and SSH at Rochester Security Summit (RSS) 2017.

Perhaps ssh-audit could cite your presentation?

jtesta commented 3 years ago

I'd like to do that, but that opens an entire project around putting in references to everything else as well... which I'm also open to doing. I suppose we'd have to come up with a strategy for this. Perhaps references can be put in as "info" notes for each algorithm? Hmm...

thecliguy commented 3 years ago

Can we not keep it simple and just add a reference directly against the note that it relates to?

So in the case of [fail] using weak elliptic curves, we'd update the DB value in ssh2_kexdb.py with something like:

WARN_CURVES_WEAK = 'using weak elliptic curves: https://www.rochestersecurity.org/wp-content/uploads/2017/10/RSS2017-T2-Testa.pdf'

If you add references as independent note values then you have to invent a way of associating them with the relevant note. That could get very complicated to maintain.

oam7575 commented 3 years ago

EDIT Gah . For anyone unfortunate enough to read my post before this edit. Forget I even mentioned it.

Not enough sleep and I am a goose.

jtesta commented 3 years ago

however ssh-audit 2.4.0 still gives warnings for these KEX algorithm's ( and a few other things ) .

I have restarted / rebooted and rebuilt configuration files to try and ensure that I haven't missed anything

In a new issue, can you post the exact output of ssh-audit, as well as the sshd configuration changes you made? That would be very helpful for debugging.

thecliguy commented 3 years ago

@jtesta oam7575 has now retracted the question, I guess the problem was down to user error?

Getting back on topic, do you have any thoughts about how to progress this? Do you still want to go as far as building a mechanism to handle references, or can we keep it simple and simply add a reference directly against the note it relates to?

jtesta commented 3 years ago

On Sat, 2021-05-29 at 06:36 -0700, thecliguy wrote:

Getting back on topic, do you have any thoughts about how to progress this? Do you still want to go as far as building a mechanism to handle references, or can we keep it simple and simply add a reference directly against the note it relates to?

I suppose we could put the references into comments. After all, that's the entire purpose of the comments anyway!

I'll import the references from ssh-audit.com, since I already have a database in the web app.

thecliguy commented 3 years ago

Just as an example, if you were to add a reference to ecdsa-sha2-nistp521 how will it be made clear to the user which of the three existing comments the reference relates to? Will the reference be appended to the relevant comment?

oam7575 commented 3 years ago

<---- PEBKAC

jtesta commented 3 years ago

How about this:

(key) ecdsa-sha2-nistp256                   -- [fail] using weak elliptic curves
                                            `- [warn] using weak random number generator could reveal the key
                                            `- [info] available since OpenSSH 5.7, Dropbear SSH 2013.62
                                            `- [info] reference: <https://reference.com/>
Keisial commented 3 years ago

I think it would make more sense to include it along the error:

(key) ecdsa-sha2-nistp256                   -- [fail] using weak elliptic curves -- <https://reference.com/>
                                            `- [warn] using weak random number generator could reveal the key
                                            `- [info] available since OpenSSH 5.7, Dropbear SSH 2013.62

As you could have multiple references:

(key) ssh-foo                               -- [fail] using weak hashing algorithm: <example.com/foo-considered-insecure>
                                            `- [info] available since OpenSSH 2.5.0, Dropbear SSH 0.28
                                            `- [info] deprecated in OpenSSH 42: https://www.openssh.com/txt/release-42

Although I guess you could use the same format for adding references in a new sublevel:

(key) ssh-foo                               -- [fail] using weak hashing algorithm:
                                             --  <example.com/foo-considered-insecure>
                                             `-  <example.com/foo-is-still-insecure>
                                            `- [info] available since OpenSSH 2.5.0, Dropbear SSH 0.28
                                            `- [info] deprecated in OpenSSH 42: https://www.openssh.com/txt/release-42
thecliguy commented 3 years ago

@Keisial Thank you. :+1: You've illustrated the point that I have been trying to make very well.

We must devise a way to clearly indicate to the user which comment a reference relates to.

I think we need to build a relationship between comments and references. So perhaps we need to turn comments into some sort of data structure, EG:

# Format: 'comment_name': [['comment_text'], ['reference_1', 'reference_2', ...']]
'WARN_CURVES_WEAK': [['using weak elliptic curves'], ['https://reference1.com', 'https://reference2.com']]
thecliguy commented 3 years ago

@jtesta - Hello Joe, any ideas how to progress this...?

Keisial and I appear to be of the same opinion which is that we need to make it clear to the user which comment a reference relates to, EG:

(key) ssh-foo           -- [fail] using weak hashing algorithm:
                         --  <https://example.com/foo-considered-insecure>
                         `-  <https://example.com/foo-is-still-insecure>

In order to achieve this I think we need to form a relationship between comments and references, EG:

# Format: 'comment_name': [['comment_text'], ['reference_1', 'reference_2', ...']]
'FAIL_HASH_WEAK': [['using weak hashing algorithm'], ['https://example.com/foo-considered-insecure', 'https://example.com/foo-is-still-insecure']]

If ssh-audit were to store references in ssh2_kexdb.py using a structure as suggested above then I suppose that ssh-audit.com won't need to read the values from a database any more. Therefore I'm assuming that you'd just need to do a one-off export of the values from the database into ssh2_kexdb.py .

What do you think about this?

jtesta commented 3 years ago

What do you think about this?

Yep, that looks good.

When verbose mode is enabled, the JSON output should include the references as well.

halfluke commented 7 months ago

Sorry for reviving this old issue, but can you please shed a light on which portion of the code marks these ecdsa-sha2-nistp as a FAIL? As in the code I can only find references to WARN:

src/ssh_audit/ssh2_kexdb.py:            'ecdh-sha2-nistp521': [['5.7,d2013.62'], [WARN_CURVES_WEAK]],
src/ssh_audit/ssh2_kexdb.py:            'ecdsa-sha2-nistp521': [['5.7,d2013.62,l10.6.4'], [WARN_CURVES_WEAK], [WARN_RNDSIG_KEY]],
src/ssh_audit/ssh2_kexdb.py:            'ecdsa-sha2-nistp521-cert-v01@openssh.com': [['5.7'], [WARN_CURVES_WEAK], [WARN_RNDSIG_KEY]],

Although I can also see: test/docker/expected_results/openssh_8.0p1_test1.txt:(kex) ecdh-sha2-nistp521 -- [fail] using weak elliptic curves But I do not understand how the 'test' directory is used

Thank you

jtesta commented 7 months ago

It appears you're looking at old source code. In the current master branch, the WARN_CURVES_WEAK tag was renamed to FAIL_NSA_BACKDOORED_CURVE at some point in the past (see https://github.com/jtesta/ssh-audit/blob/master/src/ssh_audit/ssh2_kexdb.py#L153 ).

To answer your question about the database format, the second list of strings for an algorithm denotes the failures. (The first list describes when the algorithm was implemented. The third list holds warnings. The fourth list holds notes.) A summary is found in https://github.com/jtesta/ssh-audit/blob/master/src/ssh_audit/ssh2_kexdb.py#L83 .

The test/ directory holds unit tests run by pytest (via Tox; at the top level of the source code, run: pip3 install -U tox; python3 -m tox). It also holds the configuration for the Docker tests (run with: ./docker_test.sh).

halfluke commented 7 months ago

thank you... yes I was checking an old vesion on my machine, from 2022.

The mystery is getting worse though. I checked the 2022 version after using the python2 version of ssh-audit, the one from 8 years ago, on a machine that only had python2 installed (and no chance to update, not even connection to the internet). The result from the old ssh-audit is exactly [fail] using weak elliptic curves In that case I still don't understand how it is possible because there is a single ssh-audit.py file with everything in it, and there is definitely WARN_CURVES_WEAK, and still that is assigned a [fail] I must miss something in the code, but of course we are no longer talking about your code here

halfluke commented 7 months ago

If I had read more carefully your replied I would have spared myself from several hours of NOOB (like I am) troubleshooting. The problem with the arthepsy code (and maybe with your code before the change from WARN to FAIL), was that a [ ] was missing in position 2 in the "db":

'ecdh-sha2-nistp256': [['5.7,d2013.62,l10.6.0'], [WARN_CURVES_WEAK]],
            'ecdh-sha2-nistp384': [['5.7,d2013.62'], [WARN_CURVES_WEAK]],
            'ecdh-sha2-nistp521': [['5.7,d2013.62'], [WARN_CURVES_WEAK]],

That's why WARN_CURVES_WEAK was reported as a 'fail'.