From c9cefdbe1c2c013f281f7610643556d12522d48b Mon Sep 17 00:00:00 2001 From: Bessie Jiang Date: Fri, 24 Jan 2025 13:33:41 +0000 Subject: [PATCH] Rename the CTLogger APIs to reflect the atom name change The atom no longer just represents log list update failures, and will instead be representing the entire log list update result regardless of success or failure. Flag: com.android.net.ct.flags.certificate_transparency_service Bug: 378626065 Test: atest NetworkSecurityUnitTests Change-Id: Ide254542aeb9c9657e79a154e53f651478bb1a54 --- .../ct/CertificateTransparencyDownloader.java | 17 +++++++------ .../net/ct/CertificateTransparencyLogger.java | 13 +++++----- .../ct/CertificateTransparencyLoggerImpl.java | 12 ++++++---- ...CertificateTransparencyDownloaderTest.java | 24 +++++++++---------- 4 files changed, 36 insertions(+), 30 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 c81b32b326..1478fd1e95 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -237,11 +237,13 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { success = mSignatureVerifier.verify(contentUri, metadataUri); } catch (MissingPublicKeyException e) { updateFailureCount(); - failureReason = CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND; + failureReason = + CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND; Log.e(TAG, "No public key found for log list verification", e); } catch (InvalidKeyException e) { updateFailureCount(); - failureReason = CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; + failureReason = + CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; Log.e(TAG, "Signature invalid for log list verification", e); } catch (IOException | GeneralSecurityException e) { Log.e(TAG, "Could not verify new log list", e); @@ -253,11 +255,12 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { // Avoid logging failure twice if (failureReason == -1) { updateFailureCount(); - failureReason = CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; + failureReason = + CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; } if (failureReason != -1) { - mLogger.logCTLogListUpdateFailedEvent( + mLogger.logCTLogListUpdateStateChangedEvent( failureReason, mDataStore.getPropertyInt( Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)); @@ -278,7 +281,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { mDataStore.store(); } else { updateFailureCount(); - mLogger.logCTLogListUpdateFailedEvent( + mLogger.logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS, mDataStore.getPropertyInt( Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)); @@ -294,13 +297,13 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0); if (status.isHttpError()) { - mLogger.logCTLogListUpdateFailedEvent( + mLogger.logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_HTTP_ERROR, failureCount, status.reason()); } else { // TODO(b/384935059): handle blocked domain logging - mLogger.logCTLogListUpdateFailedEventWithDownloadStatus( + mLogger.logCTLogListUpdateStateChangedEventWithDownloadStatus( status.reason(), failureCount); } } 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 913c472aea..a6b15abef8 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java @@ -20,29 +20,30 @@ package com.android.server.net.ct; public interface CertificateTransparencyLogger { /** - * Logs a CTLogListUpdateFailed event to statsd, when failure is provided by DownloadManager. + * Logs a CTLogListUpdateStateChanged event to statsd, when failure is from DownloadManager. * * @param downloadStatus DownloadManager failure status why the log list wasn't updated * @param failureCount number of consecutive log list update failures */ - void logCTLogListUpdateFailedEventWithDownloadStatus(int downloadStatus, int failureCount); + void logCTLogListUpdateStateChangedEventWithDownloadStatus( + int downloadStatus, int failureCount); /** - * Logs a CTLogListUpdateFailed event to statsd, when no HTTP error status code is present. + * Logs a CTLogListUpdateStateChanged event to statsd without a HTTP error status code. * * @param failureReason reason why the log list wasn't updated * @param failureCount number of consecutive log list update failures */ - void logCTLogListUpdateFailedEvent(int failureReason, int failureCount); + void logCTLogListUpdateStateChangedEvent(int failureReason, int failureCount); /** - * Logs a CTLogListUpdateFailed event to statsd, when an HTTP error status code is provided. + * Logs a CTLogListUpdateStateChanged event to statsd with an HTTP error status code. * * @param failureReason reason why the log list wasn't updated (e.g. DownloadManager failures) * @param failureCount number of consecutive log list update failures * @param httpErrorStatusCode if relevant, the HTTP error status code from DownloadManager */ - void logCTLogListUpdateFailedEvent( + void logCTLogListUpdateStateChangedEvent( int failureReason, int failureCount, int httpErrorStatusCode); } \ No newline at end of file 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 7499cce0f6..3f5d1aa9d9 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java @@ -31,18 +31,20 @@ import android.app.DownloadManager; class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger { @Override - public void logCTLogListUpdateFailedEventWithDownloadStatus( + public void logCTLogListUpdateStateChangedEventWithDownloadStatus( int downloadStatus, int failureCount) { - logCTLogListUpdateFailedEvent(downloadStatusToFailureReason(downloadStatus), failureCount); + logCTLogListUpdateStateChangedEvent( + downloadStatusToFailureReason(downloadStatus), failureCount); } @Override - public void logCTLogListUpdateFailedEvent(int failureReason, int failureCount) { - logCTLogListUpdateFailedEvent(failureReason, failureCount, /* httpErrorStatusCode= */ 0); + public void logCTLogListUpdateStateChangedEvent(int failureReason, int failureCount) { + logCTLogListUpdateStateChangedEvent( + failureReason, failureCount, /* httpErrorStatusCode= */ 0); } @Override - public void logCTLogListUpdateFailedEvent( + public void logCTLogListUpdateStateChangedEvent( int failureReason, int failureCount, int httpErrorStatusCode) { CertificateTransparencyStatsLog.write( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED, 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 8004a51db6..dc8e54bcc3 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 @@ -212,7 +212,7 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEventWithDownloadStatus( + .logCTLogListUpdateStateChangedEventWithDownloadStatus( DownloadManager.ERROR_INSUFFICIENT_SPACE, /* failureCount= */ 1); } @@ -261,7 +261,7 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEventWithDownloadStatus( + .logCTLogListUpdateStateChangedEventWithDownloadStatus( DownloadManager.ERROR_INSUFFICIENT_SPACE, /* failureCount= */ 1); } @@ -314,7 +314,7 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEventWithDownloadStatus( + .logCTLogListUpdateStateChangedEventWithDownloadStatus( DownloadManager.ERROR_INSUFFICIENT_SPACE, /* failureCount= */ 1); } @@ -357,16 +357,16 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND, /* failureCount= */ 1); verify(mLogger, never()) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( eq( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), anyInt()); verify(mLogger, never()) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( eq( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION), anyInt()); @@ -397,12 +397,12 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( eq( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), anyInt()); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION, /* failureCount= */ 1); } @@ -432,17 +432,17 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( eq( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), anyInt()); verify(mLogger, never()) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( eq( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND), anyInt()); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION, /* failureCount= */ 1); } @@ -466,7 +466,7 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( + .logCTLogListUpdateStateChangedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS, /* failureCount= */ 1); }