Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extmod/modussl_axtls: Implement key and cert kw args to wrap_socket. #3398

Closed
wants to merge 1 commit into from

Conversation

dpgeorge
Copy link
Member

This adds key/cert support to ussl (axtls implementation, mbedtls already has it). The key and cert must both be a str/bytes object in DER format (PEM format is not supported, but could be if additional options were enabled in axtls).

One modification that could be done to this PR would be to allow any object with the buffer protocol to be passed for key/cert (not just str/bytes), which, for example, would allow to use a bytearray and file.readinto() instead of file.read() when loading the data. But doing it that way would also require updating the mbedtls version to have the same behaviour, so for now it accepts just str/bytes.

Total code size increase is +247 bytes on unix x86-64, and +180 bytes on esp8266.

This PR was tested on unix and esp8266 with the MQTT endpoint of AWS IoT via the following code:

from umqtt.simple import MQTTClient

with open('my_thing.private.key.der') as f:
    key_data = f.read()

with open('my_thing.cert.pem.der') as f:
    cert_data = f.read()

client = MQTTClient('my_thing', 'endpoint.iot.region.amazonaws.com', ssl=True, ssl_params={'key': key_data, 'cert': cert_data})
client.connect()
client.publish('hello', 'from my_thing')
client.disconnect()

Note that esp8266 running at 160MHz takes 25 seconds to do the client.connect(), due to the large amount of processing needed for the key.

The key and cert must both be a str/bytes object in DER format.
@pfalcon
Copy link
Contributor

pfalcon commented Oct 30, 2017

This is cool, but the same comment as for mbedtls implementation - this should be optional. The original usecase for adding SSL support to MicroPython was to workaround sites which force SSL upon users, and it remains valid usecase.

Note that esp8266 running at 160MHz takes 25 seconds to do the client.connect(), due to the large amount of processing needed for the key.

Yeah, we could enable chinese reminder theorem or similar optimizations in axTLS, which were conservatively disabled to save code size (just for esp8266, unix should have enough power to crunch numbers ;-)).

@dpgeorge
Copy link
Member Author

This is cool, but the same comment as for mbedtls implementation - this should be optional.

What do you mean? The mbedtls ussl has key and cert as basic features, always available. And key/cert is a basic feature so should be there for everyone to use. axtls already has default key/cert enabled so makes sense to be able to dynamically configure them. (Note that the AWS MQTT interface requires key/cert as part of the connection, why do we want to prevent users from using that?)

The original usecase for adding SSL support to MicroPython was to workaround sites which force SSL upon users, and it remains valid usecase.

These days ssl is a big part of the Internet, it's no longer just a nice-to-have.

@pfalcon
Copy link
Contributor

pfalcon commented Oct 31, 2017

What do you mean? The mbedtls ussl has key and cert as basic features, always available.

Then this is a good time to consistently add features to disable them.

axtls already has default key/cert enabled so makes sense to be able to dynamically configure them.

No objections here.

(Note that the AWS MQTT interface requires key/cert as part of the connection, why do we want to prevent users from using that?)

The question should be put differently: Should AWS MQTT existence prevent users of low-code-storage devices prevent usage of TLS at all, because AWS featureset doesn't fit into their code space?

These days ssl is a big part of the Internet, it's no longer just a nice-to-have.

Exactly, so bare minimum should be maintained to all as many devices to use it as possible, in addition to all "on the top" functionality, that's the whole matter of the talk.

@dpgeorge
Copy link
Member Author

This was merged in e511f24.

Any further fine-grained options to disable features like this can be discussed and incorporated when there's a pressing need.

@dpgeorge dpgeorge closed this Nov 24, 2017
@dpgeorge dpgeorge deleted the extmod-axtls-key-cert branch November 24, 2017 04:59
@noodlemind
Copy link

SSL Connection still gives the OSError[Errno 12] ENOMEM by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants