From 802c8f1a4da53f634457e22ac67d9158aaf33fac Mon Sep 17 00:00:00 2001 From: Sandro Montanari Date: Wed, 9 Oct 2024 13:17:40 +0000 Subject: [PATCH] Move public key to a DeviceConfig flag Instead of hard-coding the public key value in code. Flag: com.android.net.ct.flags.certificate_transparency_service Bug: 319829948 Test: atest NetworkSecurityUnitTests Change-Id: Ided9a9402241d91cae3cee25f5511c3a881e087d --- .../ct/CertificateTransparencyDownloader.java | 51 ++++++++++--------- .../CertificateTransparencyFlagsListener.java | 30 +++++++++-- .../src/com/android/server/net/ct/Config.java | 1 + ...CertificateTransparencyDownloaderTest.java | 38 +++++++++++--- 4 files changed, 84 insertions(+), 36 deletions(-) diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java index b2ef3456bf..fd73b29276 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -15,6 +15,7 @@ */ package com.android.server.net.ct; +import android.annotation.NonNull; import android.annotation.RequiresApi; import android.app.DownloadManager; import android.content.BroadcastReceiver; @@ -31,10 +32,13 @@ import androidx.annotation.VisibleForTesting; import java.io.IOException; import java.io.InputStream; import java.security.GeneralSecurityException; +import java.security.InvalidKeyException; import java.security.KeyFactory; +import java.security.PublicKey; import java.security.Signature; import java.security.spec.X509EncodedKeySpec; import java.util.Base64; +import java.util.Optional; /** Helper class to download certificate transparency log files. */ @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) @@ -42,41 +46,23 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { private static final String TAG = "CertificateTransparencyDownloader"; - // TODO: move key to a DeviceConfig flag. - private static final byte[] PUBLIC_KEY_BYTES = - Base64.getDecoder() - .decode( - "MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAsu0BHGnQ++W2CTdyZyxv" - + "HHRALOZPlnu/VMVgo2m+JZ8MNbAOH2cgXb8mvOj8flsX/qPMuKIaauO+PwROMjiq" - + "fUpcFm80Kl7i97ZQyBDYKm3MkEYYpGN+skAR2OebX9G2DfDqFY8+jUpOOWtBNr3L" - + "rmVcwx+FcFdMjGDlrZ5JRmoJ/SeGKiORkbbu9eY1Wd0uVhz/xI5bQb0OgII7hEj+" - + "i/IPbJqOHgB8xQ5zWAJJ0DmG+FM6o7gk403v6W3S8qRYiR84c50KppGwe4YqSMkF" - + "bLDleGQWLoaDSpEWtESisb4JiLaY4H+Kk0EyAhPSb+49JfUozYl+lf7iFN3qRq/S" - + "IXXTh6z0S7Qa8EYDhKGCrpI03/+qprwy+my6fpWHi6aUIk4holUCmWvFxZDfixox" - + "K0RlqbFDl2JXMBquwlQpm8u5wrsic1ksIv9z8x9zh4PJqNpCah0ciemI3YGRQqSe" - + "/mRRXBiSn9YQBUPcaeqCYan+snGADFwHuXCd9xIAdFBolw9R9HTedHGUfVXPJDiF" - + "4VusfX6BRR/qaadB+bqEArF/TzuDUr6FvOR4o8lUUxgLuZ/7HO+bHnaPFKYHHSm+" - + "+z1lVDhhYuSZ8ax3T0C3FZpb7HMjZtpEorSV5ElKJEJwrhrBCMOD8L01EoSPrGlS" - + "1w22i9uGHMn/uGQKo28u7AsCAwEAAQ=="); - private final Context mContext; private final DataStore mDataStore; private final DownloadHelper mDownloadHelper; private final CertificateTransparencyInstaller mInstaller; - private final byte[] mPublicKey; + + @NonNull private Optional mPublicKey = Optional.empty(); @VisibleForTesting CertificateTransparencyDownloader( Context context, DataStore dataStore, DownloadHelper downloadHelper, - CertificateTransparencyInstaller installer, - byte[] publicKey) { + CertificateTransparencyInstaller installer) { mContext = context; mDataStore = dataStore; mDownloadHelper = downloadHelper; mInstaller = installer; - mPublicKey = publicKey; } CertificateTransparencyDownloader(Context context, DataStore dataStore) { @@ -84,8 +70,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { context, dataStore, new DownloadHelper(context), - new CertificateTransparencyInstaller(), - PUBLIC_KEY_BYTES); + new CertificateTransparencyInstaller()); } void registerReceiver() { @@ -98,6 +83,20 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { } } + void setPublicKey(String publicKey) throws GeneralSecurityException { + mPublicKey = + Optional.of( + KeyFactory.getInstance("RSA") + .generatePublic( + new X509EncodedKeySpec( + Base64.getDecoder().decode(publicKey)))); + } + + @VisibleForTesting + void resetPublicKey() { + mPublicKey = Optional.empty(); + } + void startMetadataDownload(String metadataUrl) { long downloadId = download(metadataUrl); if (downloadId == -1) { @@ -202,9 +201,11 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { } private boolean verify(Uri file, Uri signature) throws IOException, GeneralSecurityException { + if (!mPublicKey.isPresent()) { + throw new InvalidKeyException("Missing public key for signature verification"); + } Signature verifier = Signature.getInstance("SHA256withRSA"); - KeyFactory keyFactory = KeyFactory.getInstance("RSA"); - verifier.initVerify(keyFactory.generatePublic(new X509EncodedKeySpec(mPublicKey))); + verifier.initVerify(mPublicKey.get()); ContentResolver contentResolver = mContext.getContentResolver(); try (InputStream fileStream = contentResolver.openInputStream(file); diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyFlagsListener.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyFlagsListener.java index a263546fc6..914af06400 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyFlagsListener.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyFlagsListener.java @@ -23,6 +23,7 @@ import android.provider.DeviceConfig.Properties; import android.text.TextUtils; import android.util.Log; +import java.security.GeneralSecurityException; import java.util.concurrent.Executors; /** Listener class for the Certificate Transparency Phenotype flags. */ @@ -57,21 +58,35 @@ class CertificateTransparencyFlagsListener implements DeviceConfig.OnPropertiesC return; } + String newPublicKey = + DeviceConfig.getString( + Config.NAMESPACE_NETWORK_SECURITY, + Config.FLAG_PUBLIC_KEY, + /* defaultValue= */ ""); String newVersion = - DeviceConfig.getString(Config.NAMESPACE_NETWORK_SECURITY, Config.FLAG_VERSION, ""); + DeviceConfig.getString( + Config.NAMESPACE_NETWORK_SECURITY, + Config.FLAG_VERSION, + /* defaultValue= */ ""); String newContentUrl = DeviceConfig.getString( - Config.NAMESPACE_NETWORK_SECURITY, Config.FLAG_CONTENT_URL, ""); + Config.NAMESPACE_NETWORK_SECURITY, + Config.FLAG_CONTENT_URL, + /* defaultValue= */ ""); String newMetadataUrl = DeviceConfig.getString( - Config.NAMESPACE_NETWORK_SECURITY, Config.FLAG_METADATA_URL, ""); - if (TextUtils.isEmpty(newVersion) + Config.NAMESPACE_NETWORK_SECURITY, + Config.FLAG_METADATA_URL, + /* defaultValue= */ ""); + if (TextUtils.isEmpty(newPublicKey) + || TextUtils.isEmpty(newVersion) || TextUtils.isEmpty(newContentUrl) || TextUtils.isEmpty(newMetadataUrl)) { return; } if (Config.DEBUG) { + Log.d(TAG, "newPublicKey=" + newPublicKey); Log.d(TAG, "newVersion=" + newVersion); Log.d(TAG, "newContentUrl=" + newContentUrl); Log.d(TAG, "newMetadataUrl=" + newMetadataUrl); @@ -88,6 +103,13 @@ class CertificateTransparencyFlagsListener implements DeviceConfig.OnPropertiesC return; } + try { + mCertificateTransparencyDownloader.setPublicKey(newPublicKey); + } catch (GeneralSecurityException e) { + Log.e(TAG, "Error setting the public Key", e); + return; + } + // TODO: handle the case where there is already a pending download. mDataStore.setProperty(Config.VERSION_PENDING, newVersion); diff --git a/networksecurity/service/src/com/android/server/net/ct/Config.java b/networksecurity/service/src/com/android/server/net/ct/Config.java index 2a6b8e21c5..611a5c791f 100644 --- a/networksecurity/service/src/com/android/server/net/ct/Config.java +++ b/networksecurity/service/src/com/android/server/net/ct/Config.java @@ -40,6 +40,7 @@ final class Config { static final String FLAG_CONTENT_URL = FLAGS_PREFIX + "content_url"; static final String FLAG_METADATA_URL = FLAGS_PREFIX + "metadata_url"; static final String FLAG_VERSION = FLAGS_PREFIX + "version"; + static final String FLAG_PUBLIC_KEY = FLAGS_PREFIX + "public_key"; // properties static final String VERSION_PENDING = "version_pending"; diff --git a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java index a056c35498..1aad028018 100644 --- a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java +++ b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java @@ -48,9 +48,10 @@ import java.io.OutputStream; import java.security.GeneralSecurityException; import java.security.KeyPair; import java.security.KeyPairGenerator; -import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; +import java.security.PublicKey; import java.security.Signature; +import java.util.Base64; /** Tests for the {@link CertificateTransparencyDownloader}. */ @RunWith(JUnit4.class) @@ -60,18 +61,20 @@ public class CertificateTransparencyDownloaderTest { @Mock private CertificateTransparencyInstaller mCertificateTransparencyInstaller; private PrivateKey mPrivateKey; + private PublicKey mPublicKey; private Context mContext; private File mTempFile; private DataStore mDataStore; private CertificateTransparencyDownloader mCertificateTransparencyDownloader; @Before - public void setUp() throws IOException, NoSuchAlgorithmException { + public void setUp() throws IOException, GeneralSecurityException { MockitoAnnotations.initMocks(this); KeyPairGenerator instance = KeyPairGenerator.getInstance("RSA"); KeyPair keyPair = instance.generateKeyPair(); mPrivateKey = keyPair.getPrivate(); + mPublicKey = keyPair.getPublic(); mContext = InstrumentationRegistry.getInstrumentation().getContext(); mTempFile = File.createTempFile("datastore-test", ".properties"); @@ -80,16 +83,13 @@ public class CertificateTransparencyDownloaderTest { mCertificateTransparencyDownloader = new CertificateTransparencyDownloader( - mContext, - mDataStore, - mDownloadHelper, - mCertificateTransparencyInstaller, - keyPair.getPublic().getEncoded()); + mContext, mDataStore, mDownloadHelper, mCertificateTransparencyInstaller); } @After public void tearDown() { mTempFile.delete(); + mCertificateTransparencyDownloader.resetPublicKey(); } @Test @@ -155,6 +155,8 @@ public class CertificateTransparencyDownloaderTest { long metadataId = 123; File metadataFile = sign(logListFile); Uri metadataUri = Uri.fromFile(metadataFile); + mCertificateTransparencyDownloader.setPublicKey( + Base64.getEncoder().encodeToString(mPublicKey.getEncoded())); setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri); when(mCertificateTransparencyInstaller.install(any(), eq(version))).thenReturn(true); @@ -212,6 +214,28 @@ public class CertificateTransparencyDownloaderTest { assertThat(mDataStore.getProperty(Config.METADATA_URL)).isNull(); } + @Test + public void testDownloader_handleContentCompleteMissingVerificationPublicKey() + throws Exception { + String version = "666"; + long contentId = 666; + File logListFile = File.createTempFile("log_list", "json"); + Uri contentUri = Uri.fromFile(logListFile); + long metadataId = 123; + File metadataFile = sign(logListFile); + Uri metadataUri = Uri.fromFile(metadataFile); + + setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri); + + mCertificateTransparencyDownloader.onReceive( + mContext, makeDownloadCompleteIntent(contentId)); + + verify(mCertificateTransparencyInstaller, never()).install(any(), eq(version)); + assertThat(mDataStore.getProperty(Config.VERSION)).isNull(); + assertThat(mDataStore.getProperty(Config.CONTENT_URL)).isNull(); + assertThat(mDataStore.getProperty(Config.METADATA_URL)).isNull(); + } + private Intent makeDownloadCompleteIntent(long downloadId) { return new Intent(DownloadManager.ACTION_DOWNLOAD_COMPLETE) .putExtra(DownloadManager.EXTRA_DOWNLOAD_ID, downloadId);