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 41b58fac59..a477cc8559 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -32,7 +32,6 @@ import com.android.server.net.ct.DownloadHelper.DownloadStatus; import java.io.IOException; import java.io.InputStream; -import java.security.GeneralSecurityException; import java.util.Collection; import java.util.Optional; @@ -151,13 +150,19 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { return; } + LogListUpdateStatus updateStatus; try { - mSignatureVerifier.setPublicKeyFrom(publicKeyUri); - } catch (GeneralSecurityException | IOException | IllegalArgumentException e) { + updateStatus = mSignatureVerifier.setPublicKeyFrom(publicKeyUri); + } catch (IOException e) { Log.e(TAG, "Error setting the public Key", e); return; } + if (!updateStatus.isPublicKeySet()) { + mLogger.logCTLogListUpdateStateChangedEvent(updateStatus); + return; + } + startMetadataDownload(); } diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java index 2a37d8f778..8d13e452b5 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java @@ -36,6 +36,8 @@ public interface CertificateTransparencyLogger { UNKNOWN_STATE, HTTP_ERROR, LOG_LIST_INVALID, + PUBLIC_KEY_INVALID, + PUBLIC_KEY_NOT_ALLOWED, PUBLIC_KEY_NOT_FOUND, SIGNATURE_INVALID, SIGNATURE_NOT_FOUND, diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java index f617523f71..12cdef32a2 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java @@ -22,6 +22,8 @@ import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFIC import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_HTTP_ERROR; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_LOG_LIST_INVALID; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_NO_DISK_SPACE; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_INVALID; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_ALLOWED; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_INVALID; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND; @@ -133,6 +135,10 @@ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_HTTP_ERROR; case LOG_LIST_INVALID: return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_LOG_LIST_INVALID; + case PUBLIC_KEY_INVALID: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_INVALID; + case PUBLIC_KEY_NOT_ALLOWED: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_ALLOWED; case PUBLIC_KEY_NOT_FOUND: return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND; case SIGNATURE_INVALID: diff --git a/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java b/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java index 3f9b762b4d..df32e42b69 100644 --- a/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java +++ b/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java @@ -15,6 +15,8 @@ */ package com.android.server.net.ct; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_INVALID; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_NOT_ALLOWED; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_INVALID; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_NOT_FOUND; @@ -41,6 +43,11 @@ public abstract class LogListUpdateStatus { abstract Optional downloadStatus(); + boolean isPublicKeySet() { + // Check that none of the public key setting failures have been set as the state + return state() != PUBLIC_KEY_INVALID && state() != PUBLIC_KEY_NOT_ALLOWED; + } + boolean isSignatureVerified() { // Check that none of the signature verification failures have been set as the state return state() != PUBLIC_KEY_NOT_FOUND 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 87a497383f..22953f42b4 100644 --- a/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java +++ b/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java @@ -15,6 +15,8 @@ */ package com.android.server.net.ct; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_INVALID; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_NOT_ALLOWED; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_INVALID; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_NOT_FOUND; @@ -84,28 +86,35 @@ public class SignatureVerifier { mPublicKey = Optional.empty(); } - void setPublicKeyFrom(Uri file) throws GeneralSecurityException, IOException { + LogListUpdateStatus setPublicKeyFrom(Uri file) throws IOException { try (InputStream fileStream = mContext.getContentResolver().openInputStream(file)) { - setPublicKey(new String(fileStream.readAllBytes())); + return setPublicKey(new String(fileStream.readAllBytes())); } } - void setPublicKey(String publicKey) throws GeneralSecurityException { + private LogListUpdateStatus setPublicKey(String publicKey) { byte[] decodedPublicKey = null; + LogListUpdateStatus.Builder statusBuilder = LogListUpdateStatus.builder(); + try { decodedPublicKey = Base64.getDecoder().decode(publicKey); - } catch (IllegalArgumentException e) { - throw new GeneralSecurityException("Invalid public key base64 encoding", e); - } - setPublicKey( + setPublicKey( KeyFactory.getInstance("RSA") .generatePublic(new X509EncodedKeySpec(decodedPublicKey))); + } catch (IllegalArgumentException e) { + statusBuilder.setState(PUBLIC_KEY_INVALID); + Log.w(TAG, "Invalid public key base64 encoding", e); + } catch (GeneralSecurityException e) { + statusBuilder.setState(PUBLIC_KEY_NOT_ALLOWED); + Log.e(TAG, "Public key not in allowlist", e); + } + + return statusBuilder.build(); } @VisibleForTesting 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); 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 956bad57cd..f5e3b7d685 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 @@ -182,6 +182,41 @@ public class CertificateTransparencyDownloaderTest { assertThat(mCertificateTransparencyDownloader.hasMetadataDownloadId()).isFalse(); } + @Test + public void + testDownloader_publicKeyDownloadSuccess_publicKeyNotAllowed_logsFailure() + throws Exception { + mCertificateTransparencyDownloader.startPublicKeyDownload(); + PublicKey notAllowed = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPublic(); + + mCertificateTransparencyDownloader.onReceive( + mContext, makePublicKeyDownloadCompleteIntent(writePublicKeyToFile(notAllowed))); + + verify(mLogger, times(1)) + .logCTLogListUpdateStateChangedEvent( + LogListUpdateStatus.builder() + .setState(CTLogListUpdateState.PUBLIC_KEY_NOT_ALLOWED) + .build()); + } + + @Test + public void + testDownloader_publicKeyDownloadSuccess_publicKeyInvalidEncoding_logsFailure() + throws Exception { + mCertificateTransparencyDownloader.startPublicKeyDownload(); + + mCertificateTransparencyDownloader.onReceive( + mContext, + makePublicKeyDownloadCompleteIntent( + writeToFile("i_am_not_a_base64_encoded_public_key".getBytes()))); + + verify(mLogger, times(1)) + .logCTLogListUpdateStateChangedEvent( + LogListUpdateStatus.builder() + .setState(CTLogListUpdateState.PUBLIC_KEY_INVALID) + .build()); + } + @Test public void testDownloader_publicKeyDownloadSuccess_updatePublicKeyFail_doNotStartMetadataDownload() @@ -216,7 +251,7 @@ public class CertificateTransparencyDownloaderTest { } @Test - public void testDownloader_publicKeyDownloadFail_logsFailure() throws Exception { + public void testDownloader_publicKeyDownloadFail_logsDownloadFailure() throws Exception { mCertificateTransparencyDownloader.startPublicKeyDownload(); mCertificateTransparencyDownloader.onReceive(