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

Use constant time compare in HMAC example #284

Merged
merged 2 commits into from
Nov 3, 2019

Conversation

bdewater
Copy link
Contributor

@bdewater bdewater commented Nov 1, 2019

We could also call to_s inside fixed_length_secure_compare to make it a bit easier to pass in instances like these?

@ioquatix
Copy link
Member

ioquatix commented Nov 1, 2019

What about to_str

@ioquatix
Copy link
Member

ioquatix commented Nov 1, 2019

Need to be careful that people don’t end up comparing object descriptions.

@bdewater
Copy link
Contributor Author

bdewater commented Nov 1, 2019

In the example given or in fixed_length_secure_compare?

@ioquatix
Copy link
Member

ioquatix commented Nov 2, 2019

In the compare function.

@bdewater bdewater force-pushed the hmac-secure-compare branch 2 times, most recently from e73dbf1 to b0f0ada Compare November 2, 2019 21:02
@ioquatix
Copy link
Member

ioquatix commented Nov 2, 2019

When I look at OpenSSL.fixed_length_secure_compare(instance, other_instance)

I think why are we doing that.

If users should do that by default in most situations, why not just implement it like that...

e.g.

class HMAC
	def == other
		OpenSSL.fixed_lenght_secure_compare(self.digest, other.digest)
	end
end

Of course we need to handle a few other details... but that's kind of what makes more sense to me.

@bdewater
Copy link
Contributor Author

bdewater commented Nov 3, 2019

Yep that's a lot easier for people. I've left the original README commit intact in case that's all you want to ship that for the upcoming release, otherwise feel free to squash it down and use the last commit message only :)

@ioquatix ioquatix merged commit 664ba34 into ruby:master Nov 3, 2019
@bdewater bdewater deleted the hmac-secure-compare branch November 3, 2019 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants