Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AndroidCrypto] Fix handling of no peer certificates #51316

Merged
merged 3 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,30 +173,18 @@ internal static string SSLStreamGetProtocol(SafeSslHandle ssl)
}

[DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_SSLStreamGetPeerCertificate")]
private static extern int SSLStreamGetPeerCertificate(SafeSslHandle ssl, out SafeX509Handle cert);
internal static SafeX509Handle SSLStreamGetPeerCertificate(SafeSslHandle ssl)
{
SafeX509Handle cert;
int ret = Interop.AndroidCrypto.SSLStreamGetPeerCertificate(ssl, out cert);
if (ret != SUCCESS)
throw new SslException();

return cert;
}
internal static extern SafeX509Handle SSLStreamGetPeerCertificate(SafeSslHandle ssl);

[DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_SSLStreamGetPeerCertificates")]
private static extern int SSLStreamGetPeerCertificates(
private static extern void SSLStreamGetPeerCertificates(
SafeSslHandle ssl,
[MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)] out IntPtr[] certs,
out int count);
internal static IntPtr[] SSLStreamGetPeerCertificates(SafeSslHandle ssl)
internal static IntPtr[]? SSLStreamGetPeerCertificates(SafeSslHandle ssl)
{
IntPtr[] ptrs;
IntPtr[]? ptrs;
int count;
int ret = Interop.AndroidCrypto.SSLStreamGetPeerCertificates(ssl, out ptrs, out count);
if (ret != SUCCESS)
throw new SslException();

Interop.AndroidCrypto.SSLStreamGetPeerCertificates(ssl, out ptrs, out count);
return ptrs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,16 @@ static bool IsHandshaking(int handshakeStatus)

static PAL_SSLStreamStatus Close(JNIEnv* env, SSLStream* sslStream)
{
// Call wrap to clear any remaining data before closing
int unused;
PAL_SSLStreamStatus ret = DoWrap(env, sslStream, &unused);

// sslEngine.closeOutbound();
(*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineCloseOutbound);
if (ret != SSLStreamStatus_OK)
return ret;

// Call wrap to clear any remaining data
int unused;
// Flush any remaining data (e.g. sending close notification)
return DoWrap(env, sslStream, &unused);
}

Expand Down Expand Up @@ -698,41 +703,40 @@ int32_t AndroidCryptoNative_SSLStreamGetProtocol(SSLStream* sslStream, uint16_t*
return ret;
}

int32_t AndroidCryptoNative_SSLStreamGetPeerCertificate(SSLStream* sslStream, jobject* out)
jobject /*X509Certificate*/ AndroidCryptoNative_SSLStreamGetPeerCertificate(SSLStream* sslStream)
{
assert(sslStream != NULL);
assert(out != NULL);

JNIEnv* env = GetJNIEnv();
int32_t ret = FAIL;
*out = NULL;
jobject ret = NULL;

// Certificate[] certs = sslSession.getPeerCertificates();
// out = certs[0];
jobjectArray certs = (*env)->CallObjectMethod(env, sslStream->sslSession, g_SSLSessionGetPeerCertificates);
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);

// If there are no peer certificates, getPeerCertificates will throw. Return null to indicate no certificate.
if (TryClearJNIExceptions(env))
goto cleanup;

jsize len = (*env)->GetArrayLength(env, certs);
if (len > 0)
{
// First element is the peer's own certificate
jobject cert = (*env)->GetObjectArrayElement(env, certs, 0);
*out = ToGRef(env, cert);
ret = ToGRef(env, cert);
}

ret = SUCCESS;

cleanup:
(*env)->DeleteLocalRef(env, certs);
return ret;
}

int32_t AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream, jobject** out, int32_t* outLen)
void AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream, jobject** out, int32_t* outLen)
{
assert(sslStream != NULL);
assert(out != NULL);

JNIEnv* env = GetJNIEnv();
int32_t ret = FAIL;
*out = NULL;
*outLen = 0;

Expand All @@ -741,7 +745,11 @@ int32_t AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream, j
// out[i] = certs[i];
// }
jobjectArray certs = (*env)->CallObjectMethod(env, sslStream->sslSession, g_SSLSessionGetPeerCertificates);
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);

// If there are no peer certificates, getPeerCertificates will throw. Return null and length of zero to indicate no certificates.
if (TryClearJNIExceptions(env))
goto cleanup;

jsize len = (*env)->GetArrayLength(env, certs);
*outLen = len;
if (len > 0)
Expand All @@ -754,11 +762,8 @@ int32_t AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream, j
}
}

ret = SUCCESS;

cleanup:
(*env)->DeleteLocalRef(env, certs);
return ret;
}

static jstring GetSslProtocolAsString(JNIEnv* env, PAL_SslProtocol protocol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,21 +130,18 @@ PALEXPORT int32_t AndroidCryptoNative_SSLStreamGetProtocol(SSLStream* sslStream,
/*
Get the peer certificate for the current session

Returns 1 on success, 0 otherwise
Returns the peer certificate or null if there is no peer certificate.
*/
PALEXPORT int32_t AndroidCryptoNative_SSLStreamGetPeerCertificate(SSLStream* sslStream,
jobject* /*X509Certificate*/ out);
PALEXPORT jobject /*X509Certificate*/ AndroidCryptoNative_SSLStreamGetPeerCertificate(SSLStream* sslStream);

/*
Get the peer certificates for the current session

The peer's own certificate will be first, followed by any certificate authorities.

Returns 1 on success, 0 otherwise
*/
PALEXPORT int32_t AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream,
jobject** /*X509Certificate[]*/ out,
int32_t* outLen);
PALEXPORT void AndroidCryptoNative_SSLStreamGetPeerCertificates(SSLStream* sslStream,
jobject** /*X509Certificate[]*/ out,
int32_t* outLen);

/*
Set enabled protocols
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ internal static SslPolicyErrors VerifyCertificateProperties(
}
else
{
IntPtr[] ptrs = Interop.AndroidCrypto.SSLStreamGetPeerCertificates(sslContext);
if (ptrs.Length > 0)
IntPtr[]? ptrs = Interop.AndroidCrypto.SSLStreamGetPeerCertificates(sslContext);
if (ptrs != null && ptrs.Length > 0)
{
// This is intentionally a different object from the cert added to the remote certificate store
// to match the behaviour on other platforms.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ public static SecurityStatusPal DecryptMessage(
if (attribute == ChannelBindingKind.Endpoint)
return EndpointChannelBindingToken.Build(securityContext);

throw new NotImplementedException(nameof(QueryContextChannelBinding));
// Android doesn't expose the Finished messages, so a Unique binding token cannot be built.
// Return null for not supported kinds
return null;
}

public static void QueryContextStreamSizes(
Expand Down