Skip to content

Ensure consistent JWK generation. - #45

Open
thedodd wants to merge 1 commit into
onur:masterfrom
thedodd:consistent-jwk
Open

Ensure consistent JWK generation.#45
thedodd wants to merge 1 commit into
onur:masterfrom
thedodd:consistent-jwk

Conversation

@thedodd

@thedodd thedodd commented Feb 14, 2019

Copy link
Copy Markdown

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

Added a few debugging logs as well.
@thedodd

thedodd commented Feb 14, 2019

Copy link
Copy Markdown
Author

@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?

@onur

onur commented Feb 15, 2019

Copy link
Copy Markdown
Owner

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.

@thedodd

thedodd commented Mar 11, 2019

Copy link
Copy Markdown
Author

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.

@axos88

axos88 commented Apr 18, 2019

Copy link
Copy Markdown

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

@thedodd

thedodd commented Apr 18, 2019

Copy link
Copy Markdown
Author

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

@dacut

dacut commented Feb 12, 2021

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants