summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFilip Skrzeczkowski <f.skrzeczkow@samsung.com>2023-08-23 15:01:00 +0200
committerTomasz Swierczek <t.swierczek@samsung.com>2023-11-29 11:07:23 +0100
commitb0af7f7ec35cf04eebbb768d3852fe4a81048761 (patch)
tree56464d7faf5de0b35e874e7015185a4c1671b8a3
parent581d0dfd29a485c286e7a3ff383bf89fddb7bfa8 (diff)
downloadsecurity-manager-b0af7f7ec35cf04eebbb768d3852fe4a81048761.tar.gz
security-manager-b0af7f7ec35cf04eebbb768d3852fe4a81048761.tar.bz2
security-manager-b0af7f7ec35cf04eebbb768d3852fe4a81048761.zip
Implement permissible file integrity verification
Permissible files have a SHA-1 hash attached at the beginning. Upon opening it is compared with a new hash calculated from the file content in order to verify if the file's integrity is intact. An error is thrown should the hashes differ. Files with no hash are still supported but reading/updating the rules stored in them will cause them to automatically switch to the new system. Change-Id: I5ec379b58cc78e63bcde084ada43273237d61beb
-rw-r--r--src/client/client-label-monitor.cpp3
-rw-r--r--src/common/include/permissible-set.h22
-rw-r--r--src/common/permissible-set.cpp95
-rw-r--r--src/include/security-manager-types.h3
-rw-r--r--test/CMakeLists.txt2
-rw-r--r--test/test_permissible-set.cpp256
6 files changed, 377 insertions, 4 deletions
diff --git a/src/client/client-label-monitor.cpp b/src/client/client-label-monitor.cpp
index 6b40e052..19583761 100644
--- a/src/client/client-label-monitor.cpp
+++ b/src/client/client-label-monitor.cpp
@@ -87,6 +87,9 @@ static lib_retcode apply_relabel_list(const std::string &global_label_file,
} catch (PermissibleSet::PermissibleSetException::FileReadError &e) {
LogError("Failed to read the configuration files");
return SECURITY_MANAGER_ERROR_UNKNOWN;
+ } catch (PermissibleSet::PermissibleSetException::FileIntegrityError &e) {
+ LogError("File integrity verification failed - the file may be corrupted");
+ return SECURITY_MANAGER_ERROR_FILE_INTEGRITY;
}
return SECURITY_MANAGER_SUCCESS;
}
diff --git a/src/common/include/permissible-set.h b/src/common/include/permissible-set.h
index 9c2889ae..aaa6d5b0 100644
--- a/src/common/include/permissible-set.h
+++ b/src/common/include/permissible-set.h
@@ -48,6 +48,7 @@ public:
DECLARE_EXCEPTION_TYPE(Base, FileWriteError)
DECLARE_EXCEPTION_TYPE(Base, FileInitError)
DECLARE_EXCEPTION_TYPE(Base, FileRemoveError)
+ DECLARE_EXCEPTION_TYPE(Base, FileIntegrityError)
};
/**
@@ -61,11 +62,31 @@ public:
std::string getPermissibleFileLocation(uid_t uid, int installationType);
/**
+ * Calculate a SHA-1 hash of given string
+ *
+ * @param[in] content content string to be hashed
+ * @return a 40 char long string that contains the hash encoded in hex
+ */
+std::string calculateHash(const std::string& content);
+
+/**
+ * Hash the file content and compare the result with the hash at the beginning
+ * of the file (if present) in order to detect possible corruption.
+ * @throws FileReadError
+ * @throws FileIntegrityError
+ *
+ * @param[in] nameFile path to the labels file
+ * @param[in] fstream open ifstream from the labe;s file
+ */
+void verifyFileIntegrity(const std::string &nameFile, std::ifstream &fstream);
+
+/**
* Update permissible file, 1st removing some labels from the list and then,
* adding new labels to the list (in this particular order).
* @throws FileLockError
* @throws FileOpenError
* @throws FileWriteError
+ * @throws FileIntegrityError
*
* @param[in] uid user id
* @param[in] installationType type of installation (global or local)
@@ -81,6 +102,7 @@ void updatePermissibleFile(uid_t uid, int installationType,
* @throws FileLockError
* @throws FileOpenError
* @throws FileReadError
+ * @throws FileIntegrityError
*
* @param[in] nameFile path to the labels file
* @param[out] appLabels vector to which application labels are added
diff --git a/src/common/permissible-set.cpp b/src/common/permissible-set.cpp
index cf53100a..b8e826ef 100644
--- a/src/common/permissible-set.cpp
+++ b/src/common/permissible-set.cpp
@@ -33,10 +33,12 @@
#include <cstdio>
#include <cstring>
+#include <filesystem>
#include <fstream>
#include <linux/xattr.h>
#include <memory>
#include <pwd.h>
+#include <sstream>
#include <string>
#include <sys/file.h>
#include <sys/smack.h>
@@ -52,6 +54,7 @@
#include <filesystem.h>
#include <permissible-set.h>
#include <privilege_db.h>
+#include <openssl/sha.h>
#include <security-manager-types.h>
#include <smack-labels.h>
#include <tzplatform_config.h>
@@ -61,6 +64,10 @@
namespace SecurityManager {
namespace PermissibleSet {
+static const char* hashMarker = "-SHA-1";
+static const int hashMarkerLength = strlen(hashMarker);
+static const int hashLength = 40;
+
template <typename T>
static inline int getFd(T &fstream)
{
@@ -96,6 +103,77 @@ static void markPermissibleFileValid(int fd, const std::string &nameFile, bool v
LogAndThrowErrno(PermissibleSetException::FileWriteError, "fchmod " << nameFile);
}
+std::string calculateHash(const std::string& content)
+{
+ unsigned char hashBinary[hashLength / 2];
+ SHA1(reinterpret_cast<const unsigned char*>(content.c_str()), content.length(), hashBinary);
+
+ // write the 160 bits of data as 40 hex digits
+ std::stringstream ss;
+ for (int i = 0; i < hashLength / 2; ++i)
+ ss << std::setw(2) << std::setfill('0') << std::hex << static_cast<unsigned int>(hashBinary[i]);
+
+ return ss.str();
+}
+
+void verifyFileIntegrity(const std::string &nameFile, std::ifstream &fstream)
+{
+ auto fileSize = std::filesystem::file_size(nameFile);
+
+ if (0 == fileSize) {
+ // empty file is considered valid
+ return;
+ }
+
+ std::string markerRead;
+ fstream.seekg(0, std::ios::beg);
+ fstream >> markerRead;
+ if (!fstream.good()) {
+ LogAndThrowErrno(PermissibleSetException::FileReadError, "reading hash marker " << nameFile);
+ }
+
+ // check if the file has a hash at all (for backward compatibility)
+ if (markerRead == hashMarker) {
+
+ std::string hashRead;
+ fstream >> hashRead >> std::ws;
+ if (!fstream) {
+ LogAndThrowErrno(PermissibleSetException::FileReadError, "reading hash " << nameFile);
+ }
+
+ if (hashRead.length() != hashLength) {
+ LogAndThrowErrno(PermissibleSetException::FileIntegrityError, "incorrect hash length: " << hashRead << " " << nameFile);
+ }
+
+ if (fstream.eof()) {
+ if (hashRead != calculateHash(""))
+ LogAndThrowErrno(PermissibleSetException::FileIntegrityError, "incorrect hash for empty content: " << hashRead << " " << nameFile);
+ else
+ return;
+ }
+
+ auto labelsStart = fstream.tellg();
+ auto contentSize = fileSize - labelsStart;
+
+ std::string content(contentSize, ' ');
+ fstream.read(&content[0], contentSize);
+ if (!fstream.good()) {
+ LogAndThrowErrno(PermissibleSetException::FileReadError, "read file content, only " << fstream.gcount() << " characters read " << nameFile);
+ }
+
+ // hash the file content and compare it with the hash that has been read
+ auto hashCalculated = calculateHash(content);
+
+ if (hashRead != hashCalculated) {
+ LogAndThrowErrno(PermissibleSetException::FileIntegrityError, "hash does not match. Read:" << hashRead << " Calculated from file " << hashCalculated << " " << nameFile);
+ }
+
+ fstream.seekg(labelsStart, std::ios::beg);
+ } else {
+ fstream.seekg(0, std::ios::beg);
+ }
+}
+
void updatePermissibleFile(uid_t uid, int installationType,
const std::vector<std::string> &labelsToAddForUser,
const std::vector<std::string> &labelsToRemoveForUser)
@@ -107,6 +185,8 @@ void updatePermissibleFile(uid_t uid, int installationType,
try {
openAndLockNameFile(nameFile, fstream);
+ verifyFileIntegrity(nameFile, fstream);
+
std::string line;
while (std::getline(fstream, line)) {
bool flag = true;
@@ -129,12 +209,19 @@ void updatePermissibleFile(uid_t uid, int installationType,
openAndLockNameFile(nameFile, fstream);
markPermissibleFileValid(getFd(fstream), nameFile, false);
+ std::stringstream ss;
for (auto &label : labelsForUser) {
- fstream << label << '\n';
- if (fstream.bad())
+ ss << label << '\n';
+ }
+
+ auto content = ss.str();
+ auto hash = calculateHash(content);
+
+ fstream << hashMarker << "\n" << hash << '\n' << content;
+
+ if (fstream.bad())
LogAndThrowErrno(PermissibleSetException::PermissibleSetException::FileWriteError,
"write to file " << nameFile);
- }
if (fstream.flush().fail())
LogAndThrowErrno(PermissibleSetException::FileWriteError, "fflush " << nameFile);
if (fsync(getFd(fstream)) == -1)
@@ -147,6 +234,8 @@ void readLabelsFromPermissibleFile(const std::string &nameFile, std::vector<std:
std::ifstream fstream;
openAndLockNameFile(nameFile, fstream);
+ verifyFileIntegrity(nameFile, fstream);
+
std::string line;
while (std::getline(fstream, line))
appLabels.push_back(line);
diff --git a/src/include/security-manager-types.h b/src/include/security-manager-types.h
index bed8b1b0..c152546a 100644
--- a/src/include/security-manager-types.h
+++ b/src/include/security-manager-types.h
@@ -55,7 +55,8 @@ enum lib_retcode {
SECURITY_MANAGER_ERROR_FILE_CREATE_FAILED,
SECURITY_MANAGER_ERROR_FILE_DELETE_FAILED,
SECURITY_MANAGER_ERROR_MOUNT_ERROR,
- SECURITY_MANAGER_ERROR_NOT_PATH_OWNER
+ SECURITY_MANAGER_ERROR_NOT_PATH_OWNER,
+ SECURITY_MANAGER_ERROR_FILE_INTEGRITY
};
/*! \brief accesses types for application installation paths*/
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 592deaff..34029cb6 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -82,6 +82,7 @@ SET(SM_TESTS_SOURCES
${SM_TEST_SRC}/test_log.cpp
${SM_TEST_SRC}/test_filesystem.cpp
${SM_TEST_SRC}/test_file-lock.cpp
+ ${SM_TEST_SRC}/test_permissible-set.cpp
${SM_TEST_SRC}/test_privilege_db_transactions.cpp
${SM_TEST_SRC}/test_privilege_db_app_pkg_getters.cpp
${SM_TEST_SRC}/test_privilege_db_add_app.cpp
@@ -113,6 +114,7 @@ SET(SM_TESTS_SOURCES
${PROJECT_SOURCE_DIR}/src/common/credentials.cpp
${PROJECT_SOURCE_DIR}/src/common/filesystem.cpp
${PROJECT_SOURCE_DIR}/src/common/file-lock.cpp
+ ${PROJECT_SOURCE_DIR}/src/common/permissible-set.cpp
${PROJECT_SOURCE_DIR}/src/common/privilege_db.cpp
${PROJECT_SOURCE_DIR}/src/common/service_impl_utils.cpp
${PROJECT_SOURCE_DIR}/src/common/smack-accesses.cpp
diff --git a/test/test_permissible-set.cpp b/test/test_permissible-set.cpp
new file mode 100644
index 00000000..16d86adc
--- /dev/null
+++ b/test/test_permissible-set.cpp
@@ -0,0 +1,256 @@
+/*
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd. All rights reserved
+ *
+ * This file is licensed under the terms of MIT License or the Apache License
+ * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
+ * See the LICENSE file or the notice below for Apache License Version 2.0
+ * details.
+ *
+ * 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.
+ */
+
+/*
+ * @file test_permissible-set.cpp
+ * @author Filip Skrzeczkowski (f.skrzeczkow@samsung.com)
+ * @version 1.0
+ */
+#include <boost/test/results_reporter.hpp>
+
+#include <filesystem>
+#include <fstream>
+#include <sstream>
+#include <string>
+#include <vector>
+
+#include <dpl/log/log.h>
+#include <permissible-set.h>
+#include <testmacros.h>
+
+using namespace SecurityManager::PermissibleSet;
+
+static constexpr int validHashLength = 40;
+static const std::string hashMarker = "-SHA-1";
+
+class PermissibleFileFixture
+{
+private:
+ void removeFile(const std::string& filename)
+ {
+ BOOST_REQUIRE_NO_THROW(std::filesystem::remove(filename));
+ }
+
+public:
+ inline static const std::string existentFile = "/tmp/SecurityManagerPermissibleFileLockExisting";
+ inline static const std::string nonExistentFile = "/tmp/SecurityManagerPermissibleFileLockNoExisting";
+
+ PermissibleFileFixture()
+ {
+ // create the file that's supposed to exist and delete the other
+ BOOST_REQUIRE_NO_THROW(std::ofstream os(existentFile));
+ removeFile(nonExistentFile);
+ }
+
+ virtual ~PermissibleFileFixture()
+ {
+ removeFile(existentFile);
+ removeFile(nonExistentFile);
+ }
+};
+
+BOOST_AUTO_TEST_SUITE(PERMISSIBLE_SET_TEST)
+
+POSITIVE_TEST_CASE(T1700_calculate_hash_valid_content_length)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ auto hash = calculateHash(content);
+ BOOST_REQUIRE(hash.length() == validHashLength);
+}
+
+POSITIVE_TEST_CASE(T1701_calculate_hash_hex_characters)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ auto hash = calculateHash(content);
+ BOOST_REQUIRE(hash.find_first_not_of("0123456789abcdef") == std::string::npos);
+}
+
+POSITIVE_TEST_CASE(T1702_calculate_hash_deterministic)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ auto hash1 = calculateHash(content);
+ auto hash2 = calculateHash(content);
+ BOOST_REQUIRE(hash1 == hash2);
+}
+
+POSITIVE_TEST_CASE(T1703_calculate_hash_empty_content)
+{
+ std::string content = "";
+ auto hash = calculateHash(content);
+ BOOST_REQUIRE_MESSAGE(hash.length() == validHashLength, "calculated hash: " << hash);
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1710_verify_integrity_valid_hash, PermissibleFileFixture)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ auto hash = calculateHash(content);
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << hash << "\n" << content;
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_NO_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is));
+}
+
+NEGATIVE_FIXTURE_TEST_CASE(T1711_verify_integrity_hash_not_matching, PermissibleFileFixture)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ std::string hash(validHashLength, 'g'); // 'g' is not hex, so this won't ever match
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << hash << "\n" << content;
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is), PermissibleSetException::FileIntegrityError);
+}
+
+NEGATIVE_FIXTURE_TEST_CASE(T1712_verify_integrity_hash_too_short, PermissibleFileFixture)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ std::string hash(validHashLength - 1, '0');
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << hash << "\n" << content;
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is), PermissibleSetException::FileIntegrityError);
+}
+
+NEGATIVE_FIXTURE_TEST_CASE(T1713_verify_integrity_hash_too_long, PermissibleFileFixture)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ std::string hash(validHashLength + 1, '0');
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << hash << "\n" << content;
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is), PermissibleSetException::FileIntegrityError);
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1714_verify_integrity_no_hash, PermissibleFileFixture)
+{
+ std::string content = "Subject_label Object_label Access_type\n";
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << content;
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_NO_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is));
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1715_verify_integrity_empty_file, PermissibleFileFixture)
+{
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_NO_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is));
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1716_verify_integrity_valid_hash_without_content, PermissibleFileFixture)
+{
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << calculateHash("") << "\n";
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_NO_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is));
+}
+
+NEGATIVE_FIXTURE_TEST_CASE(T1717_verify_integrity_hash_not_matching_without_content, PermissibleFileFixture)
+{
+ std::string hash(validHashLength, 'g');
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << hash << "\n";
+ }
+ std::ifstream is(PermissibleFileFixture::existentFile);
+ BOOST_REQUIRE_THROW(verifyFileIntegrity(PermissibleFileFixture::existentFile, is), PermissibleSetException::FileIntegrityError);
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1720_read_valid_file_valid_hash, PermissibleFileFixture)
+{
+ std::vector<std::string> labelsRead;
+ std::vector<std::string> labelsWritten = {
+ "Subject_label1 Object_label1 Access_type1",
+ "Subject_label2 Object_label2 Access_type3",
+ "Subject_label3 Object_label3 Access_type3"
+ };
+
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+
+ std::stringstream ss;
+ for (auto& label : labelsWritten) {
+ ss << label << "\n";
+ }
+
+ auto content = ss.str();
+ auto hash = calculateHash(content);
+
+ BOOST_REQUIRE_NO_THROW(os << hashMarker << "\n" << hash << '\n' << content);
+ }
+ BOOST_REQUIRE_NO_THROW(readLabelsFromPermissibleFile(PermissibleFileFixture::existentFile, labelsRead));
+ BOOST_CHECK_EQUAL_COLLECTIONS(labelsWritten.begin(), labelsWritten.end(), labelsRead.begin(), labelsRead.end());
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1721_read_valid_file_no_hash, PermissibleFileFixture)
+{
+ std::vector<std::string> labelsRead;
+ std::vector<std::string> labelsWritten = {
+ "Subject_label1 Object_label1 Access_type1",
+ "Subject_label2 Object_label2 Access_type3",
+ "Subject_label3 Object_label3 Access_type3"
+ };
+
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ for (auto& label : labelsWritten) {
+ BOOST_REQUIRE_NO_THROW(os << label << "\n");
+ }
+ }
+ BOOST_REQUIRE_NO_THROW(readLabelsFromPermissibleFile(PermissibleFileFixture::existentFile, labelsRead));
+ BOOST_CHECK_EQUAL_COLLECTIONS(labelsWritten.begin(), labelsWritten.end(), labelsRead.begin(), labelsRead.end());
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1722_read_empty_file, PermissibleFileFixture)
+{
+ std::vector<std::string> appLabels;
+ BOOST_REQUIRE_NO_THROW(readLabelsFromPermissibleFile(PermissibleFileFixture::existentFile, appLabels));
+ BOOST_REQUIRE(0 == appLabels.size());
+}
+
+POSITIVE_FIXTURE_TEST_CASE(T1723_read_file_valid_hash_without_content, PermissibleFileFixture)
+{
+ std::vector<std::string> appLabels;
+ {
+ std::ofstream os(PermissibleFileFixture::existentFile);
+ os << hashMarker << " " << calculateHash("") << "\n";
+ }
+ BOOST_REQUIRE_NO_THROW(readLabelsFromPermissibleFile(PermissibleFileFixture::existentFile, appLabels));
+ BOOST_REQUIRE(0 == appLabels.size());
+}
+
+NEGATIVE_FIXTURE_TEST_CASE(T1724_read_non_existent_file, PermissibleFileFixture)
+{
+ std::vector<std::string> appLabels;
+ BOOST_REQUIRE_THROW(readLabelsFromPermissibleFile(PermissibleFileFixture::nonExistentFile, appLabels), PermissibleSetException::FileOpenError);
+}
+
+BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file