szaszm commented on a change in pull request #914: URL: https://github.com/apache/nifi-minifi-cpp/pull/914#discussion_r498804163
########## File path: encrypt-config/tests/ConfigFileEncryptorTests.cpp ########## @@ -0,0 +1,128 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +#include "ConfigFileEncryptor.h" + +#include "TestBase.h" +#include "utils/RegexUtils.h" + +using org::apache::nifi::minifi::encrypt_config::ConfigFile; +using org::apache::nifi::minifi::encrypt_config::encryptSensitivePropertiesInFile; + +namespace { +size_t base64_length(size_t unencoded_length) { + return (unencoded_length + 2) / 3 * 4; +} + +bool check_encryption(const ConfigFile& test_file, const std::string& property_name, size_t original_value_length) { + utils::optional<std::string> encrypted_value = test_file.getValue(property_name); + if (!encrypted_value) { return false; } + + utils::optional<std::string> encryption_type = test_file.getValue(property_name + ".protected"); + if (!encryption_type || *encryption_type != utils::crypto::EncryptionType::name()) { return false; } + + auto length = base64_length(utils::crypto::EncryptionType::nonceLength()) + + utils::crypto::EncryptionType::separator().size() + + base64_length(original_value_length + utils::crypto::EncryptionType::macLength()); + return utils::Regex::matchesFullInput("[0-9A-Za-z/+=|]{" + std::to_string(length) + "}", *encrypted_value); +} +} // namespace + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace encrypt_config { + +// NOTE(fgerlits): these ==/!= operators are in the test file on purpose, and should not be part of production code, +// as they take a varying amount of time depending on which character in the line differs, so they would open up +// our code to timing attacks. If you need == in production code, make sure to compare all pairs of chars/lines. +bool operator==(const ConfigLine& left, const ConfigLine& right) { return left.getLine() == right.getLine(); } +bool operator!=(const ConfigLine& left, const ConfigLine& right) { return !(left == right); } +bool operator==(const ConfigFile& left, const ConfigFile& right) { return left.config_lines_ == right.config_lines_; } +bool operator!=(const ConfigFile& left, const ConfigFile& right) { return !(left == right); } Review comment: Is this a real risk? `ConfigLine` and `ConfigFile` contain the properties file with the sensitive properties in ciphertext, so it's not sensitive IMO. I'm not asking this to be moved back, just want to know how real the risk is. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
