Fix certificate checker callback lifetime.
OpenSSL's SSL_CTX_set_verify() function allows us to set a callback
called after certificate validation but doesn't provide a way to pass
private data to this callback. CL:183832 was passing the pointer to the
CertificateChecker instance using a global pointer, nevertheless the
lifetime of this pointer was wrong since libcurl can trigger this
callback asynchronously when the SSL certificates are downloaded.
This patch converts the CertificateChecker into a singleton class and
uses the same trick previously used to pass the ServerToCheck value
using different callbacks.
Bug: 25818567
Test: Run an update on edison-userdebug; FEATURES=test emerge-link update_engine
Change-Id: I84cdb2f8c5ac86d1463634e73e867f213f7a2f5a
diff --git a/common/certificate_checker.h b/common/certificate_checker.h
index bc675be..c785192 100644
--- a/common/certificate_checker.h
+++ b/common/certificate_checker.h
@@ -78,6 +78,9 @@
enum class ServerToCheck {
kUpdate = 0,
kDownload,
+
+ // Ignore this server.
+ kNone,
};
// Responsible for checking whether update server certificates change, and
@@ -96,10 +99,8 @@
CertificateCheckResult result) = 0;
};
- CertificateChecker(PrefsInterface* prefs,
- OpenSSLWrapper* openssl_wrapper,
- ServerToCheck server_to_check);
- virtual ~CertificateChecker() = default;
+ CertificateChecker(PrefsInterface* prefs, OpenSSLWrapper* openssl_wrapper);
+ ~CertificateChecker();
// This callback is called by libcurl just before the initialization of an
// SSL connection after having processed all other SSL related options. Used
@@ -109,6 +110,11 @@
SSL_CTX* ssl_ctx,
void* cert_checker);
+ // Initialize this class as the singleton instance. Only one instance can be
+ // initialized at a time and it should be initialized before other methods
+ // can be used.
+ void Init();
+
// Set the certificate observer to the passed instance. To remove the
// observer, pass a nullptr. The |observer| instance must be valid while this
// CertificateChecker verifies certificates.
@@ -120,24 +126,37 @@
FRIEND_TEST(CertificateCheckerTest, ChangedCertificate);
FRIEND_TEST(CertificateCheckerTest, FailedCertificate);
- // This callback is called by openssl after initial SSL verification. It is
- // used to perform any additional security verification on the connection,
- // but we use them here to get hold of the server certificate, in order to
- // determine if it has changed since the last connection. Since openssl forces
- // us to do this statically, we pass a pointer to this instance via a
- // thread-safe global pointer. The assigned callback is then called once per
+ // These callbacks are asynchronously called by openssl after initial SSL
+ // verification. They are used to perform any additional security verification
+ // on the connection, but we use them here to get hold of the server
+ // certificate, in order to determine if it has changed since the last
+ // connection. Since openssl forces us to do this statically, we define two
+ // different callbacks for the two different official update servers, and only
+ // assign the correspondent one. The assigned callback is then called once per
// each certificate on the server and returns 1 for success and 0 for failure.
- static int VerifySSLCallback(int preverify_ok, X509_STORE_CTX* x509_ctx);
+ static int VerifySSLCallbackDownload(int preverify_ok,
+ X509_STORE_CTX* x509_ctx);
+ static int VerifySSLCallbackUpdate(int preverify_ok,
+ X509_STORE_CTX* x509_ctx);
+ static int VerifySSLCallback(int preverify_ok,
+ X509_STORE_CTX* x509_ctx,
+ ServerToCheck server_to_check);
// Checks if server certificate stored in |x509_ctx| has changed since last
- // connection to that same server, specified by the |server_to_check_| member.
+ // connection to that same server, specified by |server_to_check|.
// This is called by the callbacks defined above. The result of the
// certificate check is passed to the observer, if any. Returns true on
// success and false otherwise.
- bool CheckCertificateChange(int preverify_ok, X509_STORE_CTX* x509_ctx);
+ bool CheckCertificateChange(int preverify_ok,
+ X509_STORE_CTX* x509_ctx,
+ ServerToCheck server_to_check);
// Notifies the observer, if any, of a certificate checking.
- void NotifyCertificateChecked(CertificateCheckResult result);
+ void NotifyCertificateChecked(ServerToCheck server_to_check,
+ CertificateCheckResult result);
+
+ // The CertificateChecker singleton instance.
+ static CertificateChecker* cert_checker_singleton_;
// Prefs instance used to store the certificates seen in the past.
PrefsInterface* prefs_;
@@ -145,9 +164,6 @@
// The wrapper for openssl operations.
OpenSSLWrapper* openssl_wrapper_;
- // The server to check from this certificate checker instance.
- ServerToCheck server_to_check_;
-
// The observer called whenever a certificate is checked, if not null.
Observer* observer_{nullptr};
