jberger / Mojo-ACME

Mojo-based ACME-protocol client
10 stars 5 forks source link

Should use constant-time string compare in HMAC #1

Closed hoytech closed 8 years ago

hoytech commented 8 years ago

On this line:

https://github.com/jberger/Mojo-ACME/blob/master/lib/Mojo/ACME/ChallengeServer.pm#L40

You should probably use Mojo::Util::secure_compare instead of 'eq'.

The reason for this is explained in more detail here:

https://metacpan.org/pod/String::Compare::ConstantTime

PS. Personally I like Protocol::ACME's model where it can do everything over SSH. Maybe it would be nice to mention that module in your SEE ALSO section? There's also WWW::LetsEncrypt (no idea about that one).

jberger commented 8 years ago

Yeah, I forgot to add several SEE ALSOs in my push to release (as did I forget some pod in a few places). Constant time comparison is probably worth adding but it also might be worth adding a two second minimum time too since requests might not hit any client challenge-server and return "early".

I'll have to read about Protocol::ACME's use of ssh but I suspect that it is out of the design scope for my module.

On Mon, Mar 21, 2016 at 1:45 PM Doug Hoyte notifications@github.com wrote:

On this line:

https://github.com/jberger/Mojo-ACME/blob/master/lib/Mojo/ACME/ChallengeServer.pm#L40

You should probably use Mojo::Util::secure_compare instead of 'eq'.

The reason for this is explained in more detail here:

https://metacpan.org/pod/String::Compare::ConstantTime

PS. Personally I like Protocol::ACME's model where it can do everything over SSH. Maybe it would be nice to mention that module in your SEE ALSO section? There's also WWW::LetsEncrypt (no idea about that one).

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/jberger/Mojo-ACME/issues/1