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

Structural Refactor #87

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link
Contributor

@stevefan1999-personal stevefan1999-personal commented Sep 27, 2024

This PR revamps the whole structure of the crate, making it more modular and reducing code spaghetti and feature implications. It's a significant overhaul that addresses several long-standing issues and paves the way for future improvements.

One of the key benefits of this restructuring is that it makes flexible feature inclusion possible in the future. For example, you can choose to only include AES crypto suites or remove NIST curves to customize it to your needs. This level of customization is very useful for resource-constrained systems such as embedded devices that can't afford large code sizes.

The modular approach also improves maintainability and readability of the codebase. By breaking down the monolithic structure into smaller, more focused modules, we've made it easier for developers to understand and work on specific parts of the crate without getting lost in unnecessary complexity.

Additionally, this restructuring sets the stage for easier integration of new features and optimizations in the future. The cleaner architecture will allow us to add or modify functionality with minimal impact on existing code, reducing the risk of introducing bugs or breaking changes.

Also from Zulip:

This is a big one, so I'm here to announce that rustls-rustcrypto is near ready for general use case

The biggest point of the refactor is that we finally have a rustls provider where we can choose the TLS suites we wanted based on the combinations, yours truly and natively in Rust. So from the top to bottom, you need almost 0% C/C++ code and get a fully working TLS client without much quirks, in constrast to ring/aws-lc.

All the existing CI/CD and testing pipelines retrofitted to the changes and passed right away, including a new test case (test_basic_round_trip) that produces a cartesian product of all crypto suites and key exchange method, one by one at each point and all tested multithreaded and doing in-memory data roundtrip without relying on system socket allocation, so the test is not only fast, but the code coverage should drastically improve.

This should also be the only Rustls provider in the wild that works sufficiently well with embedded system and MCUs out of the box without having to deal with WolfSSL and mbedTLS where both of which are really hellish to use back when 6 years ago for me (although at the moment, I just need it for low density secure wireless communication for some Home Assistant sensor data, so efficiency and high throughput is not an issue yet).

While it is by no mean entirely production ready given the harsh environment it was deployed in, but I think it is prime time to get it merged and release 0.1.0 to crates.io as well.

@stevefan1999-personal
Copy link
Contributor Author

stevefan1999-personal commented Sep 27, 2024

After testing it through a private functional test, I would say the effort to make this refactor really pays off. It's been a game-changer for my development process. Before that, I had to include every suite possible, which was a real headache and made the codebase unnecessarily bloated. Now, I can just choose whatever crypto suite I want, say for my ESP32 code now. It's given me so much more flexibility and control over my projects.

While I didn't get a chance to run a real test on the ESP32 hardware itself (which is definitely on my to-do list), I used the following setting in my Windows binary test to simulate the environment:

rustls-rustcrypto = { version = "0.0.2-alpha", default-features = false, features = [
    "alloc",
    "chacha20poly1305",
    "kx-p256",
    "ecdsa-p256",
    "pkcs8"
] }

This is a bare-minimum dual TLS1.3/TLS1.2 compliant setting, using only TLS13_CHACHA20_POLY1305_SHA256 and TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (for TLS1.2). But if I ran only TLS1.3, it would shave off about 300KB from the binary, down from 1MB to 713KB (30% binary size saved!). That's a scary amount. It's wild to think about how much space these security protocols take up.

This approach allowed me to get a good feel for how the refactored code would perform in a more constrained environment like the ESP32. The results were promising, showing improved efficiency and reduced overhead. It's really exciting to see how this change could potentially optimize performance on embedded systems.

I mean, 300KB might not seem like a lot in today's world of gigabyte-sized apps, but when you're working with embedded systems or trying to optimize for performance, every kilobyte counts. And let's be real, the fact that dropping down to only using TLS1.3 and just one suite saves that much space is kind of mind-blowing. It really makes you wonder about the trade-offs between security and efficiency, especially in resource-constrained environments. I guess that's the price we pay for keeping our data safe in transit, right?

PS: The environment is simulated using Rustls unbuffered API, which is for running in an embedded, no_std but alloc available environment. Together with this provider being no_std friendly, this is probably the first publicly-maintained provider that could run on MCUs officially.

@tarcieri
Copy link
Member

@stevefan1999-personal it looks like there are conflicts with master that need to be resolved

@stevefan1999-personal
Copy link
Contributor Author

@tarcieri the problem is that I re-sorted the entries, so it is not in symphony deliberately...maybe this could be another PR

@stevefan1999-personal
Copy link
Contributor Author

stevefan1999-personal commented Dec 7, 2024

@tarcieri Maybe merge it soon? btw a new challenger approaches: https://github.com/wolfSSL/rustls-wolfcrypt-provider

(also give me the invite for the maintainer status again i didnt receive the invite as i was told in zulip)

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.

2 participants