Merge "Add checks for CT public key against allowlist" into main
diff --git a/networksecurity/service/Android.bp b/networksecurity/service/Android.bp
index d7aacdb..3c964e5 100644
--- a/networksecurity/service/Android.bp
+++ b/networksecurity/service/Android.bp
@@ -32,6 +32,7 @@
"framework-connectivity-pre-jarjar",
"service-connectivity-pre-jarjar",
"framework-statsd.stubs.module_lib",
+ "ServiceConnectivityResources",
],
static_libs: [
diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyJob.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyJob.java
index e6f1379..f1b9a4f 100644
--- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyJob.java
+++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyJob.java
@@ -38,6 +38,7 @@
private final DataStore mDataStore;
private final CertificateTransparencyDownloader mCertificateTransparencyDownloader;
private final CompatibilityVersion mCompatVersion;
+ private final SignatureVerifier mSignatureVerifier;
private final AlarmManager mAlarmManager;
private final PendingIntent mPendingIntent;
@@ -49,11 +50,13 @@
Context context,
DataStore dataStore,
CertificateTransparencyDownloader certificateTransparencyDownloader,
- CompatibilityVersion compatVersion) {
+ CompatibilityVersion compatVersion,
+ SignatureVerifier signatureVerifier) {
mContext = context;
mDataStore = dataStore;
mCertificateTransparencyDownloader = certificateTransparencyDownloader;
mCompatVersion = compatVersion;
+ mSignatureVerifier = signatureVerifier;
mAlarmManager = context.getSystemService(AlarmManager.class);
mPendingIntent =
@@ -127,6 +130,7 @@
private void startDependencies() {
mDataStore.load();
mCertificateTransparencyDownloader.addCompatibilityVersion(mCompatVersion);
+ mSignatureVerifier.loadAllowedKeys();
mContext.registerReceiver(
mCertificateTransparencyDownloader,
new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE),
@@ -139,6 +143,7 @@
private void stopDependencies() {
mContext.unregisterReceiver(mCertificateTransparencyDownloader);
+ mSignatureVerifier.clearAllowedKeys();
mCertificateTransparencyDownloader.clearCompatibilityVersions();
mDataStore.delete();
diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyService.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyService.java
index a71ff7c..2e910b2 100644
--- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyService.java
+++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyService.java
@@ -52,6 +52,7 @@
public CertificateTransparencyService(Context context) {
DataStore dataStore = new DataStore(Config.PREFERENCES_FILE);
+ SignatureVerifier signatureVerifier = new SignatureVerifier(context);
mCertificateTransparencyJob =
new CertificateTransparencyJob(
context,
@@ -60,13 +61,14 @@
context,
dataStore,
new DownloadHelper(context),
- new SignatureVerifier(context),
+ signatureVerifier,
new CertificateTransparencyLoggerImpl(dataStore)),
new CompatibilityVersion(
Config.COMPATIBILITY_VERSION,
Config.URL_SIGNATURE,
Config.URL_LOG_LIST,
- Config.CT_ROOT_DIRECTORY_PATH));
+ Config.CT_ROOT_DIRECTORY_PATH),
+ signatureVerifier);
}
/**
diff --git a/networksecurity/service/src/com/android/server/net/ct/PemReader.java b/networksecurity/service/src/com/android/server/net/ct/PemReader.java
new file mode 100644
index 0000000..56b3973
--- /dev/null
+++ b/networksecurity/service/src/com/android/server/net/ct/PemReader.java
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2025 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.server.net.ct;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.PublicKey;
+import java.security.spec.KeySpec;
+import java.security.spec.X509EncodedKeySpec;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Collection;
+
+/** Utility class to read keys in PEM format. */
+class PemReader {
+
+ private static final String BEGIN = "-----BEGIN";
+ private static final String END = "-----END";
+
+ /**
+ * Parse the provided input stream and return the list of keys from the stream.
+ *
+ * @param input the input stream
+ * @return the keys
+ */
+ public static Collection<PublicKey> readKeysFrom(InputStream input)
+ throws IOException, GeneralSecurityException {
+ KeyFactory instance = KeyFactory.getInstance("RSA");
+ Collection<PublicKey> keys = new ArrayList<>();
+
+ try (BufferedReader reader = new BufferedReader(new InputStreamReader(input))) {
+ String line = reader.readLine();
+ while (line != null) {
+ if (line.startsWith(BEGIN)) {
+ keys.add(instance.generatePublic(readNextKey(reader)));
+ } else {
+ throw new IOException("Unexpected line in the reader: " + line);
+ }
+ line = reader.readLine();
+ }
+ } catch (IllegalArgumentException e) {
+ throw new GeneralSecurityException("Invalid public key base64 encoding", e);
+ }
+
+ return keys;
+ }
+
+ private static KeySpec readNextKey(BufferedReader reader) throws IOException {
+ StringBuilder publicKeyBuilder = new StringBuilder();
+
+ String line = reader.readLine();
+ while (line != null) {
+ if (line.startsWith(END)) {
+ return new X509EncodedKeySpec(
+ Base64.getDecoder().decode(publicKeyBuilder.toString()));
+ } else {
+ publicKeyBuilder.append(line);
+ }
+ line = reader.readLine();
+ }
+
+ throw new IOException("Unexpected end of the reader");
+ }
+}
diff --git a/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java b/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java
index 6040ef6..87a4973 100644
--- a/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java
+++ b/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java
@@ -30,6 +30,9 @@
import androidx.annotation.VisibleForTesting;
+import com.android.connectivity.resources.R;
+import com.android.server.connectivity.ConnectivityResources;
+
import java.io.IOException;
import java.io.InputStream;
import java.security.GeneralSecurityException;
@@ -39,21 +42,39 @@
import java.security.Signature;
import java.security.spec.X509EncodedKeySpec;
import java.util.Base64;
+import java.util.HashSet;
import java.util.Optional;
+import java.util.Set;
/** Verifier of the log list signature. */
@RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM)
public class SignatureVerifier {
- private final Context mContext;
private static final String TAG = "SignatureVerifier";
+ private final Context mContext;
+
@NonNull private Optional<PublicKey> mPublicKey = Optional.empty();
+ private final Set<PublicKey> mAllowedKeys = new HashSet<>();
+
public SignatureVerifier(Context context) {
mContext = context;
}
+ void loadAllowedKeys() {
+ try (InputStream input =
+ new ConnectivityResources(mContext).get().openRawResource(R.raw.ct_public_keys)) {
+ mAllowedKeys.addAll(PemReader.readKeysFrom(input));
+ } catch (GeneralSecurityException | IOException e) {
+ Log.e(TAG, "Error loading public keys", e);
+ }
+ }
+
+ void clearAllowedKeys() {
+ mAllowedKeys.clear();
+ }
+
@VisibleForTesting
Optional<PublicKey> getPublicKey() {
return mPublicKey;
@@ -82,7 +103,11 @@
}
@VisibleForTesting
- void setPublicKey(PublicKey publicKey) {
+ void setPublicKey(PublicKey publicKey) throws GeneralSecurityException {
+ if (!mAllowedKeys.contains(publicKey)) {
+ // TODO(b/400704086): add logging for this failure.
+ throw new GeneralSecurityException("Public key not in allowlist");
+ }
mPublicKey = Optional.of(publicKey);
}
@@ -105,21 +130,18 @@
byte[] signatureBytes = signatureStream.readAllBytes();
statusBuilder.setSignature(new String(signatureBytes));
- try {
- byte[] decodedSigBytes = Base64.getDecoder().decode(signatureBytes);
- if (!verifier.verify(decodedSigBytes)) {
- // Leave the UpdateState as UNKNOWN_STATE if successful as there are other
- // potential failures past the signature verification step
- statusBuilder.setState(SIGNATURE_VERIFICATION_FAILED);
- }
- } catch (IllegalArgumentException e) {
- Log.w(TAG, "Invalid signature base64 encoding", e);
- statusBuilder.setState(SIGNATURE_INVALID);
- return statusBuilder.build();
+ if (!verifier.verify(Base64.getDecoder().decode(signatureBytes))) {
+ // Leave the UpdateState as UNKNOWN_STATE if successful as there are other
+ // potential failures past the signature verification step
+ statusBuilder.setState(SIGNATURE_VERIFICATION_FAILED);
}
+ } catch (IllegalArgumentException e) {
+ Log.w(TAG, "Invalid signature base64 encoding", e);
+ statusBuilder.setState(SIGNATURE_INVALID);
+ return statusBuilder.build();
} catch (InvalidKeyException e) {
- Log.e(TAG, "Signature invalid for log list verification", e);
+ Log.e(TAG, "Key invalid for log list verification", e);
statusBuilder.setState(SIGNATURE_INVALID);
return statusBuilder.build();
} catch (IOException | GeneralSecurityException e) {
@@ -135,4 +157,9 @@
return statusBuilder.build();
}
+
+ @VisibleForTesting
+ boolean addAllowedKey(PublicKey publicKey) {
+ return mAllowedKeys.add(publicKey);
+ }
}
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 2af0122..22dc6ab 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
@@ -109,6 +109,7 @@
mContext.getFilesDir());
prepareDownloadManager();
+ mSignatureVerifier.addAllowedKey(mPublicKey);
mDataStore.load();
mCertificateTransparencyDownloader.addCompatibilityVersion(mCompatVersion);
}
@@ -165,6 +166,22 @@
@Test
public void
+ testDownloader_publicKeyDownloadSuccess_publicKeyNotAllowed_doNotStartMetadataDownload()
+ throws Exception {
+ mCertificateTransparencyDownloader.startPublicKeyDownload();
+ PublicKey notAllowed = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPublic();
+
+ assertThat(mSignatureVerifier.getPublicKey()).isEmpty();
+ assertThat(mCertificateTransparencyDownloader.hasMetadataDownloadId()).isFalse();
+ mCertificateTransparencyDownloader.onReceive(
+ mContext, makePublicKeyDownloadCompleteIntent(writePublicKeyToFile(notAllowed)));
+
+ assertThat(mSignatureVerifier.getPublicKey()).isEmpty();
+ assertThat(mCertificateTransparencyDownloader.hasMetadataDownloadId()).isFalse();
+ }
+
+ @Test
+ public void
testDownloader_publicKeyDownloadSuccess_updatePublicKeyFail_doNotStartMetadataDownload()
throws Exception {
mCertificateTransparencyDownloader.startPublicKeyDownload();
@@ -197,8 +214,7 @@
}
@Test
- public void testDownloader_publicKeyDownloadFail_logsFailure()
- throws Exception {
+ public void testDownloader_publicKeyDownloadFail_logsFailure() throws Exception {
mCertificateTransparencyDownloader.startPublicKeyDownload();
mCertificateTransparencyDownloader.onReceive(
@@ -243,8 +259,7 @@
}
@Test
- public void testDownloader_metadataDownloadFail_logsFailure()
- throws Exception {
+ public void testDownloader_metadataDownloadFail_logsFailure() throws Exception {
mCertificateTransparencyDownloader.startMetadataDownload();
mCertificateTransparencyDownloader.onReceive(
@@ -294,8 +309,7 @@
}
@Test
- public void testDownloader_contentDownloadFail_logsFailure()
- throws Exception {
+ public void testDownloader_contentDownloadFail_logsFailure() throws Exception {
mCertificateTransparencyDownloader.startContentDownload(mCompatVersion);
mCertificateTransparencyDownloader.onReceive(
@@ -329,9 +343,8 @@
}
@Test
- public void
- testDownloader_contentDownloadSuccess_noPublicKeyFound_logsSingleFailure()
- throws Exception {
+ public void testDownloader_contentDownloadSuccess_noPublicKeyFound_logsSingleFailure()
+ throws Exception {
File logListFile = makeLogListFile("456");
File metadataFile = sign(logListFile);
mSignatureVerifier.setPublicKey(mPublicKey);
@@ -351,16 +364,17 @@
}
@Test
- public void
- testDownloader_contentDownloadSuccess_wrongSignatureAlgo_logsSingleFailure()
- throws Exception {
+ public void testDownloader_contentDownloadSuccess_wrongSignatureAlgo_logsSingleFailure()
+ throws Exception {
// Arrange
File logListFile = makeLogListFile("456");
File metadataFile = sign(logListFile);
// Set the key to be deliberately wrong by using diff algorithm
- KeyPairGenerator instance = KeyPairGenerator.getInstance("EC");
- mSignatureVerifier.setPublicKey(instance.generateKeyPair().getPublic());
+ PublicKey wrongAlgorithmKey =
+ KeyPairGenerator.getInstance("EC").generateKeyPair().getPublic();
+ mSignatureVerifier.addAllowedKey(wrongAlgorithmKey);
+ mSignatureVerifier.setPublicKey(wrongAlgorithmKey);
// Act
mCertificateTransparencyDownloader.startMetadataDownload();
@@ -377,16 +391,15 @@
}
@Test
- public void
- testDownloader_contentDownloadSuccess_signatureNotVerified_logsSingleFailure()
- throws Exception {
+ public void testDownloader_contentDownloadSuccess_signatureNotVerified_logsSingleFailure()
+ throws Exception {
// Arrange
File logListFile = makeLogListFile("456");
- File metadataFile = sign(logListFile);
+ mSignatureVerifier.setPublicKey(mPublicKey);
- // Set the key to be deliberately wrong by using diff key pair
+ // Sign the list with a disallowed key pair
KeyPairGenerator instance = KeyPairGenerator.getInstance("RSA");
- mSignatureVerifier.setPublicKey(instance.generateKeyPair().getPublic());
+ File metadataFile = sign(logListFile, instance.generateKeyPair().getPrivate());
// Act
mCertificateTransparencyDownloader.startMetadataDownload();
@@ -405,9 +418,7 @@
}
@Test
- public void
- testDownloader_contentDownloadSuccess_installFail_logsFailure()
- throws Exception {
+ public void testDownloader_contentDownloadSuccess_installFail_logsFailure() throws Exception {
File invalidLogListFile = writeToFile("not_a_json_log_list".getBytes());
File metadataFile = sign(invalidLogListFile);
mSignatureVerifier.setPublicKey(mPublicKey);
@@ -615,9 +626,14 @@
}
private File sign(File file) throws IOException, GeneralSecurityException {
+ return sign(file, mPrivateKey);
+ }
+
+ private File sign(File file, PrivateKey privateKey)
+ throws IOException, GeneralSecurityException {
File signatureFile = File.createTempFile("log_list-metadata", "sig");
Signature signer = Signature.getInstance("SHA256withRSA");
- signer.initSign(mPrivateKey);
+ signer.initSign(privateKey);
try (InputStream fileStream = new FileInputStream(file);
OutputStream outputStream = new FileOutputStream(signatureFile)) {
diff --git a/networksecurity/tests/unit/src/com/android/server/net/ct/PemReaderTest.java b/networksecurity/tests/unit/src/com/android/server/net/ct/PemReaderTest.java
new file mode 100644
index 0000000..08629db
--- /dev/null
+++ b/networksecurity/tests/unit/src/com/android/server/net/ct/PemReaderTest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2025 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.server.net.ct;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.junit.Assert.assertThrows;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyPairGenerator;
+import java.security.PublicKey;
+import java.util.Base64;
+
+/** Tests for the {@link PemReader}. */
+@RunWith(JUnit4.class)
+public class PemReaderTest {
+
+ @Test
+ public void testReadKeys_singleKey() throws GeneralSecurityException, IOException {
+ PublicKey key = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPublic();
+
+ assertThat(PemReader.readKeysFrom(toInputStream(key))).containsExactly(key);
+ }
+
+ @Test
+ public void testReadKeys_multipleKeys() throws GeneralSecurityException, IOException {
+ KeyPairGenerator instance = KeyPairGenerator.getInstance("RSA");
+ PublicKey key1 = instance.generateKeyPair().getPublic();
+ PublicKey key2 = instance.generateKeyPair().getPublic();
+
+ assertThat(PemReader.readKeysFrom(toInputStream(key1, key2))).containsExactly(key1, key2);
+ }
+
+ @Test
+ public void testReadKeys_notSupportedKeyType() throws GeneralSecurityException {
+ PublicKey key = KeyPairGenerator.getInstance("EC").generateKeyPair().getPublic();
+
+ assertThrows(
+ GeneralSecurityException.class, () -> PemReader.readKeysFrom(toInputStream(key)));
+ }
+
+ @Test
+ public void testReadKeys_notBase64EncodedKey() throws GeneralSecurityException {
+ InputStream inputStream =
+ new ByteArrayInputStream(
+ (""
+ + "-----BEGIN PUBLIC KEY-----\n"
+ + KeyPairGenerator.getInstance("RSA")
+ .generateKeyPair()
+ .getPublic()
+ .toString()
+ + "\n-----END PUBLIC KEY-----\n")
+ .getBytes());
+
+ assertThrows(GeneralSecurityException.class, () -> PemReader.readKeysFrom(inputStream));
+ }
+
+ @Test
+ public void testReadKeys_noPemBegin() throws GeneralSecurityException {
+ PublicKey key = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPublic();
+ String base64Key = Base64.getEncoder().encodeToString(key.getEncoded());
+ String pemNoBegin = base64Key + "\n-----END PUBLIC KEY-----\n";
+
+ assertThrows(
+ IOException.class,
+ () -> PemReader.readKeysFrom(new ByteArrayInputStream(pemNoBegin.getBytes())));
+ }
+
+ @Test
+ public void testReadKeys_noPemEnd() throws GeneralSecurityException {
+ PublicKey key = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPublic();
+ String base64Key = Base64.getEncoder().encodeToString(key.getEncoded());
+ String pemNoEnd = "-----BEGIN PUBLIC KEY-----\n" + base64Key;
+
+ assertThrows(
+ IOException.class,
+ () -> PemReader.readKeysFrom(new ByteArrayInputStream(pemNoEnd.getBytes())));
+ }
+
+ private InputStream toInputStream(PublicKey... keys) {
+ StringBuilder builder = new StringBuilder();
+
+ for (PublicKey key : keys) {
+ builder.append("-----BEGIN PUBLIC KEY-----\n")
+ .append(Base64.getEncoder().encodeToString(key.getEncoded()))
+ .append("\n-----END PUBLIC KEY-----\n");
+ }
+
+ return new ByteArrayInputStream(builder.toString().getBytes());
+ }
+}