Opened 14 months ago

Last modified 11 months ago

#9118 enhancement new

A public API to use alternative KEX ciphers.

Reported by: the0id Owned by: the0id
Priority: normal Milestone:
Component: conch Keywords: review
Cc: Branch:
Author:

Description (last modified by the0id)

This ticket will add an API that will allow a developer to use alternative ciphers (such as ecdsa-sha2-nistt571).

There are two new methods and a wrapper for each.

transport.addSupportedKexes(kexes) takes a list of key exchange names and will add that name and it's corresponding hash algorithm to _kex.kexAlgorithms.

transport.addSupportedPublicKeys(publicKeys) takes a list of public key algorithms and adds it to supportedPublicKeys.

transport.setSupportedKexes(kexes) and transport.setSupportedPublicKeys(publicKeys) both clear out the respective lists and repopulates them with the list that is passed in.

It should be no surprise that the tests are not finished yet because I want to make sure this approach is acceptable before spending time on the tests. :)

PR https://github.com/twisted/twisted/pull/768

Change History (7)

comment:1 Changed 14 months ago by the0id

Description: modified (diff)
Keywords: review added

comment:2 Changed 13 months ago by Adi Roiban

Keywords: review removed
Owner: set to the0id

Thanks for working on this.


From what I can see in the PR, this also adds a couple of new curves ex (ec.SECT163K1)

The title of this ticket is A public API to use alternative KEX ciphers.... I am not sure if I get what you wanted to express with "alternative"

My initial thought was that I will allow me to plug my own custom (non-standard) KEX

But from what I can see in this code, it adds a couple of extra curves and add the API required to select what KEX algorimts to use, fro


In order to better review this and see if the proposed API is sound it would help to understand why you would want this.

At least 3 real world examples should help to validate the API.

Why not add ecdsa-sha2-nistt571 directly to twisted.conch ?

I am not saying that you should add ecdsa-sha2-nistt571 directly to twisted.conch, but this is one question from which I try to find those real world examples.


The PR also lack docstring for the new public API methods so I find it hard to see what is their purpose and how we should expect them to be used.


Writing test is a good way to document how you would expect this API to be used and under which use cases.

Please provide at least one use case , use case scenario for this API.

What problem it tries to solve and why the current API is not suitable for them?

Thanks!

comment:3 in reply to:  2 Changed 13 months ago by the0id

Replying to Adi Roiban:

Thanks for working on this.


From what I can see in the PR, this also adds a couple of new curves ex (ec.SECT163K1)

The title of this ticket is A public API to use alternative KEX ciphers.... I am not sure if I get what you wanted to express with "alternative"

My initial thought was that I will allow me to plug my own custom (non-standard) KEX

But from what I can see in this code, it adds a couple of extra curves and add the API required to select what KEX algorimts to use, fro

"Alternative" just meant curves or suites that aren't included by default in conch; just easier to say. :)

I know its a little confusing to see it in the code, but the alternative curves like nistk163 aren't being added (unless specified in the API). I put those in the list just to be referenced if the API was used to include them.

I'm planning on changing the code a little so that instead of hard coding those curves into a list, they will be added dynamically when you add a curve using the API.


In order to better review this and see if the proposed API is sound it would help to understand why you would want this.

At least 3 real world examples should help to validate the API.

Having many different curves allows Twisted to be used in different situations.

One example would be that I have an environment where I'm dealing with very sensitive information and I feel I need a stronger curve like nistt571 to protect my data.

Or I have an environment where I need to transfer data, but it's not that sensitive. The nodes I'm using are small embedded systems that process lots of data. Here using a smaller curve such as nistk163 is what I need because all I care about is protecting data against some person just snooping on my network, and every CPU cycle counts.

On the mailing list Tobias mentioned that a German security institute (BSI, their equivalent to NIST it sounds like) is recommending using brainpool instead of the NIST curves. This API would allow for someone to add support for that suite.

Why not add ecdsa-sha2-nistt571 directly to twisted.conch ?

I am not saying that you should add ecdsa-sha2-nistt571 directly to twisted.conch, but this is one question from which I try to find those real world examples.

I did, but it was removed by a later merge. So as a compromise I'm adding this API. :)


The PR also lack docstring for the new public API methods so I find it hard to see what is their purpose and how we should expect them to be used.


Writing test is a good way to document how you would expect this API to be used and under which use cases.

Please provide at least one use case , use case scenario for this API.

What problem it tries to solve and why the current API is not suitable for them?

Thanks!

You're right about the tests and the docstring. I'll add those, but as much as I love writing unit tests I wanted to give this as a proposal first to see if this would be acceptable. :)

comment:4 Changed 13 months ago by the0id

Keywords: review added

comment:5 in reply to:  2 Changed 12 months ago by mark williams

Replying to Adi Roiban:

In order to better review this and see if the proposed API is sound it would help to understand why you would want this.

At least 3 real world examples should help to validate the API.

One reason is that #9019 removed support for diffie-hellman-group1-sha1, which is known to be breakable by attackers with lots of money at their disposal. It could not be disabled by default but left available, as it is in OpenSSH, because at that time conch had no way to for users to enable or disable ciphers. Some users require this cipher, though, and this ticket will allow them access to it while still ensuring conch's defaults respect best practices.

comment:7 Changed 11 months ago by the0id

Keywords: review added
Note: See TracTickets for help on using tickets.