acme-client icon indicating copy to clipboard operation
acme-client copied to clipboard

Published 20 hours ago • verified publisher icon onur

Ensure consistent JWK generation.

Open thedodd opened this issue 6 years ago • 6 comments

I'm opening this PR because I was running into issues where the HTTP-01 workflow was failing due to invalid signatures and such. I investigated the code here after thoroughly reviewing my own code in the service which is using this crate. The issue has been resolved after using BTreeMap instead of standard HashMap.

It was one of those tough bugs to resolve where the issue was presenting itself intermittently. Sometimes the workflow would succeed, other times it would fail for no apparently reason or code change on my side.

After making this update, the issue has not presented itself. For reasoning straight from the docs, see the first few paragraphs under this heading: https://doc.rust-lang.org/std/collections/index.html#iterators

thedodd avatar Feb 14 '19 17:02 thedodd

@onur looks like some of the tests on Linux are having issues:

  • 1.34-nightly: account_registration & authorization tests are failing.
  • 1.32-stable: only authorization test is failing.

It looks like the same exact test failures took place on the last two commits on master (also only for Linux), so the failures were not introduced by this PR. How would you like to proceed with this?

thedodd avatar Feb 14 '19 18:02 thedodd

Hi @thedodd. Thanks for the PR.

I am not sure why tests started failing on travis. We also have same problem on #44. I'd like to merge that one first.

onur avatar Feb 15 '19 11:02 onur

Hey @onur. That sounds fine to me. That's great too, because then it would be possible to easily implement an async version of this using reqwest's async module. I'm currently using it in such a way that blocking network calls are wrapped in a blocking section to delegate them to background threads.

Keep me posted. I'm happy to rebase or make any other updates as needed.

thedodd avatar Mar 11 '19 18:03 thedodd

Awww man, just debugged the same issue, and wanted to submit basically the same PR. Can we get this merged?

axos88 avatar Apr 18 '19 07:04 axos88

@axos88 yea, this was a pesky issue to debug.

thedodd avatar Apr 18 '19 14:04 thedodd

I am attempting to upgrade this client to ACMEv02 (in a fork, since this appears to be abandoned). I've merged this in over in my repo to the main branch in this commit: https://github.com/dacut/acme-client/commit/53a9febe9b1ea5608ca2dabb7429a8f2435e91a2

dacut avatar Feb 12 '21 20:02 dacut