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

Cert pinning #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cert pinning #4

wants to merge 3 commits into from

Conversation

rkdarst
Copy link
Contributor

@rkdarst rkdarst commented Sep 23, 2016

Hi,

This is the work by @arashbm to do SSL cert pinning like the Android aware client. It is for an older version and would take some time to rebase (and probably not all is needed), however, I'm putting it here so you can see.

With this, with latest development tools (though I'm not sure what iOS version), we could pin the certificate in the same form as Android, with no advance loading of the certificate while developing the app.

Here is a partial quote from @arashbm, hopefully he can reply with more details:

I still haven't got to see anything specifically technical about iOS10 changes in
this respect but the general atmosphere is that apple is encouraging or
even looking at mandating a certain level of pinning. To quote from iOS
security page https://developer.apple.com/security/:

Strong encryption for your network connections is not enough. To help
ensure your app is connecting to the right server, employ Apple’s
certificate trust APIs and Certificate Transparency.

Then there is a link to this documentation on how to resolve trust status
https://developer.apple.com/library/prerelease/content/documentation/Security/Conceptual/CertKeyTrustProgGuide/iPhone_Tasks/iPhone_Tasks.html#//apple_ref/doc/uid/TP40001358-CH208-SW13 .

What do you think? Can you see if this code is still valid on iOS 10, and maybe even forward port it faster?

Thanks,

  • Richard

@tetujin
Copy link
Owner

tetujin commented Sep 25, 2016

@arashbm and @rkdarst ,

Thank you very much for your pull request.

I tried your source code. However, it is not working on iOS10.
The reason is that, after iOS10, all of the "HTTP" connection are banned by iOS. Your code uses HTTP connection for downloading CRT file, so that the connection is failed.

For excepting HTTP connection and communicating with a self-signed server, we have to add domain names of the servers to Info.plit with "NSExceptionDomains" key.
https://github.com/tetujin/aware-client-ios/blob/master/AWARE/Info.plist
https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/CocoaKeys.html

Also, after version 1.8.9, the app can communicate with AWARE servers without manually installing CRT file.

Thank you for your contribution.
Best regards,
Yuuki

@rkdarst
Copy link
Contributor Author

rkdarst commented Sep 25, 2016

Hi,

So, it seems that the root cause is that the Android AWARE protocol won't work - because iOS can't do the first GET /public/server.crt over HTTP. Is this a correct analysis?

As work-arounds, could it get the certificate over a different https connection that is signed, and then use that here?

Or, could it get the certificate over a HTTPS self-signed connection, using the trust framework to explicitely ignore the signing, and then use that for future connections?

I even considered that the certificate could be embedded within the QR code, but that gets to be a lot of data.

Could you propose any type of new API for getting the certificate? I can modify my server so that it does it. One thing I have wanted to do is to add query parameters to the study url, such as "https://api.abcd.com?crt=<cert_url>". This way, arbitrary other data could be passed to the app when scanning the URL, and we can handle extending the protocol more easily. What do you think?

Thanks,

  • Richard

@tetujin
Copy link
Owner

tetujin commented Sep 26, 2016

Hi @rkdarst ,

So, it seems that the root cause is that the Android AWARE protocol won't work - because iOS can't do the first GET /public/server.crt over HTTP. Is this a correct analysis?

Yes. We should make a new function for iOS client.

I guess that predicting the URL (for downloading .crt) is difficult from parameters to the study URL. So, we should add a new function to a QRcode reader for downloading a .crt file from other servers through "https."

For example,

  1. Make a QR-code for .crt file. e.g., "https://api.abcd.com/hoge/xxx.crt".
  2. If the QR-code reader on AWARE client detects a QR-code which is for *.crt file, aware client downloads it.

Best regards,
Yuuki

@arashbm
Copy link

arashbm commented Sep 26, 2016

@tetujin That certainly solves the problem with getting crt file but there should still be a way to make sure the file is not mangled with in the process before we can actually validate the connection. That could be solved by including a sha1 hash of the file somewhere in the QR code and checking it in the joining process or by showing the sha1 hash in the study joining page and asking the user to confirm it before joining the study (like how it is done when you are connecting to an ssh server for the first time). @rkdarst What do you think about this?

@arashbm
Copy link

arashbm commented Sep 26, 2016

Despite all this, since I cannot currently test on iOS10, I have to confirm that the certificate pinning part still works as expected. That is, the API used to extract certificate from the challenge (e.g. here). I just tested this on latest iOS 9 update and it works here.

@tetujin
Copy link
Owner

tetujin commented Sep 26, 2016

Hi @arashbm,

I would like to confirm your idea.

In you idea, the QR code has public key(.crt) information with URL for a study?

https://aware.ht.sfc.keio.ac.jp/index.php/webservice/index/STUDY_ID/API_KEY?crt=PUBLIC_KEY_INFORMATION

Maximally, QR can store 4296 alphanumerics.
http://www.qrcode.com/en/about/version.html

Also, I counted a length of the URL experimentally. The length is almost 1200 characters.
In this sense, maybe we can use this idea on iOS10.

Thanks,
Yuuki

@rkdarst
Copy link
Contributor Author

rkdarst commented Sep 26, 2016

Hi,

This would be very nice. I'll work on adding support to the Android one, but I may not get to it for some time.

Once we do this, we can have these keys (do we start with the first?):

  • crt=: the raw certificate, URLencoded as normal in query parameter
  • crt_url=: A URL for downloading certificate, for iOS purposes should be SSL with valid signature!
  • crt_sha256=: hash of certiicate file (probably used with crt_url=, but could be used with either).

With Android, the trick is that this URL used to be used directly as the webservice_server URL. For QRcode scanning, these parameters should be chopped off. Previously, you could manually join a study by directly entering the URL into webservice server and then activating. In this case, we need a new way to "manually join study by entering URL/code" which handles the URL parameters, which I'll add. Does iOS have/need something like this too? (One of our use cases may have people open the study site on their phone, and they need to join... can't scan the QR code that's on your own phone screen, but you can copy and paste URL). @denzilferreira, any comments? This would also help for the "join by code" we discussed.

Thanks,

  • Richard

@denzilferreira
Copy link

Hum... this is an interesting approach to get the certificates bundled within the QRCodes. I vote for support on Android side too.

@rkdarst
Copy link
Contributor Author

rkdarst commented Oct 2, 2016

By the way: this is almost done for Android. I have implemented all three parameters, crt, crt_sha256, crt_url. Use crt for raw cert data, otherwise use crt_url to download it. In all cases, verify against crt_sha256. I kind of prefer the crt_url method since the qrcode is much more readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants