Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dba8cb76 authored by Brian Carlstrom's avatar Brian Carlstrom
Browse files

b/2511635 Browser displays incorrect SSL cert information 

Two more cases of "View certificate" problems like b/2511635

One problem is that if there are multiple resources downloaded for a
page. In that case the mCertificate shown ends up being from the last
loaded resource instead of the main resource of the page. The solution
is to only set the certificate if the LoadListener is the
mIsMainResourceLoader as well as the mIsMainPageLoader.

A larger problem was the fact that the EventHandler.certificate
interface method (in this case the LoadListener.certificate
implementation) once per https connection instead of once per request
as was documented. That meant if an https connection was reused (which
happens frequently on login pages such as
https://www.google.com/accounts which use the POST -> redirect -> GET
idiom to avoid POST data page refresh warnings) then later pages never
were associated with an SslCertificate.

The solution was to change EventHandler.certificate to be called once
per request, specifcally before the request. This means we no longer
call the certificate method in the handleSslErrorRequest case, which
is okay because it includes the SslCertificate within the SslError and
that is what the BrowserActivity expects.

Change-Id: Icbd9bd98c89db82762d1d06de85e1cde2470300d
parent 9f3168b3

core/java/android/net/http/Connection.java

+6 −0
Original line number Diff line number Diff line
@@ -222,6 +222,12 @@ abstract class Connection { 
                        }

                    }



                    /* we have a connection, let the event handler

                     * know of any associated certificate,

                     * potentially none.

                     */

                    req.mEventHandler.certificate(mCertificate);



                    try {

                        /* FIXME: don't increment failure count if old

                           connection?  There should not be a penalty for

core/java/android/net/http/EventHandler.java

+2 −2
Original line number Diff line number Diff line
@@ -125,8 +125,8 @@ public interface EventHandler { 
    public void endData();



    /**

     * SSL certificate callback called every time a resource is

     * loaded via a secure connection

     * SSL certificate callback called before resource request is

     * made, which will be null for insecure connection.

     */

    public void certificate(SslCertificate certificate);

core/java/android/net/http/HttpsConnection.java

+1 −7
Original line number Diff line number Diff line
@@ -308,12 +308,6 @@ public class HttpsConnection extends Connection { 
        SslError error = CertificateChainValidator.getInstance().

            doHandshakeAndValidateServerCertificates(this, sslSock, mHost.getHostName());



        EventHandler eventHandler = req.getEventHandler();



        // Update the certificate info (to be consistent, it is better to do it

        // here, before we start handling SSL errors, if any)

        eventHandler.certificate(mCertificate);



        // Inform the user if there is a problem

        if (error != null) {

            // handleSslErrorRequest may immediately unsuspend if it wants to
@@ -325,7 +319,7 @@ public class HttpsConnection extends Connection { 
                mSuspended = true;

            }

            // don't hold the lock while calling out to the event handler

            boolean canHandle = eventHandler.handleSslErrorRequest(error);

            boolean canHandle = req.getEventHandler().handleSslErrorRequest(error);

            if(!canHandle) {

                throw new IOException("failed to handle "+ error);

            }

core/java/android/webkit/BrowserFrame.java

+4 −1
Original line number Diff line number Diff line
@@ -586,7 +586,10 @@ class BrowserFrame extends Handler { 
     * @param headers The http headers.

     * @param postData If the method is "POST" postData is sent as the request

     *                 body. Is null when empty.

     * @param cacheMode The cache mode to use when loading this resource.

     * @param postDataIdentifier If the post data contained form this is the form identifier, otherwise it is 0.

     * @param cacheMode The cache mode to use when loading this resource. See WebSettings.setCacheMode

     * @param mainResource True if the this resource is the main request, not a supporting resource

     * @param userGesture

     * @param synchronous True if the load is synchronous.

     * @return A newly created LoadListener object.

     */

core/java/android/webkit/LoadListener.java

+16 −10
Original line number Diff line number Diff line
@@ -121,6 +121,7 @@ class LoadListener extends Handler implements EventHandler { 


    // Does this loader correspond to the main-frame top-level page?

    private boolean mIsMainPageLoader;

    // Does this loader correspond to the main content (as opposed to a supporting resource)

    private final boolean mIsMainResourceLoader;

    private final boolean mUserGesture;
@@ -520,23 +521,28 @@ class LoadListener extends Handler implements EventHandler { 
    }



    /**

     * Implementation of certificate handler for EventHandler.

     * Called every time a resource is loaded via a secure

     * connection. In this context, can be called multiple

     * times if we have redirects

     * @param certificate The SSL certifcate

     * IMPORTANT: as this is called from network thread, can't call native

     * directly

     * Implementation of certificate handler for EventHandler. Called

     * before a resource is requested. In this context, can be called

     * multiple times if we have redirects

     *

     * IMPORTANT: as this is called from network thread, can't call

     * native directly

     *

     * @param certificate The SSL certifcate or null if the request

     * was not secure

     */

    public void certificate(SslCertificate certificate) {

        if (DebugFlags.LOAD_LISTENER) {

            Log.v(LOGTAG, "LoadListener.certificate: " + certificate);

        }

        sendMessageInternal(obtainMessage(MSG_SSL_CERTIFICATE, certificate));

    }



    // Handle the certificate on the WebCore thread.

    private void handleCertificate(SslCertificate certificate) {

        // if this is the top-most main-frame page loader

        if (mIsMainPageLoader) {

            // update the browser frame (ie, the main frame)

        // if this is main resource of the top frame

        if (mIsMainPageLoader && mIsMainResourceLoader) {

            // update the browser frame with certificate

            mBrowserFrame.certificate(certificate);

        }

    }