Remove password file functionality
Adds very little security at the cost of lots of complexity.
Change-Id: I6cb94132e5afe977df5f0caefc2953f4d723449d
diff --git a/gatekeeper.cpp b/gatekeeper.cpp
index 305ceff..1f301d6 100644
--- a/gatekeeper.cpp
+++ b/gatekeeper.cpp
@@ -33,21 +33,15 @@
// Password handle does not match what is stored, generate new SecureID
GetRandom(&user_id, sizeof(secure_id_t));
} else {
- if (!ValidatePasswordFile(request.user_id, request.password_handle)) {
- response->error = ERROR_INVALID;
- return;
- } else {
- // Password handle matches password file
- password_handle_t *pw_handle =
- reinterpret_cast<password_handle_t *>(request.password_handle.buffer.get());
- if (!DoVerify(pw_handle, request.enrolled_password)) {
- // incorrect old password
- response->error = ERROR_INVALID;
- return;
- }
-
- user_id = pw_handle->user_id;
+ password_handle_t *pw_handle =
+ reinterpret_cast<password_handle_t *>(request.password_handle.buffer.get());
+ if (!DoVerify(pw_handle, request.enrolled_password)) {
+ // incorrect old password
+ response->error = ERROR_INVALID;
+ return;
}
+
+ user_id = pw_handle->user_id;
}
salt_t salt;
@@ -65,9 +59,6 @@
return;
}
-
- WritePasswordFile(request.user_id, password_handle);
-
response->SetEnrolledPasswordHandle(&password_handle);
}
@@ -79,7 +70,6 @@
return;
}
- secure_id_t user_id, authenticator_id;
password_handle_t *password_handle = reinterpret_cast<password_handle_t *>(
request.password_handle.buffer.get());
@@ -89,16 +79,8 @@
return;
}
- if (!ValidatePasswordFile(request.user_id, request.password_handle)) {
- // we don't allow access to keys if we can't validate the file.
- // we must allow this case to support authentication before we decrypt
- // /data, however.
- user_id = 0;
- authenticator_id = 0;
- } else {
- user_id = password_handle->user_id;
- authenticator_id = password_handle->authenticator_id;
- }
+ secure_id_t user_id = password_handle->user_id;
+ secure_id_t authenticator_id = password_handle->authenticator_id;
uint64_t timestamp = GetNanosecondsSinceBoot();
@@ -157,18 +139,6 @@
return memcmp_s(provided_handle.buffer.get(), expected_handle, sizeof(*expected_handle)) == 0;
}
-bool GateKeeper::ValidatePasswordFile(uint32_t uid, const SizedBuffer &provided_handle) {
- SizedBuffer stored_handle;
- ReadPasswordFile(uid, &stored_handle);
-
- if (!stored_handle.buffer.get() || stored_handle.length == 0) return false;
-
- // do we also verify the signature here?
- return stored_handle.length == provided_handle.length &&
- memcmp_s(stored_handle.buffer.get(), provided_handle.buffer.get(), stored_handle.length)
- == 0;
-}
-
void GateKeeper::MintAuthToken(UniquePtr<uint8_t> *auth_token, uint32_t *length,
uint32_t timestamp, secure_id_t user_id, secure_id_t authenticator_id) {
if (auth_token == NULL) return;
diff --git a/include/gatekeeper/gatekeeper.h b/include/gatekeeper/gatekeeper.h
index 8effc15..6b76198 100644
--- a/include/gatekeeper/gatekeeper.h
+++ b/include/gatekeeper/gatekeeper.h
@@ -137,16 +137,6 @@
const uint32_t length) const = 0;
/**
- * Write the password file to persistent storage.
- */
- virtual void ReadPasswordFile(uint32_t uid, SizedBuffer *password_file) const = 0;
-
- /**
- * Read the password file from persistent storage.
- */
- virtual void WritePasswordFile(uint32_t uid, const SizedBuffer &password_file) const = 0;
-
- /**
* Get the time since boot in nanoseconds.
*
* Should return 0 on error.
@@ -169,12 +159,6 @@
bool DoVerify(const password_handle_t *expected_handle, const SizedBuffer &password);
/**
- * Verifies that the provided handle matches byte-by-byte what was previously
- * stored as a result of a call to 'Enroll'
- */
- bool ValidatePasswordFile(uint32_t uid, const SizedBuffer &provided_handle);
-
- /**
* Populates password_handle with the data provided and computes HMAC.
*/
bool CreatePasswordHandle(SizedBuffer *password_handle, salt_t salt,
diff --git a/include/gatekeeper/soft_gatekeeper.h b/include/gatekeeper/soft_gatekeeper.h
index 2927cd7..c048d0d 100644
--- a/include/gatekeeper/soft_gatekeeper.h
+++ b/include/gatekeeper/soft_gatekeeper.h
@@ -29,15 +29,6 @@
namespace gatekeeper {
-/**
- * Convenience class for easily switching out a testing implementation
- */
-class GateKeeperFileIo {
-public:
- virtual ~GateKeeperFileIo() {}
- virtual void Write(const char *filename, const uint8_t *bytes, uint32_t length) = 0;
- virtual uint32_t Read(const char *filename, UniquePtr<uint8_t> *bytes) const = 0;
-};
class SoftGateKeeper : public GateKeeper {
public:
@@ -50,14 +41,12 @@
static const int MAX_UINT_32_CHARS = 11;
- SoftGateKeeper(GateKeeperFileIo *file_io) {
- file_io_ = file_io;
+ SoftGateKeeper() {
key_.reset(new uint8_t[SIGNATURE_LENGTH_BYTES]);
memset(key_.get(), 0, SIGNATURE_LENGTH_BYTES);
}
virtual ~SoftGateKeeper() {
- delete file_io_;
}
virtual void GetAuthTokenKey(const uint8_t **auth_token_key,
@@ -92,21 +81,6 @@
memset(signature, 0, signature_length);
}
- virtual void ReadPasswordFile(uint32_t uid, SizedBuffer *password_file) const {
- char buf[MAX_UINT_32_CHARS];
- sprintf(buf, "%u", uid);
- UniquePtr<uint8_t> password_buffer;
- uint32_t length = file_io_->Read(buf, &password_buffer);
- password_file->buffer.reset(password_buffer.release());
- password_file->length = length;
- }
-
- virtual void WritePasswordFile(uint32_t uid, const SizedBuffer &password_file) const {
- char buf[MAX_UINT_32_CHARS];
- sprintf(buf, "%u", uid);
- file_io_->Write(buf, password_file.buffer.get(), password_file.length);
- }
-
virtual uint64_t GetNanosecondsSinceBoot() const {
struct timespec time;
int res = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
@@ -114,7 +88,6 @@
return time.tv_nsec;
}
private:
- GateKeeperFileIo *file_io_;
UniquePtr<uint8_t> key_;
};
}
diff --git a/softgatekeeper/native_gatekeeper_file_io.h b/softgatekeeper/native_gatekeeper_file_io.h
deleted file mode 100644
index 6b29bde..0000000
--- a/softgatekeeper/native_gatekeeper_file_io.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright 2015 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.
- */
-
-#ifndef NATIVE_GATEKEEPER_FILE_IO_H
-#define NATIVE_GATEKEEPER_FILE_IO_H
-
-#include <gatekeeper/soft_gatekeeper.h>
-
-namespace gatekeeper {
-
-class NativeGateKeeperFileIo : public ::gatekeeper::GateKeeperFileIo {
-public:
- virtual void Write(const char *filename, const uint8_t *bytes, uint32_t length) {
- // TODO
- }
-
- virtual uint32_t Read(const char *filename, UniquePtr<uint8_t> *bytes) const {
- // TODO
- return 0;
- }
-private:
-};
-}
-
-#endif // NATIVE_GATEKEEPER_FILE_IO_H
diff --git a/softgatekeeper/soft_gatekeeper_device.cpp b/softgatekeeper/soft_gatekeeper_device.cpp
index 12bc9a1..a0fd057 100644
--- a/softgatekeeper/soft_gatekeeper_device.cpp
+++ b/softgatekeeper/soft_gatekeeper_device.cpp
@@ -15,7 +15,6 @@
*/
#include "soft_gatekeeper_device.h"
-#include "native_gatekeeper_file_io.h"
__attribute__((visibility("default")))
int softgatekeeper_device_open(const hw_module_t *module, const char *name, hw_device_t **device) {
@@ -30,7 +29,6 @@
return 0;
}
-
static struct hw_module_methods_t gatekeeper_module_methods = {
.open = softgatekeeper_device_open,
};
@@ -54,7 +52,7 @@
namespace gatekeeper {
SoftGateKeeperDevice::SoftGateKeeperDevice(const hw_module_t *module)
- : impl_(new SoftGateKeeper(new NativeGateKeeperFileIo())) {
+ : impl_(new SoftGateKeeper()) {
#if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__)
static_assert(std::is_standard_layout<SoftGateKeeperDevice>::value,
"SoftGateKeeperDevice must be standard layout");
diff --git a/tests/gatekeeper_test.cpp b/tests/gatekeeper_test.cpp
index e47198b..05554f6 100644
--- a/tests/gatekeeper_test.cpp
+++ b/tests/gatekeeper_test.cpp
@@ -30,32 +30,6 @@
using ::gatekeeper::AuthToken;
using ::gatekeeper::secure_id_t;
-class TestGateKeeperFileIo : public ::gatekeeper::GateKeeperFileIo {
-public:
- TestGateKeeperFileIo() {
- bytes_.length = 0;
- }
-
- virtual void Write(const char *filename, const uint8_t *bytes, uint32_t length) {
- bytes_.buffer.reset(new uint8_t[length]);
- memcpy(bytes_.buffer.get(), bytes, length);
- bytes_.length = length;
- }
-
- virtual uint32_t Read(const char *filename, UniquePtr<uint8_t> *bytes) const {
- if (!bytes_.buffer.get() || bytes_.length == 0) {
- bytes->reset();
- } else {
- bytes->reset(new uint8_t[bytes_.length]);
- memcpy(bytes->get(), bytes_.buffer.get(), bytes_.length);
- }
-
- return bytes_.length;
- }
-
- SizedBuffer bytes_;
-};
-
static void do_enroll(SoftGateKeeper &gatekeeper, EnrollResponse *response) {
SizedBuffer password;
@@ -68,14 +42,14 @@
}
TEST(GateKeeperTest, EnrollSuccess) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
EnrollResponse response;
do_enroll(gatekeeper, &response);
ASSERT_EQ(::gatekeeper::gatekeeper_error_t::ERROR_NONE, response.error);
}
TEST(GateKeeperTest, EnrollBogusData) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
SizedBuffer password;
EnrollResponse response;
@@ -87,7 +61,7 @@
}
TEST(GateKeeperTest, VerifySuccess) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
SizedBuffer provided_password;
EnrollResponse enroll_response;
@@ -114,36 +88,8 @@
ASSERT_NE((uint64_t) 0, auth_token->auxiliary_secure_user_id);
}
-TEST(GateKeeperTest, VerifyBadPwFile) {
- TestGateKeeperFileIo *fw = new TestGateKeeperFileIo();
- SoftGateKeeper gatekeeper(fw);
- SizedBuffer provided_password;
- EnrollResponse enroll_response;
-
- provided_password.buffer.reset(new uint8_t[16]);
- provided_password.length = 16;
- memset(provided_password.buffer.get(), 0, 16);
- do_enroll(gatekeeper, &enroll_response);
- ASSERT_EQ(::gatekeeper::gatekeeper_error_t::ERROR_NONE, enroll_response.error);
-
- VerifyRequest request(0, &enroll_response.enrolled_password_handle,
- &provided_password);
- VerifyResponse response;
- fw->bytes_.buffer.reset();
- gatekeeper.Verify(request, &response);
- ASSERT_EQ(::gatekeeper::gatekeeper_error_t::ERROR_NONE, response.error);
-
- AuthToken *auth_token =
- reinterpret_cast<AuthToken *>(response.auth_token.buffer.get());
-
- ASSERT_EQ((uint32_t) 0, auth_token->authenticator_id);
- ASSERT_NE(~((uint32_t) 0), auth_token->timestamp);
- ASSERT_EQ((uint64_t) 0, auth_token->root_secure_user_id);
- ASSERT_EQ((uint64_t) 0, auth_token->auxiliary_secure_user_id);
-}
-
TEST(GateKeeperTest, TrustedReEnroll) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
SizedBuffer provided_password;
EnrollResponse enroll_response;
SizedBuffer password_handle;
@@ -198,7 +144,7 @@
TEST(GateKeeperTest, UntrustedReEnroll) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
SizedBuffer provided_password;
EnrollResponse enroll_response;
@@ -243,7 +189,7 @@
TEST(GateKeeperTest, VerifyBogusData) {
- SoftGateKeeper gatekeeper(new TestGateKeeperFileIo());
+ SoftGateKeeper gatekeeper;
SizedBuffer provided_password;
SizedBuffer password_handle;
VerifyResponse response;