Add configurable clock skew flag #887
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Rebased this back onto main to catch up with other PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is just cherry-picked from the work I previously did in #871.
At the time, I was unable to replicate the clock skew issue, and we only had one report. Since then, there's been a couple of issues (NanoMDM #71, #886) reported experiencing the same issue @tgunz first reported on the MacAdmins Slack.
As-is, this adds a flag to configure the clock skew, and it defaults to 0s.
I still think it's worth exploring setting the default to 5 minutes (see here for why that duration). It would mean the issue is fixed transparently for most users instead of issues being raised here or on the MacAdmins slack.
More importantly, I don't see any downsides to this change. In theory, I guess this could possibly open you to easier replay attacks, but the attacker would have to MiTM your client to get the header in the first place, and that would be a much larger issue. Maybe I'm missing something else here, though...