February 20th, 2017 · Public Disclosure
Having already been disappointed in a mobile banking application in the past, I've decided to take a closer look at another one. I've started by looking at the shared preferences and other miscellaneous files for the application, see if it stores anything confidential, but that was not the case. The next step was to try and man-in-the-middle the application, however, all connections failed, even though I had a trusted CA installed. While certificate pinning is a best practice, I did not expect it to be implemented, based on previous experience. So far, so good.
The next step was to grab the APK off my device and disassemble it, mostly out of curiosity, to see how the certificate pinning is implemented, and also to check if it is hiding confidential credentials in a file under a path other than what I previously checked.
Looking at the networking part of the code, a specific class is responsible for making all the calls to the backend API of the bank. This code, unfortunately, has some interesting bits: (Note: the code is not copied from the disassembled source, instead recreated to demonstrate the original functionality.)
public T sendRequest(String url, ...) {
if (!verifyConnection(url)) {
return null;
}
this.connection = new Connection(url, ...);
this.connection.setHostnameVerifier(DO_NOT_VERIFY);
}
Before sending each request to the backend, the application first checks the connection. This check (done by verifyConnection) is very brief, it only connects to the host and retrieves its SSL certificate. This certificate is then validated with custom logic, and the result (whether it is deemed valid or not) is returned to sendRequest, which continues on with the normal procedure or stops with an error.
The problematic part here is that once the connection is deemed secure, the application will create a completely new connection, where the hostname verifier is set to a variable called DO_NOT_VERIFY, which is an instance of a custom HostnameVerifier implementation, which in turn accepts any name passed to it. It is important to note here, that TLS verfication is not turned off, only the name checking phase. The type of this vulnerability is classified as CWE-297.
As a result, it is possible to write a script for your MITM tool of choice, which ignores the first TLS connection (either based on the destination being the IP of the bank, or by inspecting the TLS ClientHello's SNI field) and then MITMs the second connection, since this is after verification, when the actual connection is happening. All you need to make sure of is to send a valid certificate, but it can be valid for any domain name.
Digging deeper into the code, I've decided to take a look at their custom logic for certificate verification.
After getting the SSL certificate of the remote host, the certificate chain is built using the CertificateFactory.generateCertPath() API, and then the resulting CertPath object is converted to string, which looks something like this:
Certificate[1]:
Owner: CN=rolisoft.net
Issuer: CN=Let's Encrypt Authority X3, O=Let's Encrypt, C=US
Serial number: 37c1c5376c2a39908e37db13f27ba371ee1
Valid from: Sat Dec 10 16:03:00 EET 2016 until: Fri Mar 10 16:03:00 EET 2017
Certificate fingerprints:
MD5: 83:65:97:9D:C5:AA:79:A8:50:38:C3:6C:62:E8:9D:04
SHA1: 69:7F:60:39:6B:E7:36:0E:CA:EE:97:A9:AD:77:60:01:DF:82:05:09
SHA256: 26:C6:60:3F:96:2D:38:A9:B7:9C:0B:4B:4A:A1:85:D4:D1:A2:B2:DF:78:8D:AD:6E:D1:F9:1F:C1:EA:53:73:BB
Signature algorithm name: SHA256withRSA
Version: 3
Extensions:
[...]
PolicyQualifierInfo: [
qualifierID: 1.3.6.1.5.5.7.2.2
qualifier:
0000: 30 81 9E 0C 81 9B 54 68 69 73 20 43 65 72 74 69 0.....This Certi
0010: 66 69 63 61 74 65 20 6D 61 79 20 6F 6E 6C 79 20 ficate may only
0020: 62 65 20 72 65 6C 69 65 64 20 75 70 6F 6E 20 62 be relied upon b
0030: 79 20 52 65 6C 79 69 6E 67 20 50 61 72 74 69 65 y Relying Partie
0040: 73 20 61 6E 64 20 6F 6E 6C 79 20 69 6E 20 61 63 s and only in ac
0050: 63 6F 72 64 61 6E 63 65 20 77 69 74 68 20 74 68 cordance with th
0060: 65 20 43 65 72 74 69 66 69 63 61 74 65 20 50 6F e Certificate Po
0070: 6C 69 63 79 20 66 6F 75 6E 64 20 61 74 20 68 74 licy found at ht
0080: 74 70 73 3A 2F 2F 6C 65 74 73 65 6E 63 72 79 70 tps://letsencryp
0090: 74 2E 6F 72 67 2F 72 65 70 6F 73 69 74 6F 72 79 t.org/repository
00A0: 2F /
[...]
In order to verify that the certificate is valid for the remote host, a simple search is performed on this string:
return CertificateFactory.getInstance("X.509").generateCertPath(certs).toString().contains("CN=" + hostname);
Since the hostname is something like bankname.ro, the searched string is CN=bankname.ro as a result. This is problematic, since you could get a certificate for bankname.ro.whatever.tk and it will still match. Note that some CAs would possibly flag this certificate request, but since we're not talking about a big US bank, it's unlikely that they will, and if they do, you can just try another one.
The application is using client certificates while communicating with the remote server. Unfortunately, the certificates are static files which are included for the production and development environments within the resources of the application. While the private keys of the certificates are encrypted, the decryption code is embedded for both in the logic of the application, in plain text for the development environment, and base64-encoded for the production one.
This is not particularly an issue, since knowing the private key of the client certificate won't make the connection any less secure, but it is still not a best practice, and an instance of CWE-259.
My first gut reaction was that this is a security theatre, however, I realized this would effectively prevent mass instances of MITM (such as by an enterprise firewall), since the firewall can establish a trusted connection with the device (given that the in-house CA is installed) but the bank servers would refuse the connection on their side, since it would be lacking the client certificate. While it's an effective way to defend against mass attacks, since the client certificates can be easily extracted, it does not prevent targeted attacks. Still, I applaud the bank's security team for coming up with this layer of protection.
The fix for the first vulnerability could be to replace the DO_NOT_VERIFY (which is not a constant, but rather a variable pointing to an instance of a class implementing HostnameVerifier without performing any actual checks) with another implementation of HostnameVerifier which does at least make sure the hostname is part of the bank's actual domain names:
static final class CustomHostnameVerifier implements HostnameVerifier {
public boolean verify(String hostname, SSLSession sslSession) {
return sslSession.isValid() && hostname.endsWith(".bankname.ro");
}
}
However, the best solution would be to not override this functionality, and just let the built-in TLS stack perform its checks.
Diffing the changed classes after the decompilation of the patched version, reveals the simple fixes used to neutralize these issues:
$ diff extracted_{2,1}.java
60c60,62
< this.connection.setHostnameVerifier(DO_NOT_VERIFY);
---
> if (!production) {
> this.connection.setHostnameVerifier(DO_NOT_VERIFY);
> }
104c106,108
< if (CertificateFactory.getInstance("X.509").generateCertPath(certs).toString().contains("CN=" + hostname))
---
> certPath = CertificateFactory.getInstance("X.509").generateCertPath(certs).toString();
> hostname = "CN=" + hostname;
> if ((certPath.contains(hostname + ",")) || (certPath.contains(hostname + "\n")))
The first fix suggests that the use of the all-accepting custom hostname verifier was never meant to work on production, only on development environments. Whoops.
The second fix resolves my immediate concerns, however, it is still performing full-text search on the whole certificate chain information. Somewhat unlikely, however, it might still be possible to trick some CA into issuing a certificate, which has a description (or any field) containing a string like CN=bankname.ro,.
In summation, the bank definitely tried to make the application secure, it just missed a few logic errors. However, the communication could have been better, as to this day, I was not emailed by them at all, the whole process happened silently.