-
Notifications
You must be signed in to change notification settings - Fork 10
Structural Refactor #87
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
base: master
Are you sure you want to change the base?
Structural Refactor #87
Conversation
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:
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. |
@stevefan1999-personal it looks like there are conflicts with |
@tarcieri the problem is that I re-sorted the entries, so it is not in symphony deliberately...maybe this could be another PR |
@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) |
fwiw @stevefan1999-personal noticed that you had carriage returns a.k.a \R (CR / Carrier) which come off Windows typically and they show up as new / changed lines in github diff's and ^M in vim. https://stackoverflow.com/questions/66038334/how-to-disable-m-line-endings-in-vs-code/73568412 Not sure what editor / IDE in use but I think there is an option to avoid MS-DOS like CR characters Also I noticed there is a transmute in that concat join macro when I upgraded your work for upcoming rustls 0.24 here: rustls 0.24 seems to change the tls ciphersuites as separate slices between 1.2 and 1.3 and gets rid of the feature flag. |
} | ||
slice_idx += 1; | ||
} | ||
&unsafe { transmute::<[MaybeUninit<$ty>; TOTAL_LEN], [$ty; TOTAL_LEN]>(out) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a macro but could we get rid of this transmute maybe ? rustls 0.24 makes it easier to construct the final ciphersuite since they are supplied separately between 1.2 and 1.3 versions so not sure we will need this macro in the long run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also worth to add // SAFETY:
comments to unsafe
blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember it doesn't pass Clippy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I would argue that we should wait for rustls 0.24 to finally commit on the change first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also worth to add
// SAFETY:
comments tounsafe
blocks.
There is also no need to add safety comments since this actually runs in const evaluation, meaning we don't have to worry about runtime behavior since we can just inspect the created const beforehand by asking rust-analyzer.
That said, it is an unfortunate hack that we need combine multiple slices together for the somewhat complicated feature selection, the lack of #[cfg]
on expression, and most importantly, running mutable operation despite working with precomputed process, which is still sadly not available for Rust until the algebraic effect or any related RFC is implemented, which means 5 or 10 years idk.
Sorry, my previous (deleted) comment was incorrect. |
Ah, yes. That's because I used Linux at my workplace, and Windows at home, at the same time I have autocrlf turned on... |
Also given the chance, I think we should shamefully steal the code from https://github.com/cryspen/hpke-rs :) @cryspen @tarcieri not sure if you guys would be happy about it |
I've managed to get QUIC working. Hooray! |
This PR represents a monumental refactoring effort that transforms
rustls-rustcrypto
from its initial alpha implementation into an almost production-ready, highly modular TLS provider, if performance is not of concern. Spanning 70+ commits over an extended development cycle, this overhaul addresses fundamental architectural issues and delivers a pure-Rust TLS solution optimized for diverse deployment scenarios.📈 Evolution Timeline
Phase 1: Foundation (Initial Implementation)
Phase 2: Feature Expansion & Stability
no_std
compatibility improvementsPhase 3: Major Structural Refactor
Phase 4: Production Readiness
🔧 Key Technical Changes
🏗️ Architectural Improvements
🔐 Cryptographic Enhancements
🛠️ Infrastructure & Quality
🧪 Testing & Validation
no_std
environments)🎯 Benefits & Impact
⚡ Performance & Efficiency
🔧 Developer Experience
🏭 Production Readiness
📋 Migration Guide
Breaking Changes:
Migration Steps:
Cargo.toml
✅ Validation Results
This refactoring marks the culmination of extensive development effort, delivering a TLS provider that rivals commercial offerings while maintaining the simplicity and security of pure Rust implementation. The modular architecture ensures long-term maintainability while the comprehensive feature set supports everything from embedded IoT devices to high-performance web servers.