Skip to content

Commit 98c97fc

Browse files
stgatesstgates
committed
Incorporating feedback on Android server cert verification.
1 parent 68824c4 commit 98c97fc

File tree

1 file changed

+45
-140
lines changed

1 file changed

+45
-140
lines changed

‎Release/src/http/client/x509_cert_utilities.cpp

Lines changed: 45 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ bool verify_X509_cert_chain(const std::vector<std::string> &certChain, const std
144144
/// Helper function to check return value and see if any exceptions
145145
/// occurred when calling a JNI function.
146146
/// <summary>
147-
/// <returns><c>true</c> if JNI call <c>failed</c>, false othewise.</returns>
147+
/// <returns><c>true</c> if JNI call failed, <c>false</c> othewise.</returns>
148148
bool jni_failed(JNIEnv *env)
149149
{
150150
if(env->ExceptionOccurred())
@@ -158,18 +158,25 @@ bool jni_failed(JNIEnv *env)
158158
return false;
159159
}
160160
template <typename T>
161-
bool jni_failed(JNIEnv *env, const T &result)
161+
bool jni_failed(JNIEnv *env, const java_local_ref<T> &result)
162162
{
163-
if(jni_failed(env))
163+
if(jni_failed(env) || !result)
164164
{
165165
return true;
166166
}
167-
else if(result == nullptr)
167+
return false;
168+
}
169+
bool jni_failed(JNIEnv *env, const jmethodID &result)
170+
{
171+
if(jni_failed(env) || result == nullptr)
168172
{
169173
return true;
170174
}
171175
return false;
172176
}
177+
#define CHECK_JREF(env, obj) if(jni_failed<decltype(obj)::element_type>(env, obj)) return false;
178+
#define CHECK_JMID(env, mid) if(jni_failed(env, mid)) return false;
179+
#define CHECK_JNI(env) if(jni_failed(env)) return false;
173180

174181
bool verify_X509_cert_chain(const std::vector<std::string> &certChain, const std::string &hostName)
175182
{
@@ -183,136 +190,82 @@ bool verify_X509_cert_chain(const std::vector<std::string> &certChain, const std
183190

184191
// ByteArrayInputStream
185192
java_local_ref<jclass> byteArrayInputStreamClass(env->FindClass("java/io/ByteArrayInputStream"));
186-
if(jni_failed(env, byteArrayInputStreamClass))
187-
{
188-
return false;
189-
}
193+
CHECK_JREF(env, byteArrayInputStreamClass);
190194
jmethodID byteArrayInputStreamConstructorMethod = env->GetMethodID(
191195
byteArrayInputStreamClass.get(),
192196
"<init>",
193197
"([B)V");
194-
if(jni_failed(env, byteArrayInputStreamConstructorMethod))
195-
{
196-
return false;
197-
}
198+
CHECK_JMID(env, byteArrayInputStreamConstructorMethod);
198199

199200
// CertificateFactory
200201
java_local_ref<jclass> certificateFactoryClass(env->FindClass("java/security/cert/CertificateFactory"));
201-
if(jni_failed(env, certificateFactoryClass))
202-
{
203-
return false;
204-
}
202+
CHECK_JREF(env, certificateFactoryClass);
205203
jmethodID certificateFactoryGetInstanceMethod = env->GetStaticMethodID(
206204
certificateFactoryClass.get(),
207205
"getInstance",
208206
"(Ljava/lang/String;)Ljava/security/cert/CertificateFactory;");
209-
if(jni_failed(env, certificateFactoryGetInstanceMethod))
210-
{
211-
return false;
212-
}
207+
CHECK_JMID(env, certificateFactoryGetInstanceMethod);
213208
jmethodID generateCertificateMethod = env->GetMethodID(
214209
certificateFactoryClass.get(),
215210
"generateCertificate",
216211
"(Ljava/io/InputStream;)Ljava/security/cert/Certificate;");
217-
if(jni_failed(env, generateCertificateMethod))
218-
{
219-
return false;
220-
}
212+
CHECK_JMID(env, generateCertificateMethod);
221213

222214
// X509Certificate
223215
java_local_ref<jclass> X509CertificateClass(env->FindClass("java/security/cert/X509Certificate"));
224-
if(jni_failed(env, X509CertificateClass))
225-
{
226-
return false;
227-
}
216+
CHECK_JREF(env, X509CertificateClass);
228217

229218
// TrustManagerFactory
230219
java_local_ref<jclass> trustManagerFactoryClass(env->FindClass("javax/net/ssl/TrustManagerFactory"));
231-
if(jni_failed(env, trustManagerFactoryClass))
232-
{
233-
return false;
234-
}
220+
CHECK_JREF(env, trustManagerFactoryClass);
235221
jmethodID trustManagerFactoryGetInstanceMethod = env->GetStaticMethodID(
236222
trustManagerFactoryClass.get(),
237223
"getInstance",
238224
"(Ljava/lang/String;)Ljavax/net/ssl/TrustManagerFactory;");
239-
if(jni_failed(env, trustManagerFactoryGetInstanceMethod))
240-
{
241-
return false;
242-
}
225+
CHECK_JMID(env, trustManagerFactoryGetInstanceMethod);
243226
jmethodID trustManagerFactoryInitMethod = env->GetMethodID(
244227
trustManagerFactoryClass.get(),
245228
"init",
246229
"(Ljava/security/KeyStore;)V");
247-
if(jni_failed(env, trustManagerFactoryInitMethod))
248-
{
249-
return false;
250-
}
230+
CHECK_JMID(env, trustManagerFactoryInitMethod);
251231
jmethodID trustManagerFactoryGetTrustManagersMethod = env->GetMethodID(
252232
trustManagerFactoryClass.get(),
253233
"getTrustManagers",
254234
"()[Ljavax/net/ssl/TrustManager;");
255-
if(jni_failed(env, trustManagerFactoryGetTrustManagersMethod))
256-
{
257-
return false;
258-
}
235+
CHECK_JMID(env, trustManagerFactoryGetTrustManagersMethod);
259236

260237
// X509TrustManager
261238
java_local_ref<jclass> X509TrustManagerClass(env->FindClass("javax/net/ssl/X509TrustManager"));
262-
if(jni_failed(env, X509TrustManagerClass))
263-
{
264-
return false;
265-
}
239+
CHECK_JREF(env, X509TrustManagerClass);
266240
jmethodID X509TrustManagerCheckServerTrustedMethod = env->GetMethodID(
267241
X509TrustManagerClass.get(),
268242
"checkServerTrusted",
269243
"([Ljava/security/cert/X509Certificate;Ljava/lang/String;)V");
270-
if(jni_failed(env, X509TrustManagerCheckServerTrustedMethod))
271-
{
272-
return false;
273-
}
244+
CHECK_JMID(env, X509TrustManagerCheckServerTrustedMethod);
274245

275246
// StrictHostnameVerifier
276247
java_local_ref<jclass> strictHostnameVerifierClass(env->FindClass("org/apache/http/conn/ssl/StrictHostnameVerifier"));
277-
if(jni_failed(env, strictHostnameVerifierClass))
278-
{
279-
return false;
280-
}
248+
CHECK_JREF(env, strictHostnameVerifierClass);
281249
jmethodID strictHostnameVerifierConstructorMethod = env->GetMethodID(strictHostnameVerifierClass.get(), "<init>", "()V");
282-
if(jni_failed(env, strictHostnameVerifierConstructorMethod))
283-
{
284-
return false;
285-
}
250+
CHECK_JMID(env, strictHostnameVerifierConstructorMethod);
286251
jmethodID strictHostnameVerifierVerifyMethod = env->GetMethodID(
287252
strictHostnameVerifierClass.get(),
288253
"verify",
289254
"(Ljava/lang/String;Ljava/security/cert/X509Certificate;)V");
290-
if(jni_failed(env, strictHostnameVerifierVerifyMethod))
291-
{
292-
return false;
293-
}
255+
CHECK_JMID(env, strictHostnameVerifierVerifyMethod);
294256

295257
// Create CertificateFactory
296258
java_local_ref<jstring> XDot509String(env->NewStringUTF("X.509"));
297-
if(jni_failed(env, XDot509String))
298-
{
299-
return false;
300-
}
259+
CHECK_JREF(env, XDot509String);
301260
java_local_ref<jobject> certificateFactory(env->CallStaticObjectMethod(
302261
certificateFactoryClass.get(),
303262
certificateFactoryGetInstanceMethod,
304263
XDot509String.get()));
305-
if(jni_failed(env, certificateFactory))
306-
{
307-
return false;
308-
}
264+
CHECK_JREF(env, certificateFactory);
309265

310266
// Create Java array to store all the certs in.
311267
java_local_ref<jobjectArray> certsArray(env->NewObjectArray(certChain.size(), X509CertificateClass.get(), nullptr));
312-
if(jni_failed(env, certsArray))
313-
{
314-
return false;
315-
}
268+
CHECK_JREF(env, certsArray);
316269

317270
// For each certificate perform the following steps:
318271
// 1. Create ByteArrayInputStream backed by DER certificate bytes
@@ -322,116 +275,68 @@ bool verify_X509_cert_chain(const std::vector<std::string> &certChain, const std
322275
for(const auto &certData : certChain)
323276
{
324277
java_local_ref<jbyteArray> byteArray(env->NewByteArray(certData.size()));
325-
if(jni_failed(env, byteArray))
326-
{
327-
return false;
328-
}
278+
CHECK_JREF(env, byteArray);
329279
env->SetByteArrayRegion(byteArray.get(), 0, certData.size(), reinterpret_cast<const jbyte *>(certData.c_str()));
330-
if(jni_failed(env))
331-
{
332-
return false;
333-
}
280+
CHECK_JNI(env);
334281
java_local_ref<jobject> byteArrayInputStream(env->NewObject(
335282
byteArrayInputStreamClass.get(),
336283
byteArrayInputStreamConstructorMethod,
337284
byteArray.get()));
338-
if(jni_failed(env, byteArrayInputStream))
339-
{
340-
return false;
341-
}
285+
CHECK_JREF(env, byteArrayInputStream);
342286

343287
java_local_ref<jobject> cert(env->CallObjectMethod(
344288
certificateFactory.get(),
345289
generateCertificateMethod,
346290
byteArrayInputStream.get()));
347-
if(jni_failed(env, cert))
348-
{
349-
return false;
350-
}
291+
CHECK_JREF(env, cert);
351292

352293
env->SetObjectArrayElement(certsArray.get(), i, cert.get());
353-
if(jni_failed(env))
354-
{
355-
return false;
356-
}
294+
CHECK_JNI(env);
357295
++i;
358296
}
359297

360298
// Create TrustManagerFactory, init with Android system certs
361299
java_local_ref<jstring> X509String(env->NewStringUTF("X509"));
362-
if(jni_failed(env, X509String))
363-
{
364-
return false;
365-
}
300+
CHECK_JREF(env, X509String);
366301
java_local_ref<jobject> trustFactoryManager(env->CallStaticObjectMethod(
367302
trustManagerFactoryClass.get(),
368303
trustManagerFactoryGetInstanceMethod,
369304
X509String.get()));
370-
if(jni_failed(env, trustFactoryManager))
371-
{
372-
return false;
373-
}
305+
CHECK_JREF(env, trustFactoryManager);
374306
env->CallVoidMethod(trustFactoryManager.get(), trustManagerFactoryInitMethod, nullptr);
375-
if(jni_failed(env))
376-
{
377-
return false;
378-
}
307+
CHECK_JNI(env);
379308

380309
// Get TrustManager
381310
java_local_ref<jobjectArray> trustManagerArray(static_cast<jobjectArray>(
382311
env->CallObjectMethod(trustFactoryManager.get(), trustManagerFactoryGetTrustManagersMethod)));
383-
if(jni_failed(env, trustManagerArray))
384-
{
385-
return false;
386-
}
312+
CHECK_JREF(env, trustManagerArray);
387313
java_local_ref<jobject> trustManager(env->GetObjectArrayElement(trustManagerArray.get(), 0));
388-
if(jni_failed(env, trustManager))
389-
{
390-
return false;
391-
}
314+
CHECK_JREF(env, trustManager);
392315

393316
// Validate certificate chain.
394317
java_local_ref<jstring> RSAString(env->NewStringUTF("RSA"));
395-
if(jni_failed(env, RSAString))
396-
{
397-
return false;
398-
}
318+
CHECK_JREF(env, RSAString);
399319
env->CallVoidMethod(
400320
trustManager.get(),
401321
X509TrustManagerCheckServerTrustedMethod,
402322
certsArray.get(),
403323
RSAString.get());
404-
if(jni_failed(env))
405-
{
406-
return false;
407-
}
324+
CHECK_JNI(env);
408325

409326
// Verify hostname on certificate according to RFC 2818.
410327
java_local_ref<jobject> hostnameVerifier(env->NewObject(
411328
strictHostnameVerifierClass.get(), strictHostnameVerifierConstructorMethod));
412-
if(jni_failed(env, hostnameVerifier))
413-
{
414-
return false;
415-
}
329+
CHECK_JREF(env, hostnameVerifier);
416330
java_local_ref<jstring> hostNameString(env->NewStringUTF(hostName.c_str()));
417-
if(jni_failed(env, hostNameString))
418-
{
419-
return false;
420-
}
331+
CHECK_JREF(env, hostNameString);
421332
java_local_ref<jobject> cert(env->GetObjectArrayElement(certsArray.get(), 0));
422-
if(jni_failed(env, cert))
423-
{
424-
return false;
425-
}
333+
CHECK_JREF(env, cert);
426334
env->CallVoidMethod(
427335
hostnameVerifier.get(),
428336
strictHostnameVerifierVerifyMethod,
429337
hostNameString.get(),
430338
cert.get());
431-
if(jni_failed(env))
432-
{
433-
return false;
434-
}
339+
CHECK_JNI(env);
435340

436341
return true;
437342
}

0 commit comments

Comments
 (0)