Skip to content

Conversation

@tvernum
Copy link
Contributor

@tvernum tvernum commented Sep 19, 2025

Changes the SecureString methods equals startsWith and regionMatches to operate in constant time relative to the length of that comparison string, regardless of the length of the secure string.

That is, the time to perform each of these comparisons should be the same (even though some of them could be computed more efficiently)

new SecureString("a").equals("abcdefghijklmn")
new SecureString("abcdefghijklmn").equals("abcdefghijklmn")
new SecureString("abcdefghijklmn").equals("##############")
new SecureString("abcdefghijklmX").equals("abcdefghijklmn")
new SecureString("X".repeat(5000)).equals("ababababababab")

And similarly for startsWith and regionMatches

Changes the SecureString methods `equals` `startsWith` and
`regionMatches` to operate in constant time relative to the length of
that comparison string, regardless of the length of the secure string.

That is, the time to perform each of these comparisons should be the
same (even though some of them _could_ be computed more efficiently)

    new SecureString("a").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("##############")
    new SecureString("abcdefghijklmX").equals("abcdefghijklmn")
    new SecureString("X".repeat(5000)).equals("ababababababab")

And similarly for `startsWith` and `regionMatches`
@tvernum tvernum requested a review from a team as a code owner September 19, 2025 04:31
@tvernum tvernum added >enhancement :Core/Infra/Core Core issues without another label labels Sep 19, 2025
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.2.0 labels Sep 19, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @tvernum, I've created a changelog YAML for you.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@tvernum tvernum merged commit fb76bed into elastic:main Sep 22, 2025
34 checks passed
szybia added a commit to szybia/elasticsearch that referenced this pull request Sep 22, 2025
* upstream/main: (50 commits)
  Disable utf-8 parsing optimization (elastic#135172)
  rest-api-spec: fix master_timeout typo (elastic#135167)
  Fixes countDistinctWithConditions in csv-spec tests (elastic#135097)
  Fix test failure by checking for feature flag (elastic#135174)
  Fix deadlock in ThreadPoolMergeScheduler when a failing merge closes the IndexWriter (elastic#134656)
  Make SecureString comparisons constant time (elastic#135053)
  Mute org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT test {p0=search/160_exists_query/Test exists query on mapped geo_point field with no doc values} elastic#135164
  ESQL: Replace function count tests (elastic#134951)
  Mute org.elasticsearch.compute.aggregation.SampleBooleanAggregatorFunctionTests testSimpleWithCranky elastic#135163
  Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=analytics/nested_top_metrics_sort/terms order by top metrics numeric not null integer values} elastic#135162
  Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=analytics/nested_top_metrics_sort/terms order by top metrics numeric not null double values} elastic#135159
  TSDB ingest performance: combine routing and tsdb hashing (elastic#132566)
  Mute org.elasticsearch.compute.aggregation.SampleBytesRefAggregatorFunctionTests testSimpleWithCranky elastic#135157
  Mute org.elasticsearch.xpack.logsdb.qa.BulkStoredSourceChallengeRestIT testHistogramAggregation elastic#135156
  Mute org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT testHistogramAggregation elastic#135155
  Mute org.elasticsearch.xpack.logsdb.qa.LogsDbVersusLogsDbReindexedIntoStandardModeChallengeRestIT testHistogramAggregation elastic#135154
  Mute org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT testHistogramAggregation elastic#135153
  Mute org.elasticsearch.discovery.ClusterDisruptionIT testAckedIndexing elastic#117024
  Mute org.elasticsearch.lucene.RollingUpgradeSearchableSnapshotIndexCompatibilityIT testMountSearchableSnapshot {p0=[9.2.0, 9.2.0, 9.2.0]} elastic#135151
  Mute org.elasticsearch.lucene.RollingUpgradeSearchableSnapshotIndexCompatibilityIT testSearchableSnapshotUpgrade {p0=[9.2.0, 9.2.0, 9.2.0]} elastic#135150
  ...
gmjehovich pushed a commit to gmjehovich/elasticsearch that referenced this pull request Sep 22, 2025
Changes the SecureString methods `equals` `startsWith` and
`regionMatches` to operate in constant time relative to the length of
that comparison string, regardless of the length of the secure string.

That is, the time to perform each of these comparisons should be the
same (even though some of them _could_ be computed more efficiently)

    new SecureString("a").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("##############")
    new SecureString("abcdefghijklmX").equals("abcdefghijklmn")
    new SecureString("X".repeat(5000)).equals("ababababababab")

And similarly for `startsWith` and `regionMatches`
DonalEvans pushed a commit to DonalEvans/elasticsearch that referenced this pull request Sep 22, 2025
Changes the SecureString methods `equals` `startsWith` and
`regionMatches` to operate in constant time relative to the length of
that comparison string, regardless of the length of the secure string.

That is, the time to perform each of these comparisons should be the
same (even though some of them _could_ be computed more efficiently)

    new SecureString("a").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("abcdefghijklmn")
    new SecureString("abcdefghijklmn").equals("##############")
    new SecureString("abcdefghijklmX").equals("abcdefghijklmn")
    new SecureString("X".repeat(5000)).equals("ababababababab")

And similarly for `startsWith` and `regionMatches`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants