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

Island: Add a small throttle to shorten the OTP #4188

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

VakarisZ
Copy link
Contributor

What does this PR do?

Part of #4187

Calculations

7 chars (alphanumeric + 3 special characters) gives total of 4,764,996,883,946 combinations (based on this
The current throttle is 0.001, so the result is that you're able to check 120 000 passwords per 2 minutes.
This means that you chance of guessing the OTP is 1 in 39,708,307. In practice it's much higher if you account for network latency. I think this is sufficient security for a problem whose real solution is NAC.

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Copy link

codecov bot commented Jun 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.06%. Comparing base (af54997) to head (cee2e2f).

Current head cee2e2f differs from pull request most recent head 3ccd633

Please upload reports for the commit 3ccd633 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4188   +/-   ##
========================================
  Coverage    77.06%   77.06%           
========================================
  Files          442      442           
  Lines        14125    14126    +1     
  Branches        18       18           
========================================
+ Hits         10885    10886    +1     
  Misses        3240     3240           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VakarisZ VakarisZ marked this pull request as ready for review June 11, 2024 07:57
Copy link
Collaborator

@mssalvatore mssalvatore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to squash these two commits together?

mssalvatore pushed a commit that referenced this pull request Jun 11, 2024
16 characters is plenty secure for a token with a 2-minute lifespan and
shortens the payload sufficiently

Issue #4187
PR #4188
16 characters is plenty secure for a token with a 2-minute lifespan and
shortens the payload sufficiently

Issue #4187
PR #4188
@mssalvatore mssalvatore merged commit 925c986 into develop Jun 11, 2024
1 check failed
@mssalvatore mssalvatore deleted the 4187-shorten-otp branch June 11, 2024 13:58
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.

3 participants