MemorizingTrustManager icon indicating copy to clipboard operation
MemorizingTrustManager copied to clipboard

Replace MemorizingActivity with a pluggable UI interface

Open ge0rg opened this issue 10 years ago • 5 comments

Some apps (F-Droid, ChatSecure, maybe others) want to have better control of when and how to ask the user about certificates, for example to suppress the dialog when connecting to a server for the first time (TOFU/POP approach).

For this, the startActivity() / interactResult() "API" is insufficient, so it needs to be replaced with a real API. MemorizingActivity will become the default / example implementation for how to use this interface.

Before this can be implemented, some things need to be specified:

  1. Blocking or asynchronous (callback) interface?
  2. Required abstraction level: somewhere between "provide raw CertificateException" and "provide translated string for the user"
  3. Which serialization mechanism to use for the data?
  4. How to ensure the API can not be abused by third-party apps (i.e. accepting a bad certificate through Intents)?

Please contribute to the discussion!

ge0rg avatar Sep 30 '14 18:09 ge0rg

  1. hmm, off the top of my head, I think either will work.
  2. how about providing a fully parsed certificate in a convenient java data structure?
  3. what data needs to be serialized?
  4. use java methods not Intents

eighthave avatar Sep 30 '14 18:09 eighthave

  1. Do you want the callbacks to happen on your UI thread or on your network thread?
  2. Does X509Certificate[] qualify as fully parsed? What kind of data structure do you want to get passed so you know what caused the certificate problem? Please consider that different Android versions used different types of CertificateException for a given problem.
  3. Whatever is the data structure for (2) needs to be serialized in a way so it can be passed from the network thread to the UI thread.
  4. Alright!

ge0rg avatar Sep 30 '14 18:09 ge0rg

For (1) I suppose "UI thread" is the right thing. Currently, I'm getting an Android Handler reference from context.getMainLooper() to run my UI code on - that might be "good enough" for you as well.

What would you think about the following proposal for (2):

interface TrustDecisionHandler {
    public void decideOnCertificateException(X509Certificate[] chain,
            CertificateException cause, int decisionId);
    // optionally separate API method for hostname validation:
    public void decideOnHostnameMismatch(X509Certificate[] chain,
            String hostname, int decisionId);
}

The result would be communicated via static MTM.interactResult(int decisionId, int decisionValue); with decisionValue being a value from MTMDecision, or possibly an enum.

It looks like (3) is a non-issue as long as we are living in the same process, which we are. I only used serialization in MTM for the startActivity() intent. That would move inside the example, making the above API possible without woes.

ge0rg avatar Sep 30 '14 19:09 ge0rg

For me it would also be sufficient if I could somehow turn an 'interactive mode' on and off. On a per connection basis. Where the interactive mode displays the dialog and the non-interactive mode just throws an InvalidCertifiacateException

iNPUTmice avatar Oct 10 '14 10:10 iNPUTmice

@iNPUTmice It looks like your use case can be easily implemented with the above interface.

Another problem I am pondering about is TOFU / pinning. The above API is "stateless" as it does not support information about what MTM stored previously for a given server name. Therefore, it does not allow the application to show a more severe warning screen when it encounters a self-signed certificate for a host that already has a (different) certificate known to MTM.

To support this use-case I'd either need to add more methods (think of decideOnCertificateExceptionAndAlsoTheServerHasANewCertNow()) or to add some kind of state reference to the above-mentioned methods (i.e. decideOnCertificateException(new_cert_chain, cause, known_good_cert_chain, decisionId)). Comments?

ge0rg avatar Dec 03 '14 17:12 ge0rg