From 84522169bed37020285a7484067b64c4161d8c5b Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Thu, 7 Mar 2019 18:58:50 +0000 Subject: Revert "DO NOT MERGE Add data integrity checking for wifi passwords" This reverts commit fe21aeee563b916a738c2a21e824e9ecf4b03416. Reason for revert: b/127655135. This might cause devices to discard saved wifi networks stored on disk. Bug: 127655135 Change-Id: I4628098a91e5906f9e66ca333b36e9da4c560e76 --- .../com/android/server/wifi/WifiConfigStore.java | 35 +-- .../server/wifi/util/DataIntegrityChecker.java | 284 --------------------- .../android/server/wifi/util/EncryptedData.java | 48 ---- .../server/wifi/util/DataIntegrityCheckerTest.java | 84 ------ 4 files changed, 5 insertions(+), 446 deletions(-) delete mode 100644 service/java/com/android/server/wifi/util/DataIntegrityChecker.java delete mode 100644 service/java/com/android/server/wifi/util/EncryptedData.java delete 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 255e41228..17a6670ff 100644 --- a/service/java/com/android/server/wifi/WifiConfigStore.java +++ b/service/java/com/android/server/wifi/WifiConfigStore.java @@ -29,7 +29,6 @@ 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; @@ -43,7 +42,6 @@ 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; @@ -467,10 +465,9 @@ 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 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. + * 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. */ public static class StoreFile { /** @@ -489,15 +486,10 @@ 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); } /** @@ -512,31 +504,16 @@ 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 or the data has - * been altered. + * @return raw data read from the file or null if the file is not found. * @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 { - 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; - } + return mAtomicFile.readFully(); } 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; } /** @@ -574,8 +551,6 @@ 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 deleted file mode 100644 index b00552947..000000000 --- a/service/java/com/android/server/wifi/util/DataIntegrityChecker.java +++ /dev/null @@ -1,284 +0,0 @@ -/* - * 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.security.keystore.KeyGenParameterSpec; -import android.security.keystore.KeyProperties; -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"; - - private 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) throws NullPointerException { - if (integrityFilename == null || integrityFilename.equals("")) { - throw new NullPointerException("integrityFilename must not be null or the empty " - + "string"); - } else { - mIntegrityFile = new File(integrityFilename + FILE_SUFFIX); - } - } - - /** - * Compute 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 || mIntegrityFile == null) { - return; - } - byte[] digest = getDigest(data); - if (digest == null) { - return; - } - String alias = mIntegrityFile.getName() + ALIAS_SUFFIX; - EncryptedData integrityData = encrypt(digest, alias); - if (integrityData != null) { - writeIntegrityData(integrityData, mIntegrityFile); - } - } - - /** - * 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 || mIntegrityFile == null) { - return false; - } - byte[] currentDigest = getDigest(data); - if (currentDigest == null) { - return false; - } - 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 false; - } - return constantTimeEquals(storedDigest, currentDigest); - } - - private static 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); - 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); - } - } catch (NoSuchAlgorithmException e) { - Log.e(TAG, "encrypt could not find the algorithm: " + CIPHER_ALGORITHM); - } catch (NoSuchPaddingException e) { - Log.e(TAG, "encrypt had a padding exception"); - } catch (InvalidKeyException e) { - Log.e(TAG, "encrypt received an invalid key"); - } catch (BadPaddingException e) { - Log.e(TAG, "encrypt had a padding problem"); - } catch (IllegalBlockSizeException e) { - Log.e(TAG, "encrypt had an illegal block size"); - } - return encryptedData; - } - - private static 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); - } catch (NoSuchPaddingException e) { - Log.e(TAG, "decrypt could not find padding algorithm"); - } catch (IllegalBlockSizeException e) { - Log.e(TAG, "decrypt had a illegal block size"); - } catch (BadPaddingException e) { - Log.e(TAG, "decrypt had bad padding"); - } catch (InvalidKeyException e) { - Log.e(TAG, "decrypt had an invalid key"); - } catch (InvalidAlgorithmParameterException e) { - Log.e(TAG, "decrypt had an invalid algorithm parameter"); - } - return decryptedData; - } - - private static 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 { // 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."); - } catch (InvalidAlgorithmParameterException e) { - Log.e(TAG, "getOrCreateSecretKey had an invalid algorithm parameter"); - } catch (IOException e) { - Log.e(TAG, "getOrCreateSecretKey had an IO exception."); - } catch (KeyStoreException e) { - Log.e(TAG, "getOrCreateSecretKey cannot find the keystore: " + KEY_STORE); - } catch (NoSuchAlgorithmException e) { - Log.e(TAG, "getOrCreateSecretKey cannot find algorithm"); - } catch (NoSuchProviderException e) { - Log.e(TAG, "getOrCreateSecretKey cannot find crypto provider"); - } catch (UnrecoverableEntryException e) { - Log.e(TAG, "getOrCreateSecretKey had an unrecoverable entry exception."); - } - return secretKey; - } - - private static void writeIntegrityData(EncryptedData encryptedData, File file) { - try { - FileOutputStream fos = new FileOutputStream(file); - ObjectOutputStream oos = new ObjectOutputStream(fos); - oos.writeObject(encryptedData); - oos.close(); - } catch (FileNotFoundException e) { - Log.e(TAG, "writeIntegrityData could not find the integrity file"); - } catch (IOException e) { - Log.e(TAG, "writeIntegrityData had an IO exception"); - } - } - - private static EncryptedData readIntegrityData(File file) { - EncryptedData encryptedData = null; - try { - FileInputStream fis = new FileInputStream(file); - ObjectInputStream ois = new ObjectInputStream(fis); - encryptedData = (EncryptedData) ois.readObject(); - } catch (FileNotFoundException e) { - Log.e(TAG, "readIntegrityData could not find integrity file"); - } catch (IOException e) { - Log.e(TAG, "readIntegrityData had an IO exception"); - } catch (ClassNotFoundException e) { - Log.e(TAG, "readIntegrityData could not find the class EncryptedData"); - } - return encryptedData; - } - - 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); - } -} diff --git a/service/java/com/android/server/wifi/util/EncryptedData.java b/service/java/com/android/server/wifi/util/EncryptedData.java deleted file mode 100644 index 468f28ec0..000000000 --- a/service/java/com/android/server/wifi/util/EncryptedData.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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 deleted file mode 100644 index 5e4f6cc0d..000000000 --- a/tests/wifitests/src/com/android/server/wifi/util/DataIntegrityCheckerTest.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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.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 - 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 - 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) - 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