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

fix: refactor rust bindings fingerprint methods #4474

Merged
merged 10 commits into from
Mar 29, 2024
Merged

Conversation

maddeleine
Copy link
Contributor

@maddeleine maddeleine commented Mar 25, 2024

Resolved issues:

N/A

Description of changes:

The feature flag unstable-fingerprint was tagged to the client_hello method, which means that no one could retrieve the client hello or any of the getters on it without turning on the unstable-fingerprint flag. I uncoupled the client hello methods from the fingerprint feature by moving the fingerprint-specific code to a module.

Call-outs:

Testing:

Includes test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Mar 25, 2024
@maddeleine maddeleine requested a review from jmayclin March 25, 2024 23:41
@maddeleine maddeleine changed the title fix: Made Client Hello methods public fix: refactor rust bindings fingerprint methods Mar 26, 2024
@maddeleine maddeleine requested a review from lrstewart March 26, 2024 16:36
@maddeleine maddeleine requested a review from lrstewart March 27, 2024 16:32
@maddeleine maddeleine requested review from camshaft and removed request for camshaft March 28, 2024 21:28
use super::ClientHello;

#[derive(Copy, Clone)]
pub enum FingerprintType {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in the scope of this PR, but we should probably mark this non-exhaustive before we do the actual 0.2 release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already did the 0.2 release :) But we can mark it now, since fingerprinting is unstable anyway.

Comment on lines 400 to -227
let message_head = self.raw_message().map_err(|_| fmt::Error)?;
let mut hash = Vec::new();
self.fingerprint_hash(FingerprintType::JA3, &mut hash)
.map_err(|_| fmt::Error)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm disappointed that you couldn't figure out a way to keep this, but we can always add it back later.

@maddeleine maddeleine merged commit 80b9242 into main Mar 29, 2024
35 checks passed
@maddeleine maddeleine deleted the server_name_fix branch March 29, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants