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.

Sign up for GitHub

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

Jump to bottom

GRPC - Wallet username vulnerable to timing attacks #5810

Closed
SWvheerden opened this issue Sep 26, 2023 · 1 comment · Fixed by #5902
Closed

GRPC - Wallet username vulnerable to timing attacks #5810

SWvheerden opened this issue Sep 26, 2023 · 1 comment · Fixed by #5902
Assignees
@hansieodendaal
Labels
release-blocker Something that needs to be fixed before a release can be made

Comments

@SWvheerden
Copy link
Collaborator

The validate method does not use a constant-time compare function to check
usernames. This makes it possible for an attacker to analyze the time it takes for
the method to return in order to optimize their bruteforce attack.

if self.user_name.as_bytes() != username.as_bytes() {
return Err(BasicAuthError::InvalidUsername);
}

The absence of constant-time comparison permits an attacker to infer details
about the correct username by observing how long the system takes to respond to
various inputs. Consequently, an attacker can exploit this vulnerability to perform a
side-channel timing attack, systematically guessing the username and analyzing
the time taken for the system to respond. Over repeated attempts, this could
allow an attacker to reconstruct the correct username.

Implement a constant-time comparison method for usernames to prevent sidechannel attacks.
@SWvheerden SWvheerden added the release-blocker Something that needs to be fixed before a release can be made label Oct 23, 2023
@hansieodendaal hansieodendaal self-assigned this Nov 1, 2023
@hansieodendaal hansieodendaal mentioned this issue Nov 2, 2023
Merged
4 tasks
@hansieodendaal
Copy link
Contributor

See #5902

SWvheerden pushed a commit that referenced this issue Nov 10, 2023
@hansieodendaal
feat: add constant time comparison for grpc authentication (#5902)
2fe44db 
Description
---
- Added constant-time username comparison for gRPC authentication. This
will largely mitigate side-channel attacks to uncover the gRPC username.
(See `BasicAuthCredentials::constant_time_compare_username`)

- **Edit:** Credential validation for the combined username and password
will now also run in constant time and not return if the username did
not match as it did previously.

- **Edit:** Fixed an issue where the `BasicAuthCredentials` from header
did not pass validation, impacted in `fn it_generates_a_valid_header()`
and `it_decodes_from_well_formed_header`.

(Closes #5810)

Motivation and Context
---
See #5810

How Has This Been Tested?
---
- Added two unit tests to compare constant time performance with varying
length username guesses. The unit test was performed in release mode
with a no-load and fully loaded CPU.
- `fn it_compares_user_names_in_constant_time()` 
```
        // This unit test asserts that the minimum variance is less than 10% (chosen to be robust for running the unit
        // test with CI), indicating that the function behaves within acceptable constant-time constraints.
        //
        // Some consecutive results running in release mode on a Core i7-12700H (with no other processes running):
        //
        // Minimum variance:                          0.247 %
        // Average variance:                          4.65738 %
        // Average short username time:               1.17486 microseconds
        // Average long username time:                1.17344 microseconds
        // Average actual username time:              1.18388 microseconds
        //
        // Minimum variance:                          0.10214 %
        // Average variance:                          4.32226 %
        // Average short username time:               1.1619 microseconds
        // Average long username time:                1.16591 microseconds
        // Average actual username time:              1.18157 microseconds
        //
        // Minimum variance:                          0.17953 %
        // Average variance:                          5.51519 %
        // Average short username time:               1.17974 microseconds
        // Average long username time:                1.19232 microseconds
        // Average actual username time:              1.18709 microseconds
        //
        // Some consecutive results running in release mode on a Core i7-12700H (while entire CPU fully stressed):
        //
        // Minimum variance:                          0.60357 %
        // Average variance:                          6.30167 %
        // Average short username time:               1.81708 microseconds
        // Average long username time:                1.77562 microseconds
        // Average actual username time:              1.74824 microseconds
        //
        // Minimum variance:                          0.28176 %
        // Average variance:                          6.47136 %
        // Average short username time:               1.8317 microseconds
        // Average long username time:                1.8304 microseconds
        // Average actual username time:              1.80362 microseconds
        //
        // Minimum variance:                          0.53593 %
        // Average variance:                          6.99394 %
        // Average short username time:               1.82322 microseconds
        // Average long username time:                1.81431 microseconds
        // Average actual username time:              1.78002 microseconds
```
- `fn it_compares_credentials_in_constant_time()`
```
        // This unit test asserts that the minimum variance is less than 10% (chosen to be robust for running the unit
        // test with CI), indicating that the function behaves within acceptable constant-time constraints.
        //
        // Some consecutive results running in release mode on a Core i7-12700H (with no other processes running):
        //
        // Minimum variance:                          0.43478 %
        // Average variance:                          2.08995 %
        // Average short username time:               34.580 microseconds
        // Average long username time:                34.315 microseconds
        // Average actual username time:              34.260 microseconds
        //
        // Minimum variance:                          0.43731 %
        // Average variance:                          1.77209 %
        // Average short username time:               34.560 microseconds
        // Average long username time:                34.755 microseconds
        // Average actual username time:              34.690 microseconds
        //
        // Minimum variance:                          0.43988 %
        // Average variance:                          1.61299 %
        // Average short username time:               34.33999 microseconds
        // Average long username time:                34.38500 microseconds
        // Average actual username time:              34.28500 microseconds
        //
        // Some consecutive results running in release mode on a Core i7-12700H (while entire CPU fully stressed):
        //
        // Minimum variance:                          0.30326 %
        // Average variance:                          2.29341 %
        // Average short username time:               64.87500 microseconds
        // Average long username time:                65.55499 microseconds
        // Average actual username time:              65.81000 microseconds
        //
        // Minimum variance:                          1.18168 %
        // Average variance:                          2.99206 %
        // Average short username time:               67.970 microseconds
        // Average long username time:                68.000 microseconds
        // Average actual username time:              68.005 microseconds
        //
        // Minimum variance:                          1.01083 %
        // Average variance:                          2.31316 %
        // Average short username time:               68.715 microseconds
        // Average long username time:                69.675 microseconds
        // Average actual username time:              69.715 microseconds
```

What process can a PR reviewer use to test or verify this change?
---
Code walk through
Run the unit tests

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hansieodendaal hansieodendaal

Labels
release-blocker Something that needs to be fixed before a release can be made
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add constant time comparison for grpc authentication hansieodendaal/tari
2 participants
@SWvheerden @hansieodendaal