From b6347cb862d6bdbd4ee45cb2385899ceeda22930 Mon Sep 17 00:00:00 2001 From: Bessie Jiang Date: Wed, 8 Jan 2025 14:20:35 +0000 Subject: [PATCH] Move DownloadManager.COLUMN_REASON conversion to failure reason into CTLogger This CL adds a new API that will convert the DownloadManager.COLUMN_REASON into the relevant proto failure reason enum value. The reason why it is a new API rather than reusing an existing one is because the parameter type is the same (int), so a separate API will keep it clearer when the DownloadManager value vs. proto enum value is directly used. Flag: com.android.net.ct.flags.certificate_transparency_service Bug: 378626065 Test: atest NetworkSecurityUnitTests Change-Id: I9028ba8f2cb95ccc6d21b19c58e0d15a9405c3f7 --- .../ct/CertificateTransparencyDownloader.java | 35 ++---------------- .../net/ct/CertificateTransparencyLogger.java | 10 ++++- .../ct/CertificateTransparencyLoggerImpl.java | 37 +++++++++++++++++++ ...CertificateTransparencyDownloaderTest.java | 21 +++++++---- 4 files changed, 63 insertions(+), 40 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 c679ab858a..764300ceba 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -16,16 +16,10 @@ package com.android.server.net.ct; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DEVICE_OFFLINE; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DOWNLOAD_CANNOT_RESUME; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_HTTP_ERROR; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_SIGNATURE_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_SIGNATURE_VERIFICATION; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_TOO_MANY_REDIRECTS; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_UNKNOWN; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_VERSION_ALREADY_EXISTS; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__PENDING_WAITING_FOR_WIFI; import android.annotation.RequiresApi; import android.app.DownloadManager; @@ -303,42 +297,19 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { mDataStore.getPropertyInt( Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0); - // HTTP Error - if (400 <= status.reason() && status.reason() <= 600) { + if (status.isHttpError()) { mLogger.logCTLogListUpdateFailedEvent( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_HTTP_ERROR, failureCount, status.reason()); } else { // TODO(b/384935059): handle blocked domain logging - mLogger.logCTLogListUpdateFailedEvent( - downloadStatusToFailureReason(status.reason()), failureCount); + mLogger.logCTLogListUpdateFailedEventWithDownloadStatus( + status.reason(), failureCount); } } } - /** Converts DownloadStatus reason into failure reason to log. */ - private int downloadStatusToFailureReason(int downloadStatusReason) { - switch (downloadStatusReason) { - case DownloadManager.PAUSED_WAITING_TO_RETRY: - case DownloadManager.PAUSED_WAITING_FOR_NETWORK: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DEVICE_OFFLINE; - case DownloadManager.ERROR_UNHANDLED_HTTP_CODE: - case DownloadManager.ERROR_HTTP_DATA_ERROR: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_HTTP_ERROR; - case DownloadManager.ERROR_TOO_MANY_REDIRECTS: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_TOO_MANY_REDIRECTS; - case DownloadManager.ERROR_CANNOT_RESUME: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DOWNLOAD_CANNOT_RESUME; - case DownloadManager.ERROR_INSUFFICIENT_SPACE: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE; - case DownloadManager.PAUSED_QUEUED_FOR_WIFI: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__PENDING_WAITING_FOR_WIFI; - default: - return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_UNKNOWN; - } - } - /** * Updates the data store with the current number of consecutive log list update failures. * 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 70fb1ae228..913c472aea 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java @@ -19,10 +19,18 @@ package com.android.server.net.ct; /** Interface with logging to statsd for Certificate Transparency. */ public interface CertificateTransparencyLogger { + /** + * Logs a CTLogListUpdateFailed event to statsd, when failure is provided by 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); + /** * Logs a CTLogListUpdateFailed event to statsd, when no HTTP error status code is present. * - * @param failureReason reason why the log list wasn't updated (e.g. DownloadManager failures) + * @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); 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 f660752661..b97a885773 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java @@ -17,10 +17,25 @@ package com.android.server.net.ct; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DEVICE_OFFLINE; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DOWNLOAD_CANNOT_RESUME; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_HTTP_ERROR; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_TOO_MANY_REDIRECTS; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_UNKNOWN; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__PENDING_WAITING_FOR_WIFI; + +import android.app.DownloadManager; /** Implementation for logging to statsd for Certificate Transparency. */ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger { + @Override + public void logCTLogListUpdateFailedEventWithDownloadStatus( + int downloadStatus, int failureCount) { + logCTLogListUpdateFailedEvent(downloadStatusToFailureReason(downloadStatus), failureCount); + } + @Override public void logCTLogListUpdateFailedEvent(int failureReason, int failureCount) { logCTLogListUpdateFailedEvent(failureReason, failureCount, /* httpErrorStatusCode= */ 0); @@ -37,4 +52,26 @@ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger ); } + /** Converts DownloadStatus reason into failure reason to log. */ + private int downloadStatusToFailureReason(int downloadStatusReason) { + switch (downloadStatusReason) { + case DownloadManager.PAUSED_WAITING_TO_RETRY: + case DownloadManager.PAUSED_WAITING_FOR_NETWORK: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DEVICE_OFFLINE; + case DownloadManager.ERROR_UNHANDLED_HTTP_CODE: + case DownloadManager.ERROR_HTTP_DATA_ERROR: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_HTTP_ERROR; + case DownloadManager.ERROR_TOO_MANY_REDIRECTS: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_TOO_MANY_REDIRECTS; + case DownloadManager.ERROR_CANNOT_RESUME: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_DOWNLOAD_CANNOT_RESUME; + case DownloadManager.ERROR_INSUFFICIENT_SPACE: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE; + case DownloadManager.PAUSED_QUEUED_FOR_WIFI: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__PENDING_WAITING_FOR_WIFI; + default: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_UNKNOWN; + } + } + } 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 555268996d..34f8dd1abd 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 @@ -16,7 +16,6 @@ package com.android.server.net.ct; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_SIGNATURE_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_SIGNATURE_VERIFICATION; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_VERSION_ALREADY_EXISTS; @@ -215,8 +214,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE, + .logCTLogListUpdateFailedEventWithDownloadStatus( + DownloadManager.ERROR_INSUFFICIENT_SPACE, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); } @@ -235,6 +234,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()).logCTLogListUpdateFailedEvent(anyInt(), anyInt()); + verify(mLogger, never()).logCTLogListUpdateFailedEventWithDownloadStatus( + anyInt(), anyInt()); } @Test @@ -284,8 +285,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE, + .logCTLogListUpdateFailedEventWithDownloadStatus( + DownloadManager.ERROR_INSUFFICIENT_SPACE, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); } @@ -306,6 +307,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()).logCTLogListUpdateFailedEvent(anyInt(), anyInt()); + verify(mLogger, never()).logCTLogListUpdateFailedEventWithDownloadStatus( + anyInt(), anyInt()); } @Test @@ -359,8 +362,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); verify(mLogger, times(1)) - .logCTLogListUpdateFailedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_FAILED__FAILURE_REASON__FAILURE_NO_DISK_SPACE, + .logCTLogListUpdateFailedEventWithDownloadStatus( + DownloadManager.ERROR_INSUFFICIENT_SPACE, Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD); } @@ -381,6 +384,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()).logCTLogListUpdateFailedEvent(anyInt(), anyInt()); + verify(mLogger, never()).logCTLogListUpdateFailedEventWithDownloadStatus( + anyInt(), anyInt()); } @Test @@ -594,6 +599,8 @@ public class CertificateTransparencyDownloaderTest { Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); verify(mLogger, never()).logCTLogListUpdateFailedEvent(anyInt(), anyInt()); + verify(mLogger, never()).logCTLogListUpdateFailedEventWithDownloadStatus( + anyInt(), anyInt()); } @Test