Add logging of when the public key has invalid encoding or is not on the
allowlist Flag: com.android.net.ct.flags.certificate_transparency_service Fixes: 400704086 Test: atest NetworkSecurityUnitTests Change-Id: I76669d184731d873a944bf77534bb73381cb7ef0
This commit is contained in:
parent
d64a649df9
commit
bf4402fe67
6 changed files with 76 additions and 12 deletions
|
|
@ -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();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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<Integer> 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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue