Crispy-fried-chicken opened a new pull request, #21061:
URL: https://github.com/apache/pulsar/pull/21061
<!--
### Contribution Checklist
- PR title format should be *[fix][broker]fix 'Observable Timing
Discrepancy' vulnerability by replacing String.equals*. For details, see
*[Guideline - Pulsar PR Naming
Convention](https://pulsar.apache.org/contribute/develop-semantic-title/)*.
- Fill out the template below to describe the changes contributed by the
pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from
multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and
this checklist, leaving only the filled out template below.
-->
<!-- Either this PR fixes an issue, -->
Fixes #21053
### Motivation
<!-- Explain here the context, and why you're making that change. What is
the problem you're trying to solve. -->
I may have discovered a method in the newest version of
[pulsar](https://github.com/apache/pulsar), which has "Observable Timing
Discrepancy" vulnerability. The vulnerability is located in the method
org.apache.pulsar.broker.authentication.SaslRoleTokenSigner.verifyAndExtract(String
signedStr) . The vulnerability bears similarities to a recent CVE disclosure
https://github.com/advisories/GHSA-54g4-5cf6-hjp3 in the "apache/hive" project.
The source vulnerability information is as follows:
> **Vulnerability Detail:**
>
> **CVE Identifier:** CVE-2020-1926
>
> **Description**:Apache Hive cookie signature verification used a non
constant time comparison which is known to be vulnerable to timing attacks.
This could allow recovery of another users cookie signature. The issue was
addressed in Apache Hive 2.3.8.
>
> **Reference:**https://nvd.nist.gov/vuln/detail/CVE-2020-1926
>
> **Patch**:
https://github.com/apache/hive/commit/ee5a6be81a87bb21b3779edad6e61b67b365997b
**Vulnerability Description**: The vulnerability exists within the String
verifyAndExtract(String signedStr) method of the
org.apache.pulsar.broker.authentication.SaslRoleTokenSigner class. It is
responsible for authenticating and extracting SaslRoleToken. SaslRoleToken is a
token used for authentication and authorization, which contains a signed string
of information about roles and permissions.**But the authentication snippet is
similar to the vulnerable snippet for CVE-2020-1926 and may have the same
consequence as CVE-2020-1926, where isvulnerable to timing attacks.**
Therefore, maybe you need to fix the vulnerability with much the same fix code
as the CVE-2020-1926 patch.
### Modifications
<!-- Describe the modifications you've done. -->
### Verifying this change
I fix it by replacing `String.equals` with `MessageDigest.isEqual`
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
**This change is a trivial rework / code cleanup without any test coverage.**
*(or)*
This change is already covered by existing tests, such as *(please describe
tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads
(10MB)*
- *Extended integration test for recovery after broker failure*
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update
later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
