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 1478fd1e95..45871dec8d 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -16,11 +16,6 @@ package com.android.server.net.ct; -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_PUBLIC_KEY_NOT_FOUND; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS; - import android.annotation.RequiresApi; import android.app.DownloadManager; import android.content.BroadcastReceiver; @@ -33,6 +28,7 @@ import android.util.Log; import androidx.annotation.VisibleForTesting; +import com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState; import com.android.server.net.ct.DownloadHelper.DownloadStatus; import java.io.IOException; @@ -231,19 +227,17 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { } boolean success = false; - int failureReason = -1; + CTLogListUpdateState failureReason = CTLogListUpdateState.UNKNOWN_STATE; try { 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 = CTLogListUpdateState.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 = CTLogListUpdateState.SIGNATURE_INVALID; 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,18 +247,16 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { Log.w(TAG, "Log list did not pass verification"); // Avoid logging failure twice - if (failureReason == -1) { + if (failureReason == CTLogListUpdateState.UNKNOWN_STATE) { updateFailureCount(); - failureReason = - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; + failureReason = CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED; } - if (failureReason != -1) { - mLogger.logCTLogListUpdateStateChangedEvent( - failureReason, - mDataStore.getPropertyInt( - Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)); - } + mLogger.logCTLogListUpdateStateChangedEvent( + failureReason, + mDataStore.getPropertyInt( + Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)); + return; } @@ -282,7 +274,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { } else { updateFailureCount(); mLogger.logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS, + CTLogListUpdateState.VERSION_ALREADY_EXISTS, mDataStore.getPropertyInt( Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)); } @@ -298,7 +290,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { if (status.isHttpError()) { mLogger.logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_HTTP_ERROR, + CTLogListUpdateState.HTTP_ERROR, failureCount, status.reason()); } else { 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 a6b15abef8..8d53983403 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java @@ -34,7 +34,7 @@ public interface CertificateTransparencyLogger { * @param failureReason reason why the log list wasn't updated * @param failureCount number of consecutive log list update failures */ - void logCTLogListUpdateStateChangedEvent(int failureReason, int failureCount); + void logCTLogListUpdateStateChangedEvent(CTLogListUpdateState failureReason, int failureCount); /** * Logs a CTLogListUpdateStateChanged event to statsd with an HTTP error status code. @@ -44,6 +44,20 @@ public interface CertificateTransparencyLogger { * @param httpErrorStatusCode if relevant, the HTTP error status code from DownloadManager */ void logCTLogListUpdateStateChangedEvent( - int failureReason, int failureCount, int httpErrorStatusCode); + CTLogListUpdateState failureReason, int failureCount, int httpErrorStatusCode); + /** + * Intermediate enum for use with CertificateTransparencyStatsLog. + * + * This enum primarily exists to avoid 100+ char line alert fatigue. + */ + enum CTLogListUpdateState { + UNKNOWN_STATE, + HTTP_ERROR, + PUBLIC_KEY_NOT_FOUND, + SIGNATURE_INVALID, + SIGNATURE_NOT_FOUND, + SIGNATURE_VERIFICATION_FAILED, + VERSION_ALREADY_EXISTS + } } \ 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 3f5d1aa9d9..6accdf84e6 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java @@ -21,8 +21,13 @@ 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_DOWNLOAD_CANNOT_RESUME; 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_NO_DISK_SPACE; +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; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_TOO_MANY_REDIRECTS; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_UNKNOWN; +import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS; import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__PENDING_WAITING_FOR_WIFI; import android.app.DownloadManager; @@ -34,17 +39,28 @@ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger public void logCTLogListUpdateStateChangedEventWithDownloadStatus( int downloadStatus, int failureCount) { logCTLogListUpdateStateChangedEvent( - downloadStatusToFailureReason(downloadStatus), failureCount); - } - - @Override - public void logCTLogListUpdateStateChangedEvent(int failureReason, int failureCount) { - logCTLogListUpdateStateChangedEvent( - failureReason, failureCount, /* httpErrorStatusCode= */ 0); + downloadStatusToFailureReason(downloadStatus), + failureCount, + /* httpErrorStatusCode= */ 0); } @Override public void logCTLogListUpdateStateChangedEvent( + CTLogListUpdateState failureReason, int failureCount) { + logCTLogListUpdateStateChangedEvent( + localEnumToStatsLogEnum(failureReason), + failureCount, + /* httpErrorStatusCode= */ 0); + } + + @Override + public void logCTLogListUpdateStateChangedEvent( + CTLogListUpdateState failureReason, int failureCount, int httpErrorStatusCode) { + logCTLogListUpdateStateChangedEvent( + localEnumToStatsLogEnum(failureReason), failureCount, httpErrorStatusCode); + } + + private void logCTLogListUpdateStateChangedEvent( int failureReason, int failureCount, int httpErrorStatusCode) { CertificateTransparencyStatsLog.write( CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED, @@ -76,4 +92,25 @@ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_UNKNOWN; } } + + /** Converts the local enum to the corresponding auto-generated one used by CTStatsLog. */ + private int localEnumToStatsLogEnum(CTLogListUpdateState updateState) { + switch (updateState) { + case HTTP_ERROR: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_HTTP_ERROR; + case PUBLIC_KEY_NOT_FOUND: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND; + case SIGNATURE_INVALID: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_INVALID; + case SIGNATURE_NOT_FOUND: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND; + case SIGNATURE_VERIFICATION_FAILED: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; + case VERSION_ALREADY_EXISTS: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS; + case UNKNOWN_STATE: + default: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__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 dc8e54bcc3..ec4d6beb91 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,11 +16,6 @@ package com.android.server.net.ct; -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_NOT_FOUND; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; -import static com.android.server.net.ct.CertificateTransparencyStatsLog.CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS; - import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -44,6 +39,8 @@ import android.net.Uri; import androidx.test.platform.app.InstrumentationRegistry; +import com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState; + import org.json.JSONException; import org.json.JSONObject; import org.junit.After; @@ -358,17 +355,19 @@ public class CertificateTransparencyDownloaderTest { .isEqualTo(1); verify(mLogger, times(1)) .logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND, + CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND, /* failureCount= */ 1); verify(mLogger, never()) .logCTLogListUpdateStateChangedEvent( - eq( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), + eq(CTLogListUpdateState.SIGNATURE_NOT_FOUND), anyInt()); verify(mLogger, never()) .logCTLogListUpdateStateChangedEvent( - eq( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION), + eq(CTLogListUpdateState.SIGNATURE_INVALID), + anyInt()); + verify(mLogger, never()) + .logCTLogListUpdateStateChangedEvent( + eq(CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED), anyInt()); } @@ -396,15 +395,22 @@ public class CertificateTransparencyDownloaderTest { mDataStore.getPropertyInt( Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0)) .isEqualTo(1); - verify(mLogger, never()) - .logCTLogListUpdateStateChangedEvent( - eq( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), - anyInt()); verify(mLogger, times(1)) .logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION, + CTLogListUpdateState.SIGNATURE_INVALID, /* failureCount= */ 1); + verify(mLogger, never()) + .logCTLogListUpdateStateChangedEvent( + eq(CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED), + anyInt()); + verify(mLogger, never()) + .logCTLogListUpdateStateChangedEvent( + eq(CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND), + anyInt()); + verify(mLogger, never()) + .logCTLogListUpdateStateChangedEvent( + eq(CTLogListUpdateState.SIGNATURE_NOT_FOUND), + anyInt()); } @Test @@ -433,17 +439,19 @@ public class CertificateTransparencyDownloaderTest { .isEqualTo(1); verify(mLogger, never()) .logCTLogListUpdateStateChangedEvent( - eq( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_NOT_FOUND), + eq(CTLogListUpdateState.SIGNATURE_NOT_FOUND), anyInt()); verify(mLogger, never()) .logCTLogListUpdateStateChangedEvent( - eq( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_PUBLIC_KEY_NOT_FOUND), + eq(CTLogListUpdateState.SIGNATURE_INVALID), + anyInt()); + verify(mLogger, never()) + .logCTLogListUpdateStateChangedEvent( + eq(CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND), anyInt()); verify(mLogger, times(1)) .logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION, + CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED, /* failureCount= */ 1); } @@ -467,7 +475,7 @@ public class CertificateTransparencyDownloaderTest { .isEqualTo(1); verify(mLogger, times(1)) .logCTLogListUpdateStateChangedEvent( - CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS, + CTLogListUpdateState.VERSION_ALREADY_EXISTS, /* failureCount= */ 1); }