[fix][broker] Use MessageDigest.isEqual when comparing digests

[Crispy-fried-chicken]

Fixes #21053

Main issue #21053

Motivation

I may have discovered a method in the newest version of 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 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: apache/hive@ee5a6be

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

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:

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

• doc
• doc-required
• doc-not-needed
• doc-complete

[github-actions]

@Crispy-fried-chicken Please add the following content to your PR description and select a checkbox:

- [ ] `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 -->

[RobertIndie]

Thanks for your PR. Looks good to me.

[Crispy-fried-chicken]

Thanks for your PR. Looks good to me.

I saw the Pulsar CI / Build and License check (pull_request) has been failed. Is there anything shall I change?

[RobertIndie]

I saw the Pulsar CI / Build and License check (pull_request) has been failed. Is there anything shall I change?I saw the Pulsar CI / Build and License check (pull_request) has been failed. Is there anything shall I change?

@Crispy-fried-chicken Yes, you need to fix the code style issue:

Error:  src/main/java/org/apache/pulsar/broker/authentication/SaslRoleTokenSigner.java:[79,9] (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
Error:  src/main/java/org/apache/pulsar/broker/authentication/SaslRoleTokenSigner.java:[79,63] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.

[Crispy-fried-chicken]

I saw the Pulsar CI / Build and License check (pull_request) has been failed. Is there anything shall I change?I saw the Pulsar CI / Build and License check (pull_request) has been failed. Is there anything shall I change?

@Crispy-fried-chicken Yes, you need to fix the code style issue:

Error:  src/main/java/org/apache/pulsar/broker/authentication/SaslRoleTokenSigner.java:[79,9] (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
Error:  src/main/java/org/apache/pulsar/broker/authentication/SaslRoleTokenSigner.java:[79,63] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.

OK, I've already fix it. Please review it again? Thank you!

[michaeljmarshall]

LGTM. Note: I updated the PR name to remove certain words.