From f88d296d36abd371ef11d074bbb95e47f04e8ed0 Mon Sep 17 00:00:00 2001 From: Rich Cannings Date: Fri, 29 Mar 2019 15:42:09 -0700 Subject: DO NOT MERGE Add data integrity checking for wifi passwords Cherry pick of the following CLs to pi-dev: ag/6709482, ag/6841349, ag/6869071 Bug: 128318105 Test: Triggered a failure to exercise new code and ran tests/wifitests/runtests.sh Change-Id: I2e8e3b766727365be1055d04a52d0c96d84d6465 --- .../com/android/server/wifi/WifiConfigStore.java | 35 ++- .../server/wifi/util/DataIntegrityChecker.java | 331 +++++++++++++++++++++ .../android/server/wifi/util/EncryptedData.java | 48 +++ .../server/wifi/util/DataIntegrityCheckerTest.java | 88 ++++++ 4 files changed, 497 insertions(+), 5 deletions(-) create mode 100644 service/java/com/android/server/wifi/util/DataIntegrityChecker.java create mode 100644 service/java/com/android/server/wifi/util/EncryptedData.java create mode 100644 tests/wifitests/src/com/android/server/wifi/util/DataIntegrityCheckerTest.java diff --git a/service/java/com/android/server/wifi/WifiConfigStore.java b/service/java/com/android/server/wifi/WifiConfigStore.java index 17a6670ff..255e41228 100644 --- a/service/java/com/android/server/wifi/WifiConfigStore.java +++ b/service/java/com/android/server/wifi/WifiConfigStore.java @@ -29,6 +29,7 @@ import android.util.Xml; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.os.AtomicFile; import com.android.internal.util.FastXmlSerializer; +import com.android.server.wifi.util.DataIntegrityChecker; import com.android.server.wifi.util.XmlUtil; import org.xmlpull.v1.XmlPullParser; @@ -42,6 +43,7 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.DigestException; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -465,9 +467,10 @@ public class WifiConfigStore { /** * Class to encapsulate all file writes. This is a wrapper over {@link AtomicFile} to write/read - * raw data from the persistent file. This class provides helper methods to read/write the - * entire file into a byte array. - * This helps to separate out the processing/parsing from the actual file writing. + * raw data from the persistent file with integrity. This class provides helper methods to + * read/write the entire file into a byte array. + * This helps to separate out the processing, parsing, and integrity checking from the actual + * file writing. */ public static class StoreFile { /** @@ -486,10 +489,15 @@ public class WifiConfigStore { * Store the file name for setting the file permissions/logging purposes. */ private String mFileName; + /** + * The integrity file storing integrity checking data for the store file. + */ + private DataIntegrityChecker mDataIntegrityChecker; public StoreFile(File file) { mAtomicFile = new AtomicFile(file); mFileName = mAtomicFile.getBaseFile().getAbsolutePath(); + mDataIntegrityChecker = new DataIntegrityChecker(mFileName); } /** @@ -504,16 +512,31 @@ public class WifiConfigStore { /** * Read the entire raw data from the store file and return in a byte array. * - * @return raw data read from the file or null if the file is not found. + * @return raw data read from the file or null if the file is not found or the data has + * been altered. * @throws IOException if an error occurs. The input stream is always closed by the method * even when an exception is encountered. */ public byte[] readRawData() throws IOException { + byte[] bytes = null; try { - return mAtomicFile.readFully(); + bytes = mAtomicFile.readFully(); + // Check that the file has not been altered since last writeBufferedRawData() + if (!mDataIntegrityChecker.isOk(bytes)) { + Log.e(TAG, "Data integrity problem with file: " + mFileName); + return null; + } } catch (FileNotFoundException e) { return null; + } catch (DigestException e) { + // When integrity checking is introduced. The existing data will have no related + // integrity file for validation. Thus, we will assume the existing data is correct + // and immediately create the integrity file. + Log.i(TAG, "isOK() had no integrity data to check; thus vacuously " + + "true. Running update now."); + mDataIntegrityChecker.update(bytes); } + return bytes; } /** @@ -551,6 +574,8 @@ public class WifiConfigStore { } throw e; } + // There was a legitimate change and update the integrity checker. + mDataIntegrityChecker.update(mWriteData); // Reset the pending write data after write. mWriteData = null; } diff --git a/service/java/com/android/server/wifi/util/DataIntegrityChecker.java b/service/java/com/android/server/wifi/util/DataIntegrityChecker.java new file mode 100644 index 000000000..a55fb8eb3 --- /dev/null +++ b/service/java/com/android/server/wifi/util/DataIntegrityChecker.java @@ -0,0 +1,331 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi.util; + +import android.annotation.NonNull; +import android.os.SystemProperties; +import android.security.keystore.KeyGenParameterSpec; +import android.security.keystore.KeyProperties; +import android.text.TextUtils; +import android.util.Log; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.security.DigestException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.UnrecoverableEntryException; +import java.security.cert.CertificateException; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.KeyGenerator; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; +import javax.crypto.spec.GCMParameterSpec; + +/** + * Tools to provide integrity checking of byte arrays based on NIAP Common Criteria Protection + * Profile FCS_STG_EXT.3.1. + */ +public class DataIntegrityChecker { + private static final String TAG = "DataIntegrityChecker"; + + private static final String FILE_SUFFIX = ".encrypted-checksum"; + private static final String ALIAS_SUFFIX = ".data-integrity-checker-key"; + private static final String CIPHER_ALGORITHM = "AES/GCM/NoPadding"; + private static final String DIGEST_ALGORITHM = "SHA-256"; + private static final int GCM_TAG_LENGTH = 128; + private static final String KEY_STORE = "AndroidKeyStore"; + + /** + * When KEYSTORE_FAILURE_RETURN_VALUE is true, all cryptographic operation failures will not + * enforce security and {@link #isOk(byte[])} always return true. + */ + private static final boolean KEYSTORE_FAILURE_RETURN_VALUE = true; + + private final File mIntegrityFile; + + /** + * Construct a new integrity checker to update and check if/when a data file was altered + * outside expected conditions. + * + * @param integrityFilename The {@link File} path prefix for where the integrity data is stored. + * A file will be created in the name of integrityFile with the suffix + * {@link DataIntegrityChecker#FILE_SUFFIX} We recommend using the same + * path as the file for which integrity is performed on. + * @throws NullPointerException When integrity file is null or the empty string. + */ + public DataIntegrityChecker(@NonNull String integrityFilename) { + if (TextUtils.isEmpty(integrityFilename)) { + throw new NullPointerException("integrityFilename must not be null or the empty " + + "string"); + } + mIntegrityFile = new File(integrityFilename + FILE_SUFFIX); + } + + /** + * Computes a digest of a byte array, encrypt it, and store the result + * + * Call this method immediately before storing the byte array + * + * @param data The data desired to ensure integrity + */ + public void update(byte[] data) { + if (data == null || data.length < 1) { + Log.e(TAG, "No data to update."); + reportException(new Exception("No data to update")); + return; + } + byte[] digest = getDigest(data); + if (digest == null || digest.length < 1) { + return; + } + String alias = mIntegrityFile.getName() + ALIAS_SUFFIX; + EncryptedData integrityData = encrypt(digest, alias); + if (integrityData != null) { + writeIntegrityData(integrityData, mIntegrityFile); + } else { + reportException(new Exception("integrityData null upon update")); + } + } + + /** + * Check the integrity of a given byte array + * + * Call this method immediately before trusting the byte array. This method will return false + * when the byte array was altered since the last {@link #update(byte[])} + * call, when {@link #update(byte[])} has never been called, or if there is + * an underlying issue with the cryptographic functions or the key store. + * + * @param data The data to check if its been altered + * @throws DigestException The integrity mIntegrityFile cannot be read. Ensure + * {@link #isOk(byte[])} is called after {@link #update(byte[])}. Otherwise, consider the + * result vacuously true and immediately call {@link #update(byte[])}. + * @return true if the data was not altered since {@link #update(byte[])} was last called + */ + public boolean isOk(byte[] data) throws DigestException { + if (data == null || data.length < 1) { + return KEYSTORE_FAILURE_RETURN_VALUE; + } + byte[] currentDigest = getDigest(data); + if (currentDigest == null || currentDigest.length < 1) { + return KEYSTORE_FAILURE_RETURN_VALUE; + } + EncryptedData encryptedData = readIntegrityData(mIntegrityFile); + if (encryptedData == null) { + throw new DigestException("No stored digest is available to compare."); + } + byte[] storedDigest = decrypt(encryptedData); + if (storedDigest == null) { + return KEYSTORE_FAILURE_RETURN_VALUE; + } + return constantTimeEquals(storedDigest, currentDigest); + } + + private byte[] getDigest(byte[] data) { + try { + return MessageDigest.getInstance(DIGEST_ALGORITHM).digest(data); + } catch (NoSuchAlgorithmException e) { + Log.e(TAG, "getDigest could not find algorithm: " + DIGEST_ALGORITHM); + reportException(e); + return null; + } + } + + private EncryptedData encrypt(byte[] data, String keyAlias) { + EncryptedData encryptedData = null; + try { + Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM); + SecretKey secretKeyReference = getOrCreateSecretKey(keyAlias); + if (secretKeyReference != null) { + cipher.init(Cipher.ENCRYPT_MODE, secretKeyReference); + encryptedData = new EncryptedData(cipher.doFinal(data), cipher.getIV(), keyAlias); + } else { + reportException(new Exception("secretKeyReference is null.")); + } + } catch (NoSuchAlgorithmException e) { + Log.e(TAG, "encrypt could not find the algorithm: " + CIPHER_ALGORITHM); + reportException(e); + } catch (NoSuchPaddingException e) { + Log.e(TAG, "encrypt had a padding exception"); + reportException(e); + } catch (InvalidKeyException e) { + Log.e(TAG, "encrypt received an invalid key"); + reportException(e); + } catch (BadPaddingException e) { + Log.e(TAG, "encrypt had a padding problem"); + reportException(e); + } catch (IllegalBlockSizeException e) { + Log.e(TAG, "encrypt had an illegal block size"); + reportException(e); + } + return encryptedData; + } + + private byte[] decrypt(EncryptedData encryptedData) { + byte[] decryptedData = null; + try { + Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM); + GCMParameterSpec spec = new GCMParameterSpec(GCM_TAG_LENGTH, encryptedData.getIv()); + SecretKey secretKeyReference = getOrCreateSecretKey(encryptedData.getKeyAlias()); + if (secretKeyReference != null) { + cipher.init(Cipher.DECRYPT_MODE, secretKeyReference, spec); + decryptedData = cipher.doFinal(encryptedData.getEncryptedData()); + } + } catch (NoSuchAlgorithmException e) { + Log.e(TAG, "decrypt could not find cipher algorithm " + CIPHER_ALGORITHM); + reportException(e); + } catch (NoSuchPaddingException e) { + Log.e(TAG, "decrypt could not find padding algorithm"); + reportException(e); + } catch (IllegalBlockSizeException e) { + Log.e(TAG, "decrypt had a illegal block size"); + reportException(e); + } catch (BadPaddingException e) { + Log.e(TAG, "decrypt had bad padding"); + reportException(e); + } catch (InvalidKeyException e) { + Log.e(TAG, "decrypt had an invalid key"); + reportException(e); + } catch (InvalidAlgorithmParameterException e) { + Log.e(TAG, "decrypt had an invalid algorithm parameter"); + reportException(e); + } + return decryptedData; + } + + private SecretKey getOrCreateSecretKey(String keyAlias) { + SecretKey secretKey = null; + try { + KeyStore keyStore = KeyStore.getInstance(KEY_STORE); + keyStore.load(null); + if (keyStore.containsAlias(keyAlias)) { // The key exists in key store. Get the key. + KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry) keyStore + .getEntry(keyAlias, null); + if (secretKeyEntry != null) { + secretKey = secretKeyEntry.getSecretKey(); + } else { + reportException(new Exception("keystore contains the alias and the secret key " + + "entry was null")); + } + } else { // The key does not exist in key store. Create the key and store it. + KeyGenerator keyGenerator = KeyGenerator + .getInstance(KeyProperties.KEY_ALGORITHM_AES, KEY_STORE); + + KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(keyAlias, + KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .build(); + + keyGenerator.init(keyGenParameterSpec); + secretKey = keyGenerator.generateKey(); + } + } catch (CertificateException e) { + Log.e(TAG, "getOrCreateSecretKey had a certificate exception."); + reportException(e); + } catch (InvalidAlgorithmParameterException e) { + Log.e(TAG, "getOrCreateSecretKey had an invalid algorithm parameter"); + reportException(e); + } catch (IOException e) { + Log.e(TAG, "getOrCreateSecretKey had an IO exception."); + reportException(e); + } catch (KeyStoreException e) { + Log.e(TAG, "getOrCreateSecretKey cannot find the keystore: " + KEY_STORE); + reportException(e); + } catch (NoSuchAlgorithmException e) { + Log.e(TAG, "getOrCreateSecretKey cannot find algorithm"); + reportException(e); + } catch (NoSuchProviderException e) { + Log.e(TAG, "getOrCreateSecretKey cannot find crypto provider"); + reportException(e); + } catch (UnrecoverableEntryException e) { + Log.e(TAG, "getOrCreateSecretKey had an unrecoverable entry exception."); + reportException(e); + } + return secretKey; + } + + private void writeIntegrityData(EncryptedData encryptedData, File file) { + try (FileOutputStream fos = new FileOutputStream(file); + ObjectOutputStream oos = new ObjectOutputStream(fos)) { + oos.writeObject(encryptedData); + } catch (FileNotFoundException e) { + Log.e(TAG, "writeIntegrityData could not find the integrity file"); + reportException(e); + } catch (IOException e) { + Log.e(TAG, "writeIntegrityData had an IO exception"); + reportException(e); + } + } + + private EncryptedData readIntegrityData(File file) { + try (FileInputStream fis = new FileInputStream(file); + ObjectInputStream ois = new ObjectInputStream(fis)) { + return (EncryptedData) ois.readObject(); + } catch (FileNotFoundException e) { + Log.e(TAG, "readIntegrityData could not find integrity file"); + reportException(e); + } catch (IOException e) { + Log.e(TAG, "readIntegrityData had an IO exception"); + reportException(e); + } catch (ClassNotFoundException e) { + Log.e(TAG, "readIntegrityData could not find the class EncryptedData"); + reportException(e); + } + return null; + } + + private boolean constantTimeEquals(byte[] a, byte[] b) { + if (a == null && b == null) { + return true; + } + + if (a == null || b == null || a.length != b.length) { + return false; + } + + byte differenceAccumulator = 0; + for (int i = 0; i < a.length; ++i) { + differenceAccumulator |= a[i] ^ b[i]; + } + return (differenceAccumulator == 0); + } + + /* TODO(b/128526030): Remove this error reporting code upon resolving the bug. */ + private static final boolean REQUEST_BUG_REPORT = false; + private void reportException(Exception exception) { + Log.wtf(TAG, "An irrecoverable key store error was encountered. " + + "KEYSTORE_FAILURE_RETURN_VALUE is set to " + KEYSTORE_FAILURE_RETURN_VALUE); + if (REQUEST_BUG_REPORT) { + SystemProperties.set("dumpstate.options", "bugreportwifi"); + SystemProperties.set("ctl.start", "bugreport"); + } + } +} diff --git a/service/java/com/android/server/wifi/util/EncryptedData.java b/service/java/com/android/server/wifi/util/EncryptedData.java new file mode 100644 index 000000000..468f28ec0 --- /dev/null +++ b/service/java/com/android/server/wifi/util/EncryptedData.java @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi.util; + +import java.io.Serializable; + +/** + * A class to store data created by {@link DataIntegrityChecker}. + */ +public class EncryptedData implements Serializable { + private static final long serialVersionUID = 1337L; + + private byte[] mEncryptedData; + private byte[] mIv; + private String mKeyAlias; + + public EncryptedData(byte[] encryptedData, byte[] iv, String keyAlias) { + mEncryptedData = encryptedData; + mIv = iv; + mKeyAlias = keyAlias; + } + + public byte[] getEncryptedData() { + return mEncryptedData; + } + + public byte[] getIv() { + return mIv; + } + + public String getKeyAlias() { + return mKeyAlias; + } +} diff --git a/tests/wifitests/src/com/android/server/wifi/util/DataIntegrityCheckerTest.java b/tests/wifitests/src/com/android/server/wifi/util/DataIntegrityCheckerTest.java new file mode 100644 index 000000000..b7076988b --- /dev/null +++ b/tests/wifitests/src/com/android/server/wifi/util/DataIntegrityCheckerTest.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi.util; + +import static org.junit.Assert.*; + +import org.junit.Ignore; +import org.junit.Test; + +import java.io.File; +import java.security.DigestException; + +/** + * Unit tests for {@link com.android.server.wifi.util.DataIntegrityChecker}. + */ +public class DataIntegrityCheckerTest { + private static byte[] sGoodData = {1, 2, 3, 4}; + private static byte[] sBadData = {5, 6, 7, 8}; + + /** + * Verify that updating the integrity token with known data and alias will + * pass the integrity test. This test ensure the expected outcome for + * unedited data succeeds. + * + * @throws Exception + */ + @Test + @Ignore + public void testIntegrityWithKnownDataAndKnownAlias() throws Exception { + File integrityFile = File.createTempFile("testIntegrityWithKnownDataAndKnownAlias", + ".tmp"); + DataIntegrityChecker dataIntegrityChecker = new DataIntegrityChecker( + integrityFile.getParent()); + dataIntegrityChecker.update(sGoodData); + assertTrue(dataIntegrityChecker.isOk(sGoodData)); + } + + /** + * Verify that checking the integrity of unknown data and a known alias + * will fail the integrity test. This test ensure the expected failure for + * altered data, in fact, fails. + * + * + * @throws Exception + */ + @Test + @Ignore + public void testIntegrityWithUnknownDataAndKnownAlias() throws Exception { + File integrityFile = File.createTempFile("testIntegrityWithUnknownDataAndKnownAlias", + ".tmp"); + DataIntegrityChecker dataIntegrityChecker = new DataIntegrityChecker( + integrityFile.getParent()); + dataIntegrityChecker.update(sGoodData); + assertFalse(dataIntegrityChecker.isOk(sBadData)); + } + + /** + * Verify a corner case where integrity of data that has never been + * updated passes and adds the token to the keystore. + * + * @throws Exception + */ + @Test(expected = DigestException.class) + @Ignore + public void testIntegrityWithoutUpdate() throws Exception { + File tmpFile = File.createTempFile("testIntegrityWithoutUpdate", ".tmp"); + + DataIntegrityChecker dataIntegrityChecker = new DataIntegrityChecker( + tmpFile.getAbsolutePath()); + + // the integrity data is not known, so isOk throws a DigestException + assertTrue(dataIntegrityChecker.isOk(sGoodData)); + } +} -- cgit v1.2.3