Skip to content

Commit 02b7cb3

Browse files
committed
Synchronize Mac init and JNI code.
Macs are inherently *not* thread safe, but before this change they can crash in native code if used across multiple threads.
1 parent b7e6501 commit 02b7cb3

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

common/src/main/java/org/conscrypt/OpenSSLMac.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ protected byte[] engineDoFinal() {
141141
protected abstract byte[] doFinal();
142142

143143
@Override
144-
protected void engineReset() {
144+
protected synchronized void engineReset() {
145145
if (!initialized) {
146146
return;
147147
}
@@ -163,36 +163,35 @@ public Hmac(long evpMd, int size) {
163163
}
164164

165165
@Override
166-
protected void initContext(byte[] keyBytes) {
166+
protected synchronized void initContext(byte[] keyBytes) {
167167
NativeRef.HMAC_CTX ctxLocal = new NativeRef.HMAC_CTX(NativeCrypto.HMAC_CTX_new());
168168
NativeCrypto.HMAC_Init_ex(ctxLocal, keyBytes, evpMd);
169169
this.ctx = ctxLocal;
170170
}
171171

172172
@Override
173-
protected void resetContext() {
173+
protected synchronized void resetContext() {
174174
final NativeRef.HMAC_CTX ctxLocal = ctx;
175175
NativeCrypto.HMAC_Reset(ctxLocal);
176176
}
177177

178178
@Override
179-
protected void engineUpdate(byte[] input, int offset, int len) {
179+
protected synchronized void engineUpdate(byte[] input, int offset, int len) {
180180
final NativeRef.HMAC_CTX ctxLocal = ctx;
181181
NativeCrypto.HMAC_Update(ctxLocal, input, offset, len);
182182
}
183183

184184
@Override
185-
protected void updateDirect(long ptr, int len) {
185+
protected synchronized void updateDirect(long ptr, int len) {
186186
final NativeRef.HMAC_CTX ctxLocal = ctx;
187187
NativeCrypto.HMAC_UpdateDirect(ctxLocal, ptr, len);
188188
}
189189

190190
@Override
191-
protected byte[] doFinal() {
191+
protected synchronized byte[] doFinal() {
192192
final NativeRef.HMAC_CTX ctxLocal = ctx;
193193
return NativeCrypto.HMAC_Final(ctxLocal);
194194
}
195-
196195
}
197196

198197
public static final class HmacMD5 extends Hmac {
@@ -239,32 +238,32 @@ public AesCmac() {
239238
}
240239

241240
@Override
242-
protected void initContext(byte[] keyBytes) {
241+
protected synchronized void initContext(byte[] keyBytes) {
243242
NativeRef.CMAC_CTX ctxLocal = new NativeRef.CMAC_CTX(NativeCrypto.CMAC_CTX_new());
244243
NativeCrypto.CMAC_Init(ctxLocal, keyBytes);
245244
this.ctx = ctxLocal;
246245
}
247246

248247
@Override
249-
protected void resetContext() {
248+
protected synchronized void resetContext() {
250249
final NativeRef.CMAC_CTX ctxLocal = ctx;
251250
NativeCrypto.CMAC_Reset(ctxLocal);
252251
}
253252

254253
@Override
255-
protected void updateDirect(long ptr, int len) {
254+
protected synchronized void updateDirect(long ptr, int len) {
256255
final NativeRef.CMAC_CTX ctxLocal = ctx;
257256
NativeCrypto.CMAC_UpdateDirect(ctxLocal, ptr, len);
258257
}
259258

260259
@Override
261-
protected byte[] doFinal() {
260+
protected synchronized byte[] doFinal() {
262261
final NativeRef.CMAC_CTX ctxLocal = ctx;
263262
return NativeCrypto.CMAC_Final(ctxLocal);
264263
}
265264

266265
@Override
267-
protected void engineUpdate(byte[] input, int offset, int len) {
266+
protected synchronized void engineUpdate(byte[] input, int offset, int len) {
268267
final NativeRef.CMAC_CTX ctxLocal = ctx;
269268
NativeCrypto.CMAC_Update(ctxLocal, input, offset, len);
270269
}

common/src/test/java/org/conscrypt/MacTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,27 @@ public void test(final Provider provider, final String algorithm) throws Excepti
195195
});
196196
}
197197

198+
@Test
199+
public void threadAbuse() {
200+
newMacServiceTester().run((provider, algorithm) -> {
201+
final byte[] b1 = new byte[2048];
202+
final byte[] b2 = new byte[2048];
203+
final byte[] b3 = new byte[2048];
204+
final Mac mac = Mac.getInstance(algorithm, provider);
205+
final SecretKeySpec key = findAnyKey(algorithm);
206+
207+
if (key != null) {
208+
TestUtils.stressTest(32, 32, () -> {
209+
mac.init(key);
210+
mac.update(b1);
211+
mac.update(b2);
212+
mac.update(b3);
213+
mac.reset();
214+
});
215+
}
216+
});
217+
}
218+
198219
@Test
199220
public void invalidCmacKeySizeThrows() throws Exception {
200221
// TODO(prb): extend to other Macs, deal with inconsistencies between providers.

0 commit comments

Comments
 (0)