Skip to content

Commit dacd9a7

Browse files
tvernumtvernum
authored
Add startsWith and regionMatches to SecureString (elastic#134973)
Sometimes we need to compare parts of a SecureString to another sequences of characters. We would like to do this 1. Without converting the `SecureString` into a regular `String` 2. In constant time This commit adds the `startsWith` and `regionMatches` methods to `SecureString` so that we can perform those operations according to the above requirements
1 parent 2b0153b commit dacd9a7

File tree

2 files changed

+170
-2
lines changed

2 files changed

+170
-2
lines changed

‎server/src/main/java/org/elasticsearch/common/settings/SecureString.java‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,59 @@ public synchronized boolean equals(Object o) {
5454
return false;
5555
}
5656

57+
return compareChars(0, that, 0, chars.length);
58+
}
59+
60+
public boolean startsWith(CharSequence other) {
61+
ensureNotClosed();
62+
if (this.length() < other.length()) {
63+
return false;
64+
}
65+
return compareChars(0, other, 0, other.length());
66+
}
67+
68+
/**
69+
* Compare the characters in this {@code SecureString} from {@code thisOffset} to {@code thisOffset + len}
70+
* against that characters in {@code other} from {@code otherOffset} to {@code otherOffset + len}
71+
* <br />
72+
* This operation is performed in constant time (relative to {@code len})
73+
*/
74+
public synchronized boolean regionMatches(int thisOffset, CharSequence other, int otherOffset, int len) {
75+
ensureNotClosed();
76+
if (otherOffset < 0 || thisOffset < 0) {
77+
// cannot start from a negative value
78+
return false;
79+
}
80+
81+
// Perform some calculations as long to prevent overflow
82+
if (thisOffset + (long) len > this.length() || otherOffset + (long) len > other.length()) {
83+
// cannot compare a region that runs past the end of the source
84+
return false;
85+
}
86+
87+
if (len < 0) {
88+
throw new IllegalArgumentException("length cannot be negative");
89+
}
90+
if (len == 0) {
91+
// zero length regions are always equals
92+
return true;
93+
}
94+
95+
return compareChars(thisOffset, other, otherOffset, len);
96+
}
97+
98+
/**
99+
* Constant time comparison of a range from {@link #chars} against an identical length range from {@code other}
100+
*/
101+
private boolean compareChars(int thisOffset, CharSequence other, int otherOffset, int len) {
102+
assert len <= this.chars.length : "len is longer that secure-string: " + len + " vs " + this.chars.length;
103+
assert len <= other.length() : "len is longer that comparison string: " + len + " vs " + other.length();
104+
57105
int equals = 0;
58-
for (int i = 0; i < chars.length; i++) {
59-
equals |= chars[i] ^ that.charAt(i);
106+
for (int i = 0; i < len; i++) {
107+
final char t = chars[thisOffset + i];
108+
final char o = other.charAt(otherOffset + i);
109+
equals |= t ^ o;
60110
}
61111

62112
return equals == 0;

‎server/src/test/java/org/elasticsearch/common/settings/SecureStringTests.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
import java.util.Arrays;
1515

16+
import static org.hamcrest.CoreMatchers.is;
1617
import static org.hamcrest.Matchers.containsString;
18+
import static org.hamcrest.Matchers.hasLength;
1719
import static org.hamcrest.Matchers.not;
1820
import static org.hamcrest.Matchers.sameInstance;
1921

@@ -73,6 +75,122 @@ public void testGetCloseableCharsAfterSecureStringClosed() {
7375
assertThat(e.getMessage(), containsString("already been closed"));
7476
}
7577

78+
public void testEquals() {
79+
final String string = randomAlphaOfLengthBetween(8, 128);
80+
final SecureString secureString = new SecureString(string.toCharArray());
81+
final StringBuilder stringBuilder = new StringBuilder(string);
82+
83+
assertThat(secureString.equals(string), is(true));
84+
assertThat(secureString.equals(stringBuilder), is(true));
85+
86+
final int split = randomIntBetween(1, string.length() - 1);
87+
final String altString = string.substring(0, split) + randomAlphanumericOfLength(1) + string.substring(split);
88+
assertThat(secureString.equals(altString), is(false));
89+
}
90+
91+
public void testStartsWith() {
92+
final String str = randomAlphanumericOfLength(16);
93+
final SecureString secStr = new SecureString(str.toCharArray());
94+
for (int i = 0; i <= str.length(); i++) {
95+
final String substr = str.substring(0, i);
96+
assertThat(secStr.startsWith(substr), is(true));
97+
assertThat(secStr.startsWith(new SecureString(substr.toCharArray())), is(true));
98+
assertThat(secStr.startsWith(new StringBuilder(substr)), is(true));
99+
100+
if (i != 0) {
101+
assertThat(secStr.startsWith(randomValueOtherThan(substr, () -> randomAlphanumericOfLength(substr.length()))), is(false));
102+
if (i > 1) {
103+
final int suffixLength = randomIntBetween(1, i - 1);
104+
final String altStr = substr.substring(0, i - suffixLength) + randomValueOtherThan(
105+
substr.substring(i - suffixLength),
106+
() -> randomAlphanumericOfLength(suffixLength)
107+
);
108+
assertThat(altStr, hasLength(substr.length()));
109+
assertThat(secStr.startsWith(altStr), is(false));
110+
}
111+
}
112+
}
113+
114+
assertThat(secStr.startsWith(str + randomAlphanumericOfLength(1)), is(false));
115+
}
116+
117+
public void testRegionMatches() {
118+
// Matching text
119+
assertRegionMatch(true, "abc", 0, "012abc789", 3, 3);
120+
assertRegionMatch(true, "abc", 0, "abc789", 0, 3);
121+
assertRegionMatch(true, "abc", 0, "012abc", 3, 3);
122+
assertRegionMatch(true, "XYabcZ", 2, "012abc789", 3, 3);
123+
assertRegionMatch(true, "XYabcZ", 2, "abc789", 0, 3);
124+
assertRegionMatch(true, "XYabcZ", 2, "012abc", 3, 3);
125+
assertRegionMatch(true, "XYabcZ", 2, "abc", 0, 3);
126+
127+
// Bad region boundaries
128+
assertRegionMatch(false, "abc", -1, "abc", 0, 3);
129+
assertRegionMatch(false, "abc", 0, "abc", -1, 3);
130+
assertRegionMatch(false, "abc", 0, "abc", 0, 4);
131+
assertRegionMatch(false, "abc", 0, "ab", 0, 3);
132+
assertRegionMatch(false, "abc", 0, "Xab", 1, 3);
133+
134+
// Mismatched text
135+
assertRegionMatch(false, "abc", 0, "012", 0, 3);
136+
assertRegionMatch(false, "012abc", 3, "012", 0, 3);
137+
assertRegionMatch(false, "abc012", 0, "012", 0, 3);
138+
assertRegionMatch(false, "012abc789", 3, "012", 0, 3);
139+
assertRegionMatch(false, "abc", 0, "xyz012", 3, 3);
140+
assertRegionMatch(false, "012abc", 3, "xyz012", 3, 3);
141+
assertRegionMatch(false, "abc012", 0, "xyz012", 3, 3);
142+
assertRegionMatch(false, "012abc789", 3, "xyz012", 3, 3);
143+
assertRegionMatch(false, "abc", 0, "012xyz", 0, 3);
144+
assertRegionMatch(false, "012abc", 3, "012xyz", 0, 3);
145+
assertRegionMatch(false, "abc012", 0, "012xyz", 0, 3);
146+
assertRegionMatch(false, "012abc789", 3, "012xyz", 0, 3);
147+
assertRegionMatch(false, "abc", 0, "abc012xyz", 3, 3);
148+
assertRegionMatch(false, "012abc", 3, "abc012xyz", 3, 3);
149+
assertRegionMatch(false, "abc012", 0, "abc012xyz", 3, 3);
150+
assertRegionMatch(false, "012abc789", 3, "abc012xyz", 3, 3);
151+
152+
// Zero length always matches
153+
assertRegionMatch(
154+
true,
155+
randomAlphanumericOfLength(12),
156+
randomIntBetween(0, 12),
157+
randomAlphanumericOfLength(8),
158+
randomIntBetween(0, 8),
159+
0
160+
);
161+
162+
// Random chars
163+
final String shared = randomAlphanumericOfLength(randomIntBetween(4, 64));
164+
final String rand1 = randomAlphaOfLengthBetween(0, 256);
165+
final String rand2 = randomAlphaOfLengthBetween(0, 256);
166+
assertRegionMatch(
167+
true,
168+
rand1 + shared + randomAlphaOfLengthBetween(0, 256),
169+
rand1.length(),
170+
rand2 + shared + randomAlphaOfLengthBetween(0, 256),
171+
rand2.length(),
172+
shared.length()
173+
);
174+
assertRegionMatch(
175+
false,
176+
rand1 + shared + randomAlphaOfLengthBetween(0, 256),
177+
rand1.length(),
178+
rand2 + randomValueOtherThan(shared, () -> randomAlphanumericOfLength(shared.length())) + randomAlphaOfLengthBetween(0, 256),
179+
rand2.length(),
180+
shared.length()
181+
);
182+
}
183+
184+
private void assertRegionMatch(final boolean expected, String a, int offsetA, String b, int offsetB, int len) {
185+
// First check that our test strings match using the standard `String` version
186+
assert a.regionMatches(offsetA, b, offsetB, len) == expected
187+
: "Bad test case [" + a + "][" + offsetA + "] vs [" + b + "][" + offsetB + "] : " + len + " == " + expected;
188+
189+
// Test a-vs-b and b-vs-a because these operations should be identical
190+
assertThat(new SecureString(a.toCharArray()).regionMatches(offsetA, b, offsetB, len), is(expected));
191+
assertThat(new SecureString(b.toCharArray()).regionMatches(offsetB, a, offsetA, len), is(expected));
192+
}
193+
76194
private void assertSecureStringEqualToChars(char[] expected, SecureString secureString) {
77195
int pos = 0;
78196
for (int i : secureString.chars().toArray()) {

0 commit comments

Comments
 (0)