From 0bf58dac22519c5b7321eb620377062f77e86715 Mon Sep 17 00:00:00 2001 From: Bessie Jiang Date: Thu, 27 Mar 2025 18:34:39 +0000 Subject: [PATCH] Log when an IOException when reading the public key, content or metadata file for CT signature verification Flag: com.android.net.ct.flags.certificate_transparency_service Bug: 378626065 Test: atest NetworkSecurityUnitTests Change-Id: I2df5933f4cb39b82620f8bdd391ca800dfcf8ede --- .../ct/CertificateTransparencyDownloader.java | 9 +---- .../net/ct/CertificateTransparencyLogger.java | 1 + .../ct/CertificateTransparencyLoggerImpl.java | 3 ++ .../server/net/ct/LogListUpdateStatus.java | 8 +++- .../server/net/ct/SignatureVerifier.java | 12 +++++- ...CertificateTransparencyDownloaderTest.java | 39 +++++++++++++++++++ 6 files changed, 60 insertions(+), 12 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 a477cc8559..e608417fcc 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java @@ -150,14 +150,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver { return; } - LogListUpdateStatus updateStatus; - try { - updateStatus = mSignatureVerifier.setPublicKeyFrom(publicKeyUri); - } catch (IOException e) { - Log.e(TAG, "Error setting the public Key", e); - return; - } - + LogListUpdateStatus updateStatus = mSignatureVerifier.setPublicKeyFrom(publicKeyUri); if (!updateStatus.isPublicKeySet()) { mLogger.logCTLogListUpdateStateChangedEvent(updateStatus); return; 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 8d13e452b5..b9c6d33533 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLogger.java @@ -43,6 +43,7 @@ public interface CertificateTransparencyLogger { SIGNATURE_NOT_FOUND, SIGNATURE_VERIFICATION_FAILED, SUCCESS, + UNABLE_TO_READ_FILE, 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 12cdef32a2..b551d870d1 100644 --- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java +++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyLoggerImpl.java @@ -29,6 +29,7 @@ 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_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_UNABLE_TO_READ_FILE; 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; @@ -149,6 +150,8 @@ class CertificateTransparencyLoggerImpl implements CertificateTransparencyLogger return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_SIGNATURE_VERIFICATION; case SUCCESS: return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__SUCCESS; + case UNABLE_TO_READ_FILE: + return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_UNABLE_TO_READ_FILE; case VERSION_ALREADY_EXISTS: return CERTIFICATE_TRANSPARENCY_LOG_LIST_UPDATE_STATE_CHANGED__UPDATE_STATUS__FAILURE_VERSION_ALREADY_EXISTS; case UNKNOWN_STATE: 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 df32e42b69..8b27529b45 100644 --- a/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java +++ b/networksecurity/service/src/com/android/server/net/ct/LogListUpdateStatus.java @@ -22,6 +22,7 @@ import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListU import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SUCCESS; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.UNABLE_TO_READ_FILE; import com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState; @@ -45,7 +46,9 @@ public abstract class LogListUpdateStatus { 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; + return state() != PUBLIC_KEY_INVALID + && state() != PUBLIC_KEY_NOT_ALLOWED + && state() != UNABLE_TO_READ_FILE; } boolean isSignatureVerified() { @@ -53,7 +56,8 @@ public abstract class LogListUpdateStatus { return state() != PUBLIC_KEY_NOT_FOUND && state() != SIGNATURE_INVALID && state() != SIGNATURE_NOT_FOUND - && state() != SIGNATURE_VERIFICATION_FAILED; + && state() != SIGNATURE_VERIFICATION_FAILED + && state() != UNABLE_TO_READ_FILE; } boolean hasSignature() { 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 22953f42b4..42bba035fb 100644 --- a/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java +++ b/networksecurity/service/src/com/android/server/net/ct/SignatureVerifier.java @@ -21,6 +21,7 @@ import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListU import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_INVALID; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_NOT_FOUND; import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.SIGNATURE_VERIFICATION_FAILED; +import static com.android.server.net.ct.CertificateTransparencyLogger.CTLogListUpdateState.UNABLE_TO_READ_FILE; import android.annotation.NonNull; import android.annotation.RequiresApi; @@ -86,9 +87,12 @@ public class SignatureVerifier { mPublicKey = Optional.empty(); } - LogListUpdateStatus setPublicKeyFrom(Uri file) throws IOException { + LogListUpdateStatus setPublicKeyFrom(Uri file) { try (InputStream fileStream = mContext.getContentResolver().openInputStream(file)) { return setPublicKey(new String(fileStream.readAllBytes())); + } catch (IOException e) { + Log.e(TAG, "Could not read the public key file", e); + return LogListUpdateStatus.builder().setState(UNABLE_TO_READ_FILE).build(); } } @@ -153,7 +157,11 @@ public class SignatureVerifier { Log.e(TAG, "Key invalid for log list verification", e); statusBuilder.setState(SIGNATURE_INVALID); return statusBuilder.build(); - } catch (IOException | GeneralSecurityException e) { + } catch (IOException e) { + Log.e(TAG, "Could not read log list file", e); + statusBuilder.setState(UNABLE_TO_READ_FILE); + return statusBuilder.build(); + } catch (GeneralSecurityException e) { Log.e(TAG, "Could not verify new log list", e); statusBuilder.setState(SIGNATURE_VERIFICATION_FAILED); return statusBuilder.build(); 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 f5e3b7d685..0983f8b801 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,24 @@ public class CertificateTransparencyDownloaderTest { assertThat(mCertificateTransparencyDownloader.hasMetadataDownloadId()).isFalse(); } + @Test + public void testDownloader_publicKeyDownloadSuccess_publicKeyFileNotRead_logsFailure() + throws Exception { + mCertificateTransparencyDownloader.startPublicKeyDownload(); + + File publicKeyFile = writePublicKeyToFile(mPublicKey); + // Set the public key file to not be readable to simulate an IOException being thrown + publicKeyFile.setReadable(false); + mCertificateTransparencyDownloader.onReceive( + mContext, makePublicKeyDownloadCompleteIntent(publicKeyFile)); + + verify(mLogger, times(1)) + .logCTLogListUpdateStateChangedEvent( + LogListUpdateStatus.builder() + .setState(CTLogListUpdateState.UNABLE_TO_READ_FILE) + .build()); + } + @Test public void testDownloader_publicKeyDownloadSuccess_publicKeyNotAllowed_logsFailure() @@ -400,6 +418,27 @@ public class CertificateTransparencyDownloaderTest { .isEqualTo(CTLogListUpdateState.PUBLIC_KEY_NOT_FOUND); } + @Test + public void testDownloader_contentDownloadSuccess_signatureFileNotRead_logsSingleFailure() + throws Exception { + File logListFile = makeLogListFile("456"); + File metadataFile = sign(logListFile); + mSignatureVerifier.setPublicKey(mPublicKey); + mCertificateTransparencyDownloader.startMetadataDownload(); + + mCertificateTransparencyDownloader.onReceive( + mContext, makeMetadataDownloadCompleteIntent(mCompatVersion, metadataFile)); + // Set the log list file to not be readable to simulate an IOException being thrown + logListFile.setReadable(false); + mCertificateTransparencyDownloader.onReceive( + mContext, makeContentDownloadCompleteIntent(mCompatVersion, logListFile)); + + verify(mLogger, times(1)) + .logCTLogListUpdateStateChangedEvent(mUpdateStatusCaptor.capture()); + assertThat(mUpdateStatusCaptor.getValue().state()) + .isEqualTo(CTLogListUpdateState.UNABLE_TO_READ_FILE); + } + @Test public void testDownloader_contentDownloadSuccess_wrongSignatureAlgo_logsSingleFailure() throws Exception {