subject:"\[GitHub\] \[nifi\-minifi\-cpp\] adamdebreceni commented on a change in pull request #937\: MINIFICPP\-1402 \- Encrypt flow configuration and change encryption key"

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534193427



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   so that was that sound!
   
   true, that calling a file `CollectionUtils` and then having a single 
function in it which only works for `std::set` is not great :) 
   
   made it generic, and added tests





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534140777



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   even though in my opinion "could be made generic" does not imply "should 
be made generic" (because nobody yet asked for it), since you and @lordgamez 
also think it is worth generalizing, I am turning this into a template





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534006825



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   moved it





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534001123



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533983259



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});
+
+  FileSystem(const FileSystem&) = delete;
+  FileSystem(FileSystem&&) = delete;
+  FileSystem& operator=(const FileSystem&) = delete;
+  FileSystem& operator=(FileSystem&&) = delete;
+
+  utils::optional read(const std::string& file_name);
+
+  bool write(const std::string& file_name, const std::string& file_content);
+
+ private:
+  bool should_encrypt_on_write_;
+  utils::optional encryptor_;
+  std::shared_ptr 
logger_{logging::LoggerFactory::getLogger()};
+};

Review comment:
   how about `FileIO`?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533973565



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   I would wait with generalizing it, because it is currently used once, I 
would move it to the utils on an as-needed basis, should we move it now?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533971996



##
File path: libminifi/src/utils/StringUtils.cpp
##
@@ -31,7 +31,18 @@ bool StringUtils::StringToBool(std::string input, bool 
) {
   return output;
 }
 
-std::string StringUtils::trim(std::string s) {
+utils::optional StringUtils::toBool(const std::string& str) {

Review comment:
   there were tests for it before the "StringView massacre" but I threw the 
baby out with the bathwater, added test for it (yes it aims to replace the 
`StringToBool`, but they have slightly different semantics for now, and 
`StringToBool` is used in many places, so I would retire `StringToBool` 
separately)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533970246



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533968416



##
File path: libminifi/include/utils/StringUtils.h
##
@@ -23,7 +23,7 @@
 #include 
 #include 
 #ifdef WIN32
-  #include 
+#include 

Review comment:
   indeed, it was unintentional, fixed

##
File path: libminifi/include/utils/StringUtils.h
##
@@ -143,6 +146,12 @@ class StringUtils {
 return std::equal(endString.rbegin(), endString.rend(), value.rbegin(), 
[](unsigned char lc, unsigned char rc) {return tolower(lc) == tolower(rc);});
   }
 
+  inline static bool startsWith(const std::string , const std::string & 
startString) {

Review comment:
   done, and for `endsWith` as well





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533149990



##
File path: extensions/http-curl/tests/C2ConfigEncryption.cpp
##
@@ -0,0 +1,58 @@
+/**
+ *
+ * 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.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "utils/EncryptionProvider.h"
+
+int main(int argc, char **argv) {
+  const cmd_args args = parse_cmdline_args(argc, argv, "update");
+  TestController controller;
+  // copy config file to temporary location as it will get overridden
+  char tmp_format[] = "/var/tmp/c2.XX";
+  std::string home_path = controller.createTempDirectory(tmp_format);
+  std::string live_config_file = 
utils::file::FileUtils::concat_path(home_path, "config.yml");
+  utils::file::FileUtils::copy_file(args.test_file, live_config_file);
+  // the C2 server will update the flow with the contents of args.test_file
+  // which will be encrypted and persisted to the temporary live_config_file
+  C2UpdateHandler handler(args.test_file);
+  VerifyC2Update harness(1);
+  
harness.getConfiguration()->set(minifi::Configure::nifi_flow_configuration_encrypt,
 "true");
+  harness.setKeyDir(args.key_dir);
+  harness.setUrl(args.url, );
+  handler.setC2RestResponse(harness.getC2RestUrl(), "configuration", "true");
+
+  const auto start = std::chrono::system_clock::now();

Review comment:
   removed





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533152594



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   (although the is-a relationship arising from the resulting 
inheritance-chain might be troublesome, we could go for protected inheritance 
or composition (generally I favor composition, but in this case I'd go with 
protected inheritance))





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533152594



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   (although the is-a relationship arising from the resulting 
inheritance-chain might be troublesome, we could go for protected inheritance 
or composition)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533149990



##
File path: extensions/http-curl/tests/C2ConfigEncryption.cpp
##
@@ -0,0 +1,58 @@
+/**
+ *
+ * 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.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "utils/EncryptionProvider.h"
+
+int main(int argc, char **argv) {
+  const cmd_args args = parse_cmdline_args(argc, argv, "update");
+  TestController controller;
+  // copy config file to temporary location as it will get overridden
+  char tmp_format[] = "/var/tmp/c2.XX";
+  std::string home_path = controller.createTempDirectory(tmp_format);
+  std::string live_config_file = 
utils::file::FileUtils::concat_path(home_path, "config.yml");
+  utils::file::FileUtils::copy_file(args.test_file, live_config_file);
+  // the C2 server will update the flow with the contents of args.test_file
+  // which will be encrypted and persisted to the temporary live_config_file
+  C2UpdateHandler handler(args.test_file);
+  VerifyC2Update harness(1);
+  
harness.getConfiguration()->set(minifi::Configure::nifi_flow_configuration_encrypt,
 "true");
+  harness.setKeyDir(args.key_dir);
+  harness.setUrl(args.url, );
+  handler.setC2RestResponse(harness.getC2RestUrl(), "configuration", "true");
+
+  const auto start = std::chrono::system_clock::now();

Review comment:
   remove

##
File path: libminifi/src/utils/EncryptionProvider.cpp
##
@@ -33,22 +40,19 @@ constexpr const char* CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sens
 
 }  // namespace
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-
-utils::optional Decryptor::create(const std::string& 
minifi_home) {
+utils::optional EncryptionProvider::create(const 
std::string& home_path) {
   minifi::Properties bootstrap_conf;
-  bootstrap_conf.setHome(minifi_home);
+  bootstrap_conf.setHome(home_path);
   bootstrap_conf.loadConfigureFile(DEFAULT_NIFI_BOOTSTRAP_FILE);
   return bootstrap_conf.getString(CONFIG_ENCRYPTION_KEY_PROPERTY_NAME)
-  | utils::map([](const std::string& encryption_key_hex) { return 
utils::StringUtils::from_hex(encryption_key_hex); })
-  | utils::map(::crypto::stringToBytes)
-  | utils::map([](const utils::crypto::Bytes& encryption_key_bytes) { 
return minifi::Decryptor{encryption_key_bytes}; });
+ | utils::map([](const std::string _key_hex) { return 
utils::StringUtils::from_hex(encryption_key_hex); })
+ | utils::map(::crypto::stringToBytes)
+ | utils::map([](const utils::crypto::Bytes _key_bytes) { 
return EncryptionProvider{encryption_key_bytes}; });

Review comment:
   done





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533148086



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   (also it was necessitated by my desire to factor out c2-relevant parts 
of the `FlowController` into `C2Client` (although that might not be the best 
name, I am open to suggestions))





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533147213



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   we have a `FlowController` class which controls a "flow", although 
"flow" is a concept, it was not materialized in our architecture before (as far 
as I understand), this aims to take one step in that direction, what 
constitutes a "flow" is not entirely clear to me, but currently I went with the 
layout of the network and the repositories





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533139939



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});
+
+  FileSystem(const FileSystem&) = delete;
+  FileSystem(FileSystem&&) = delete;
+  FileSystem& operator=(const FileSystem&) = delete;
+  FileSystem& operator=(FileSystem&&) = delete;
+
+  utils::optional read(const std::string& file_name);
+
+  bool write(const std::string& file_name, const std::string& file_content);
+
+ private:
+  bool should_encrypt_on_write_;
+  utils::optional encryptor_;
+  std::shared_ptr 
logger_{logging::LoggerFactory::getLogger()};
+};

Review comment:
   the problem with this, now that I think about it, is the question of 
"who owns the config file path", because if it is a `ConfigFileIO` it should 
own the path and read/write should not get a path parameter, but then again 
currently `FlowConfiguration` owns the config file path, so I think that would 
be a bigger change (which may not be beneficial at all)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533136916



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});
+
+  FileSystem(const FileSystem&) = delete;
+  FileSystem(FileSystem&&) = delete;
+  FileSystem& operator=(const FileSystem&) = delete;
+  FileSystem& operator=(FileSystem&&) = delete;
+
+  utils::optional read(const std::string& file_name);
+
+  bool write(const std::string& file_name, const std::string& file_content);
+
+ private:
+  bool should_encrypt_on_write_;
+  utils::optional encryptor_;
+  std::shared_ptr 
logger_{logging::LoggerFactory::getLogger()};
+};

Review comment:
   I specifically wanted it not to be aware of encryption, `ConfigFileIO` 
seemed a little too specific for me, but for the current purposes this seems to 
be our best bet





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529611248



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529606659



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529603865



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529602661



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529600853



##
File path: encrypt-config/ArgParser.h
##
@@ -0,0 +1,74 @@
+/**
+ * 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.
+ */
+#pragma once
+
+#include 
+#include 
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+struct Argument {
+  std::set names;
+  bool required;
+  std::string value_name;
+  std::string description;
+};
+
+struct FlagArgument {
+  std::set names;
+  std::string description;
+};
+
+class Arguments {
+  static const std::vector simple_arguments_;

Review comment:
   agree, renamed





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529558077



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   `StringView` has been eliminated





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529553417



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   as a general rule of thumb, I favour granularity, I believe we should 
promise as little as possible (because promises have to be kept and that takes 
effort), and features should be added on a as-needed basis, for example the 
lack of `operator[]` in this case makes this implementation non-conformant, but 
that is inconsequential
   
   (now on the other hand the added methods `startsWith` and `endsWith` (coming 
in `c++20`) could have pose a problem when "upgrading" to `std::string_view`, 
this is a shortcoming of the language offering no extension methods, that is 
why I believe the best course of action is to centralize the introduction of 
stl parts, so we may use whatever implementation we want, so instead of 
`std::vector` we use `minifi::std::vector`, but of course this decision better 
be made at the beginning of a project)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529553417



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   as a general rule of thumb, I favour granularity, I believe we should 
promise as little as possible (because promises have to be kept and that takes 
effort), and features should be added on a as-needed basis, for example the 
lack of `operator[]` in this case makes this implementation non-conformant, but 
that is inconsequential
   
   (now on the other hand the added methods `startsWith` and `endsWith` (coming 
in `c++20`) could have pose a problem when "upgrading" to `std::string_view`, 
this is a shortcoming of the language offering no extension methods, that is 
why I believe the best course of action is to centralize the introduction of 
stl parts, so we may use whatever implementation we want, so no `std::vector`, 
but `minifi::std::vector`)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529285840



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});
+
+  FileSystem(const FileSystem&) = delete;
+  FileSystem(FileSystem&&) = delete;
+  FileSystem& operator=(const FileSystem&) = delete;
+  FileSystem& operator=(FileSystem&&) = delete;
+
+  utils::optional read(const std::string& file_name);
+
+  bool write(const std::string& file_name, const std::string& file_content);
+
+ private:
+  bool should_encrypt_on_write_;
+  utils::optional encryptor_;
+  std::shared_ptr 
logger_{logging::LoggerFactory::getLogger()};
+};

Review comment:
   agree, do you have a specific name in mind?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529283378



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   although the utility of this class itself might be questioned (I just 
couldn't stomach copying one more `std::string` when a non-copying mechanism 
would suffice) should we put this class out of its (short) misery?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529283378



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   although the utility of this class itself might be questioned (I just 
couldn't stomach copying one more `std::string` when a non-copying mechanism 
would suffice) should we put out this class of its (short) misery?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529275334



##
File path: encrypt-config/CommandException.h
##
@@ -0,0 +1,44 @@
+/**
+ * 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.
+ */
+#pragma once
+
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+struct CommandException : std::exception {
+  CommandException(const std::string& err) : error_(err) {}
+  CommandException(const char* err) : error_(err) {}
+
+  const char* what() const noexcept override {
+return error_.c_str();
+  }
+
+ private:
+  std::string error_;
+};

Review comment:
   I wouldn't expose too much from the "minifi agent internals" into this 
tool, moreover I would consider the exception resulting from incorrect 
parameterization of the command `std::logic_error` not a `std::runtime_error`, 
so made `CommandException` extend `std::logic_error` 





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529273915



##
File path: encrypt-config/CommandException.h
##
@@ -0,0 +1,44 @@
+/**
+ * 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.
+ */
+#pragma once
+
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+struct CommandException : std::exception {
+  CommandException(const std::string& err) : error_(err) {}
+  CommandException(const char* err) : error_(err) {}

Review comment:
   done





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529273841



##
File path: encrypt-config/EncryptConfig.h
##
@@ -18,7 +18,8 @@
 
 #include 
 
-#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"

Review comment:
   removed





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529273731



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,187 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::simple_arguments_{
+{std::vector{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::flag_arguments_{
+{std::vector{"--help", "-h"},
+ "Prints this help message"},
+{std::vector{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+std::string Arguments::getHelp() {
+  std::stringstream ss;
+  ss << "Usage: " << "encrypt-config";
+  for (const auto& simple_arg : simple_arguments_) {
+ss << " ";
+if (!simple_arg.required) {
+  ss << "[";
+}
+ss << utils::StringUtils::join("|", simple_arg.names)
+<< " <" << simple_arg.value_name << ">";
+if (!simple_arg.required) {
+  ss << "]";
+}
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << " [" << utils::StringUtils::join("|", flag.names) << "]";
+  }
+  ss << std::endl;
+  for (const auto& simple_arg : simple_arguments_) {
+ss << "\t";
+ss << utils::StringUtils::join("|", simple_arg.names) << " : ";
+if (simple_arg.required) {
+  ss << "(required)";
+} else {
+  ss << "(optional)";
+}
+ss << " " << simple_arg.description;
+ss << std::endl;
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << "\t" << utils::StringUtils::join("|", flag.names) << " : "
+<< flag.description << std::endl;

Review comment:
   although the cited reason (slowdown caused by flushes) does not makes 
sense here (since we are writing into a `std::stringstream`), it also does not 
makes sense to flush so replaced them with `"\n"`s





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-24 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529270994



##
File path: libminifi/include/utils/StringViewUtils.h
##
@@ -0,0 +1,68 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "StringView.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+struct StringViewUtils {
+  static inline bool equalsIgnoreCase(StringView left, StringView right) {

Review comment:
   I wouldn't call string views, strings, maybe readonly-strings, but even 
that is a stretch, rather strings are special string views that own their 
content, moreover I think StringUtils.h is already pretty long so if anything I 
would chop off the "encoding/decoding" part of the StringUtils.h into a 
separate file





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529266987



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {

Review comment:
   as we plan on upgrading to `c++17` I am against introducing anything 
"standard-conformant-aiming", moreover it also provides partial compatibility 
in the form of "iterability", but hopefully as soon as we have 
`std::string_view`, this class becomes obsolete





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r529261449



##
File path: libminifi/include/c2/C2Payload.h
##
@@ -164,10 +161,9 @@ class C2Payload : public state::Update {
   bool isContainer() const noexcept { return is_container_; }
   void setContainer(bool is_container) noexcept { is_container_ = 
is_container; }
 
-  /**
-   * Get nested payloads.
-   */
-  const std::vector () const noexcept { return 
payloads_; }
+  const std::vector () const & noexcept { return 
payloads_; }

Review comment:
   it is, if you plan to overload it with a `&&`-qualified method (which 
was "necessitated" by some `C2Agent` methods taking rvalue ref payloads, but 
did not yet untangle them in this PR)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r528764869



##
File path: encrypt-config/EncryptConfig.cpp
##
@@ -42,81 +40,119 @@ namespace nifi {
 namespace minifi {
 namespace encrypt_config {
 
-EncryptConfig::EncryptConfig(int argc, char* argv[]) : 
minifi_home_(parseMinifiHomeFromTheOptions(argc, argv)) {
+EncryptConfig::EncryptConfig(const std::string& minifi_home) : 
minifi_home_(minifi_home) {
   if (sodium_init() < 0) {
 throw std::runtime_error{"Could not initialize the libsodium library!"};
   }
+  // encryption/decryption depends on the libsodium library which needs to be 
initialized
+  keys_ = getEncryptionKeys();
 }
 
-std::string EncryptConfig::parseMinifiHomeFromTheOptions(int argc, char* 
argv[]) {
-  if (argc >= 2) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-h") || (argstr == "--help")) {
-std::cout << USAGE_STRING << std::endl;
-std::exit(0);
-  }
-}
+EncryptConfig::EncryptionType EncryptConfig::encryptSensitiveProperties() 
const {
+  encryptSensitiveProperties(keys_);
+  if (keys_.old_key) {
+return EncryptionType::RE_ENCRYPT;
   }
+  return EncryptionType::ENCRYPT;
+}
 
-  if (argc >= 3) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-m") || (argstr == "--minifi-home")) {
-if (i+1 < argc) {
-  return std::string(argv[i+1]);
-}
-  }
-}
+void EncryptConfig::encryptFlowConfig() const {
+  encrypt_config::ConfigFile 
properties_file{std::ifstream{propertiesFilePath()}};
+  utils::optional config_path = 
properties_file.getValue(Configure::nifi_flow_configuration_file);
+  if (!config_path) {
+config_path = utils::file::PathUtils::resolve(minifi_home_, 
"conf/config.yml");
+std::cout << "Couldn't find path of configuration file, using default: \"" 
<< *config_path << "\"\n";
+  } else {
+config_path = utils::file::PathUtils::resolve(minifi_home_, *config_path);
+std::cout << "Encrypting flow configuration file: \"" << *config_path << 
"\"\n";
+  }
+  std::string config_content;
+  try {
+std::ifstream config_file{*config_path, std::ios::binary};
+config_file.exceptions(std::ios::failbit | std::ios::badbit);
+config_content = std::string{std::istreambuf_iterator(config_file), 
{}};
+  } catch (...) {
+std::cerr << "Error while reading flow configuration file \"" << 
*config_path << "\"\n";
+throw;
   }
+  try {
+utils::crypto::decrypt(config_content, keys_.encryption_key);
+std::cout << "Flow config file is already properly encrypted.\n";

Review comment:
   well, if a value is encrypted and we can't decrypt it neither with the 
encryption key nor with the old encryption key, I would call that "improperly 
encrypted" :) 





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r528764113



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,187 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::simple_arguments_{
+{std::vector{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::flag_arguments_{
+{std::vector{"--help", "-h"},
+ "Prints this help message"},
+{std::vector{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+std::string Arguments::getHelp() {
+  std::stringstream ss;
+  ss << "Usage: " << "encrypt-config";
+  for (const auto& simple_arg : simple_arguments_) {
+ss << " ";
+if (!simple_arg.required) {
+  ss << "[";
+}
+ss << utils::StringUtils::join("|", simple_arg.names)
+<< " <" << simple_arg.value_name << ">";
+if (!simple_arg.required) {
+  ss << "]";
+}
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << " [" << utils::StringUtils::join("|", flag.names) << "]";
+  }
+  ss << std::endl;
+  for (const auto& simple_arg : simple_arguments_) {
+ss << "\t";
+ss << utils::StringUtils::join("|", simple_arg.names) << " : ";
+if (simple_arg.required) {
+  ss << "(required)";
+} else {
+  ss << "(optional)";
+}
+ss << " " << simple_arg.description;
+ss << std::endl;
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << "\t" << utils::StringUtils::join("|", flag.names) << " : "
+<< flag.description << std::endl;
+  }
+  return ss.str();
+}
+
+void Arguments::set(const std::string& key, const std::string& value) {
+  if (get(key)) {
+std::cerr << "Key is specified more than once \"" << key << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  simple_args_[key] = value;
+}
+
+void Arguments::set(const std::string& flag) {
+  if (isSet(flag)) {
+std::cerr << "Flag is specified more than once \"" << flag << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  flag_args_.insert(flag);
+}
+
+utils::optional Arguments::get(const std::string ) const {
+  utils::optional opt_arg = getSimpleArg(key);
+  if (!opt_arg) {
+return {};
+  }
+  for (const auto& name : opt_arg->names) {
+auto it = simple_args_.find(name);
+if (it != simple_args_.end()) {
+  return it->second;
+}
+  }
+  return {};
+}
+
+bool Arguments::isSet(const std::string ) const {
+  utils::optional opt_flag = getFlag(flag);
+  if (!opt_flag) {
+return false;
+  }
+  return std::any_of(opt_flag->names.begin(), opt_flag->names.end(), [&] 
(const std::string& name) {
+return flag_args_.find(name) != flag_args_.end();
+  });
+}
+
+Arguments Arguments::parse(int argc, char* argv[]) {

Review comment:
   done





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r528763519



##
File path: encrypt-config/ArgParser.h
##
@@ -0,0 +1,72 @@
+/**
+ * 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.
+ */
+#pragma once
+
+#include 
+#include 
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+struct Argument {
+  std::vector names;

Review comment:
   rewrote to use std::set





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-23 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r528763519



##
File path: encrypt-config/ArgParser.h
##
@@ -0,0 +1,72 @@
+/**
+ * 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.
+ */
+#pragma once
+
+#include 
+#include 
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+struct Argument {
+  std::vector names;

Review comment:
   rewrote to use std::set-s





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-20 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r527731012



##
File path: libminifi/include/utils/file/PathUtils.h
##
@@ -17,12 +17,20 @@
 #ifndef LIBMINIFI_INCLUDE_UTILS_FILE_PATHUTILS_H_
 #define LIBMINIFI_INCLUDE_UTILS_FILE_PATHUTILS_H_
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include "utils/OptionalUtils.h"
+
+#ifdef _MSC_VER

Review comment:
   I removed this macro altogether since it wasn't even needed (`PATH_MAX` 
is used here exactly on non-win environments)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-20 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r527537299



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  std::lock_guard guard(metrics_mutex_);

Review comment:
   I would not revert as there is unsafe access to shared structures, but I 
am convinced that the cost of hunting down a possible deadlock is higher than 
what metrics consistency provides us currently
   
   I limited the scopes of locks, while trying to preserve safety
   (also removed the unused `device_information_` field)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-20 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r527537299



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  std::lock_guard guard(metrics_mutex_);

Review comment:
   I would not revert as there is unsafe access to shared structures, but I 
am convinced that the cost of hunting down a possible deadlock is higher than 
what metrics consistency provides us currently (although that comes down to 
evolving business needs)
   
   I limited the scopes of locks, while trying to preserve safety
   (also removed the unused `device_information_` field)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526895679



##
File path: libminifi/src/utils/file/FileSystem.cpp
##
@@ -0,0 +1,80 @@
+/**
+ * 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 
+#include 
+#include "utils/file/FileSystem.h"
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+FileSystem::FileSystem(bool should_encrypt, 
utils::optional encryptor)
+: should_encrypt_(should_encrypt),
+  encryptor_(std::move(encryptor)) {
+  if (should_encrypt_ && !encryptor) {
+std::string err_message = "Requested file encryption but no encryption 
utility was provided";
+logger_->log_error(err_message.c_str());
+throw std::invalid_argument(err_message);
+  }
+}
+
+utils::optional FileSystem::read(const std::string& file_name) {
+  std::ifstream input{file_name, std::ios::binary};
+  std::string content{std::istreambuf_iterator(input), {}};

Review comment:
   done, you are right, that it should return `nullopt` if there is no such 
file, not if there was any other kind of error

##
File path: libminifi/test/unit/FileSystemTests.cpp
##
@@ -0,0 +1,90 @@
+/**
+ *
+ * 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 
+#include 
+#include 
+#include "../TestBase.h"
+#include "utils/file/FileSystem.h"
+
+using utils::crypto::EncryptionProvider;
+using utils::file::FileSystem;
+
+utils::crypto::Bytes encryption_key = 
utils::crypto::stringToBytes(utils::StringUtils::from_hex(
+"4024b327fdc987ce3eb43dd1f690b9987e4072e0020e3edf4349ce1ad91a4e38"));
+
+struct FSTest : TestController {
+  FSTest() {
+char format[] = "/var/tmp/fs.XX";
+dir = createTempDirectory(format);
+encrypted_file = utils::file::FileUtils::concat_path(dir, "encrypted.txt");
+raw_file = utils::file::FileUtils::concat_path(dir, "raw.txt");
+new_file = utils::file::FileUtils::concat_path(dir, "new.txt");
+
+std::ofstream{encrypted_file, std::ios::binary} << 
crypto.encrypt("banana");
+std::ofstream{raw_file, std::ios::binary} << "banana";
+  }
+
+  EncryptionProvider crypto{encryption_key};
+  std::string encrypted_file;
+  std::string raw_file;
+  std::string new_file;
+  std::string dir;
+};
+
+TEST_CASE_METHOD(FSTest, "Can read encrypted or non-encrypted file", "[fs]") {
+  FileSystem fs{true, crypto};
+  REQUIRE(fs.read(encrypted_file) == "banana");
+  REQUIRE(fs.read(raw_file) == "banana");
+}
+
+TEST_CASE_METHOD(FSTest, "Write encrypted file", "[fs]") {
+  FileSystem fs{true, crypto};
+
+  fs.write(new_file, "red lorry, yellow lorry");
+
+  std::ifstream file{new_file, std::ios::binary};
+  std::string file_content{std::istreambuf_iterator(file), {}};
+  REQUIRE(crypto.decrypt(file_content) == "red lorry, yellow lorry");
+}
+
+TEST_CASE_METHOD(FSTest, "Can read encrypted but writes non-encrypted", 
"[fs]") {
+  FileSystem fs{false, crypto};
+  REQUIRE(fs.read(encrypted_file) == "banana");
+
+  fs.write(new_file, "red lorry, yellow lorry");
+
+  std::ifstream file{new_file, std::ios::binary};
+  std::string file_content{std::istreambuf_iterator(file), {}};
+  REQUIRE(file_content == "red lorry, yellow lorry");
+}
+
+TEST_CASE_METHOD(FSTest, "Can't read/write encrypted", "[fs]") {

Review comment:
   done

##
File

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526895238



##
File path: libminifi/test/unit/FileSystemTests.cpp
##
@@ -0,0 +1,90 @@
+/**
+ *
+ * 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 
+#include 
+#include 
+#include "../TestBase.h"
+#include "utils/file/FileSystem.h"
+
+using utils::crypto::EncryptionProvider;
+using utils::file::FileSystem;
+
+utils::crypto::Bytes encryption_key = 
utils::crypto::stringToBytes(utils::StringUtils::from_hex(
+"4024b327fdc987ce3eb43dd1f690b9987e4072e0020e3edf4349ce1ad91a4e38"));
+
+struct FSTest : TestController {
+  FSTest() {
+char format[] = "/var/tmp/fs.XX";
+dir = createTempDirectory(format);
+encrypted_file = utils::file::FileUtils::concat_path(dir, "encrypted.txt");
+raw_file = utils::file::FileUtils::concat_path(dir, "raw.txt");
+new_file = utils::file::FileUtils::concat_path(dir, "new.txt");
+
+std::ofstream{encrypted_file, std::ios::binary} << 
crypto.encrypt("banana");
+std::ofstream{raw_file, std::ios::binary} << "banana";
+  }
+
+  EncryptionProvider crypto{encryption_key};
+  std::string encrypted_file;
+  std::string raw_file;
+  std::string new_file;
+  std::string dir;
+};
+
+TEST_CASE_METHOD(FSTest, "Can read encrypted or non-encrypted file", "[fs]") {

Review comment:
   done





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526867132



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;

Review comment:
   done
   
   I was thinking of writing a `get(...)` taking a vector of keys, and 
returning an `optional`, (to handle alternate keys), but don't think 
that there is enough incentive for now (or at least not in this PR), what do 
you think?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526860105



##
File path: libminifi/src/utils/EncryptionProvider.cpp
##
@@ -15,40 +14,45 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "properties/Decryptor.h"
 
+#include 
+#include "utils/EncryptionProvider.h"
 #include "properties/Properties.h"
 #include "utils/OptionalUtils.h"
 #include "utils/StringUtils.h"
 
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace crypto {
+
 namespace {
 
 #ifdef WIN32
 constexpr const char* DEFAULT_NIFI_BOOTSTRAP_FILE = "\\conf\\bootstrap.conf";
 #else
-constexpr const char* DEFAULT_NIFI_BOOTSTRAP_FILE = "./conf/bootstrap.conf";
+constexpr const char *DEFAULT_NIFI_BOOTSTRAP_FILE = "./conf/bootstrap.conf";
 #endif  // WIN32
 
-constexpr const char* CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sensitive.key";
+constexpr const char *CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sensitive.key";

Review comment:
   done, this must have been an accident since I prefer my `*`s and `&`s 
next to the type not the identifier





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526864912



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});

Review comment:
   for now I renamed it to `should_encrypt_on_write`, we could partition 
the class further into a `FileSystemReader` and `FileSystemWriter` (having 
their own keys, or set of keys for the reader) if need arises 





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526858104



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  std::lock_guard guard(metrics_mutex_);

Review comment:
   the scope enlargement's purpose is not performance but consistency, if 
we were to decrease the scope of the lock (e.g. to its previous value), we 
would end up with possible inconsistent metrics (caused by locking & unlocking 
between every map insert)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526860105



##
File path: libminifi/src/utils/EncryptionProvider.cpp
##
@@ -15,40 +14,45 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "properties/Decryptor.h"
 
+#include 
+#include "utils/EncryptionProvider.h"
 #include "properties/Properties.h"
 #include "utils/OptionalUtils.h"
 #include "utils/StringUtils.h"
 
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace crypto {
+
 namespace {
 
 #ifdef WIN32
 constexpr const char* DEFAULT_NIFI_BOOTSTRAP_FILE = "\\conf\\bootstrap.conf";
 #else
-constexpr const char* DEFAULT_NIFI_BOOTSTRAP_FILE = "./conf/bootstrap.conf";
+constexpr const char *DEFAULT_NIFI_BOOTSTRAP_FILE = "./conf/bootstrap.conf";
 #endif  // WIN32
 
-constexpr const char* CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sensitive.key";
+constexpr const char *CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sensitive.key";

Review comment:
   this must have been an accident since I prefer my `*`s and `&`s next to 
the type not the identifier





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526858625



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  std::lock_guard guard(metrics_mutex_);
+
+  device_information_.clear();
+  component_metrics_.clear();
+  // root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+
+  std::string class_csv;
+  if (root_ != nullptr) {
+std::shared_ptr queueMetrics = 
std::make_shared();
+std::map> connections;
+root_->getConnections(connections);
+for (auto& con : connections) {
+  queueMetrics->addConnection(con.second);
+}
+device_information_[queueMetrics->getName()] = queueMetrics;
+std::shared_ptr repoMetrics = 
std::make_shared();
+repoMetrics->addRepository(provenance_repo_);
+repoMetrics->addRepository(flow_file_repo_);
+device_information_[repoMetrics->getName()] = repoMetrics;
+  }
+
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526858104



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,335 @@
+/**
+ *
+ * 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 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  bool c2_enabled = false;
+  if (configuration_->get(Configure::nifi_c2_enable, "c2.enable", 
c2_enable_str)) {
+utils::StringUtils::StringToBool(c2_enable_str, c2_enabled);
+  }
+  return c2_enabled;
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  std::lock_guard guard(metrics_mutex_);

Review comment:
   the scope enlargement's purpose is not performance but consistency, if 
we were to decrease the scope of the lock (e.g. to its previous value), we 
would end up with possible inconsistent metrics (like locking & unlocking 
between every map insert)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526772202



##
File path: libminifi/src/FlowController.cpp
##
@@ -70,60 +71,33 @@ namespace apache {
 namespace nifi {
 namespace minifi {
 
-#define DEFAULT_CONFIG_NAME "conf/config.yml"
-
-FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo, 
std::shared_ptr configure,
-   std::unique_ptr 
flow_configuration, std::shared_ptr content_repo, 
const std::string name, bool headless_mode)
+FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo,
+   std::shared_ptr configure, 
std::unique_ptr flow_configuration,
+   std::shared_ptr 
content_repo, const std::string name, bool headless_mode,
+   std::shared_ptr 
filesystem)
 : 
core::controller::ControllerServiceProvider(core::getClassName()),
-  root_(nullptr),
+  c2::C2Client(std::move(configure), std::move(provenance_repo), 
std::move(flow_file_repo),
+   std::move(content_repo), std::move(flow_configuration), 
std::move(filesystem)),
   running_(false),
   updating_(false),
-  c2_enabled_(true),
   initialized_(false),
-  provenance_repo_(provenance_repo),
-  flow_file_repo_(flow_file_repo),
   
controller_service_map_(std::make_shared()),
   thread_pool_(2, false, nullptr, "Flowcontroller threadpool"),
-  flow_configuration_(std::move(flow_configuration)),
-  configuration_(std::move(configure)),
-  content_repo_(content_repo),
   logger_(logging::LoggerFactory::getLogger()) {
-  if (provenance_repo == nullptr)
+  if (IsNullOrEmpty(provenance_repo_))

Review comment:
   rewrote them to use comparison to `nullptr`





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-19 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526678496



##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -873,6 +664,161 @@ void C2Agent::update_agent() {
   }
 }
 
+utils::TaskRescheduleInfo C2Agent::produce() {
+  // place priority on messages to send to the c2 server
+  if (protocol_.load() != nullptr) {
+std::vector payload_batch;
+payload_batch.reserve(max_c2_responses);
+auto getRequestPayload = [_batch] (C2Payload&& payload) { 
payload_batch.emplace_back(std::move(payload)); };
+for (std::size_t attempt_num = 0; attempt_num < max_c2_responses; 
++attempt_num) {
+  if (!requests.consume(getRequestPayload)) {
+break;
+  }
+}
+std::for_each(
+std::make_move_iterator(payload_batch.begin()),
+std::make_move_iterator(payload_batch.end()),
+[&] (C2Payload&& payload) {
+  try {
+C2Payload && response = 
protocol_.load()->consumePayload(std::move(payload));
+enqueue_c2_server_response(std::move(response));
+  }
+  catch(const std::exception ) {
+logger_->log_error("Exception occurred while consuming payload. 
error: %s", e.what());
+  }
+  catch(...) {
+logger_->log_error("Unknown exception occurred while consuming 
payload.");
+  }
+});
+
+try {
+  performHeartBeat();
+}
+catch (const std::exception ) {
+  logger_->log_error("Exception occurred while performing heartbeat. 
error: %s", e.what());
+}
+catch (...) {
+  logger_->log_error("Unknonwn exception occurred while performing 
heartbeat.");
+}
+  }
+
+  checkTriggers();
+
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
+}
+
+utils::TaskRescheduleInfo C2Agent::consume() {
+  const auto consume_success = responses.consume([this] (C2Payload&& payload) {
+extractPayload(std::move(payload));
+  });
+  if (!consume_success) {
+extractPayload(C2Payload{ Operation::HEARTBEAT });
+  }
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(C2RESPONSE_POLL_MS));
+}
+
+utils::optional C2Agent::fetchFlow(const std::string& uri) const {
+  if (!utils::StringUtils::startsWith(uri, "http") || protocol_.load() == 
nullptr) {
+// try to open the file
+utils::optional content = filesystem_->read(uri);
+if (content) {
+  return content;
+}
+  }
+  // couldn't open as file and we have no protocol to request the file from
+  if (protocol_.load() == nullptr) {
+return {};
+  }
+
+  std::string resolved_url = uri;
+  if (!utils::StringUtils::startsWith(uri, "http")) {
+std::stringstream adjusted_url;
+std::string base;
+if (configuration_->get(minifi::Configure::nifi_c2_flow_base_url, base)) {
+  adjusted_url << base;
+  if (!utils::StringUtils::endsWith(base, "/")) {
+adjusted_url << "/";
+  }
+  adjusted_url << uri;
+  resolved_url = adjusted_url.str();
+} else if (configuration_->get("c2.rest.url", base)) {
+  std::string host, protocol;
+  int port = -1;
+  utils::parse_url(, , , );
+  adjusted_url << protocol << host;
+  if (port > 0) {
+adjusted_url << ":" << port;
+  }
+  adjusted_url << "/c2/api/" << uri;
+  resolved_url = adjusted_url.str();
+}
+  }
+
+  C2Payload payload(Operation::TRANSFER, false, true);
+  C2Payload & = protocol_.load()->consumePayload(resolved_url, 
payload, RECEIVE, false);
+
+  auto raw_data = response.getRawData();
+  return std::string(raw_data.data(), raw_data.size());
+}
+
+bool C2Agent::handleConfigurationUpdate(const C2ContentResponse ) {
+  auto url = resp.operation_arguments.find("location");
+
+  std::string file_uri;
+  std::string configuration_str;
+
+  if (url == resp.operation_arguments.end()) {
+logger_->log_debug("Did not have location within %s", resp.ident);
+auto update_text = resp.operation_arguments.find("configuration_data");
+if (update_text == resp.operation_arguments.end()) {
+  logger_->log_debug("Neither the config file location nor the data is 
provided");
+  C2Payload response(Operation::ACKNOWLEDGE, 
state::UpdateState::SET_ERROR, resp.ident, false, true);
+  response.setRawData("Error while applying flow. Neither the config file 
location nor the data is provided.");
+  enqueue_c2_response(std::move(response));
+  return false;
+}
+configuration_str = update_text->second.to_string();
+  }
+
+  file_uri = url->second.to_string();

Review comment:
   nice catch, fixed





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526640002



##
File path: libminifi/src/FlowController.cpp
##
@@ -70,60 +71,33 @@ namespace apache {
 namespace nifi {
 namespace minifi {
 
-#define DEFAULT_CONFIG_NAME "conf/config.yml"
-
-FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo, 
std::shared_ptr configure,
-   std::unique_ptr 
flow_configuration, std::shared_ptr content_repo, 
const std::string name, bool headless_mode)
+FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo,
+   std::shared_ptr configure, 
std::unique_ptr flow_configuration,
+   std::shared_ptr 
content_repo, const std::string name, bool headless_mode,
+   std::shared_ptr 
filesystem)
 : 
core::controller::ControllerServiceProvider(core::getClassName()),
-  root_(nullptr),
+  c2::C2Client(std::move(configure), std::move(provenance_repo), 
std::move(flow_file_repo),
+   std::move(content_repo), std::move(flow_configuration), 
std::move(filesystem)),
   running_(false),
   updating_(false),
-  c2_enabled_(true),
   initialized_(false),
-  provenance_repo_(provenance_repo),
-  flow_file_repo_(flow_file_repo),
   
controller_service_map_(std::make_shared()),
   thread_pool_(2, false, nullptr, "Flowcontroller threadpool"),
-  flow_configuration_(std::move(flow_configuration)),
-  configuration_(std::move(configure)),
-  content_repo_(content_repo),
   logger_(logging::LoggerFactory::getLogger()) {
-  if (provenance_repo == nullptr)
+  if (IsNullOrEmpty(provenance_repo_))

Review comment:
   should I remove these newly added `IsNullOrEmpty` calls?
   
   (I agree that, with `string` it kind of made sense to use the empty string 
to indicate "no-string" (or some other special meaning), now that we have 
`optional<...>`, I am not so sure about that)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526036252



##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -873,6 +664,161 @@ void C2Agent::update_agent() {
   }
 }
 
+utils::TaskRescheduleInfo C2Agent::produce() {
+  // place priority on messages to send to the c2 server
+  if (protocol_.load() != nullptr) {
+std::vector payload_batch;
+payload_batch.reserve(max_c2_responses);
+auto getRequestPayload = [_batch] (C2Payload&& payload) { 
payload_batch.emplace_back(std::move(payload)); };
+for (std::size_t attempt_num = 0; attempt_num < max_c2_responses; 
++attempt_num) {
+  if (!requests.consume(getRequestPayload)) {
+break;
+  }
+}
+std::for_each(
+std::make_move_iterator(payload_batch.begin()),
+std::make_move_iterator(payload_batch.end()),
+[&] (C2Payload&& payload) {
+  try {
+C2Payload && response = 
protocol_.load()->consumePayload(std::move(payload));
+enqueue_c2_server_response(std::move(response));
+  }
+  catch(const std::exception ) {
+logger_->log_error("Exception occurred while consuming payload. 
error: %s", e.what());
+  }
+  catch(...) {
+logger_->log_error("Unknown exception occurred while consuming 
payload.");
+  }
+});
+
+try {
+  performHeartBeat();
+}
+catch (const std::exception ) {
+  logger_->log_error("Exception occurred while performing heartbeat. 
error: %s", e.what());
+}
+catch (...) {
+  logger_->log_error("Unknonwn exception occurred while performing 
heartbeat.");
+}
+  }
+
+  checkTriggers();
+
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
+}
+
+utils::TaskRescheduleInfo C2Agent::consume() {
+  const auto consume_success = responses.consume([this] (C2Payload&& payload) {
+extractPayload(std::move(payload));
+  });
+  if (!consume_success) {
+extractPayload(C2Payload{ Operation::HEARTBEAT });
+  }
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(C2RESPONSE_POLL_MS));
+}
+
+utils::optional C2Agent::fetchFlow(const std::string& uri) const {
+  if (!utils::StringUtils::startsWith(uri, "http") || protocol_.load() == 
nullptr) {
+// try to open the file
+utils::optional content = filesystem_->read(uri);
+if (content) {
+  return content;
+}
+  }
+  // couldn't open as file and we have no protocol to request the file from
+  if (protocol_.load() == nullptr) {
+return {};
+  }
+
+  std::string resolved_url = uri;
+  if (!utils::StringUtils::startsWith(uri, "http")) {
+std::stringstream adjusted_url;
+std::string base;
+if (configuration_->get(minifi::Configure::nifi_c2_flow_base_url, base)) {
+  adjusted_url << base;
+  if (!utils::StringUtils::endsWith(base, "/")) {
+adjusted_url << "/";
+  }
+  adjusted_url << uri;
+  resolved_url = adjusted_url.str();
+} else if (configuration_->get("c2.rest.url", base)) {
+  std::string host, protocol;
+  int port = -1;
+  utils::parse_url(, , , );
+  adjusted_url << protocol << host;
+  if (port > 0) {
+adjusted_url << ":" << port;
+  }
+  adjusted_url << "/c2/api/" << uri;
+  resolved_url = adjusted_url.str();
+}
+  }
+
+  C2Payload payload(Operation::TRANSFER, false, true);
+  C2Payload & = protocol_.load()->consumePayload(resolved_url, 
payload, RECEIVE, false);
+
+  auto raw_data = response.getRawData();
+  return std::string(raw_data.data(), raw_data.size());
+}
+
+bool C2Agent::handleConfigurationUpdate(const C2ContentResponse ) {
+  auto url = resp.operation_arguments.find("location");
+
+  std::string file_uri;
+  std::string configuration_str;
+
+  if (url == resp.operation_arguments.end()) {
+logger_->log_debug("Did not have location within %s", resp.ident);
+auto update_text = resp.operation_arguments.find("configuration_data");
+if (update_text == resp.operation_arguments.end()) {
+  logger_->log_debug("Neither the config file location nor the data is 
provided");
+  C2Payload response(Operation::ACKNOWLEDGE, 
state::UpdateState::SET_ERROR, resp.ident, false, true);
+  response.setRawData("Error while applying flow. Neither the config file 
location nor the data is provided.");
+  enqueue_c2_response(std::move(response));
+  return false;
+}
+configuration_str = update_text->second.to_string();
+  }
+
+  file_uri = url->second.to_string();
+  utils::optional optional_configuration_str = 
fetchFlow(file_uri);
+  if (!optional_configuration_str) {
+logger_->log_debug("Couldn't load new flow configuration from: \"%s\"", 
file_uri);
+C2Payload response(Operation::ACKNOWLEDGE, state::UpdateState::SET_ERROR, 
resp.ident, false, true);
+response.setRawData("Error while applying flow. Couldn't load flow

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526035745



##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -873,6 +664,161 @@ void C2Agent::update_agent() {
   }
 }
 
+utils::TaskRescheduleInfo C2Agent::produce() {
+  // place priority on messages to send to the c2 server
+  if (protocol_.load() != nullptr) {
+std::vector payload_batch;
+payload_batch.reserve(max_c2_responses);
+auto getRequestPayload = [_batch] (C2Payload&& payload) { 
payload_batch.emplace_back(std::move(payload)); };
+for (std::size_t attempt_num = 0; attempt_num < max_c2_responses; 
++attempt_num) {
+  if (!requests.consume(getRequestPayload)) {
+break;
+  }
+}
+std::for_each(
+std::make_move_iterator(payload_batch.begin()),
+std::make_move_iterator(payload_batch.end()),
+[&] (C2Payload&& payload) {
+  try {
+C2Payload && response = 
protocol_.load()->consumePayload(std::move(payload));
+enqueue_c2_server_response(std::move(response));
+  }
+  catch(const std::exception ) {
+logger_->log_error("Exception occurred while consuming payload. 
error: %s", e.what());
+  }
+  catch(...) {
+logger_->log_error("Unknown exception occurred while consuming 
payload.");
+  }
+});
+
+try {
+  performHeartBeat();
+}
+catch (const std::exception ) {
+  logger_->log_error("Exception occurred while performing heartbeat. 
error: %s", e.what());
+}
+catch (...) {
+  logger_->log_error("Unknonwn exception occurred while performing 
heartbeat.");
+}
+  }
+
+  checkTriggers();
+
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
+}
+
+utils::TaskRescheduleInfo C2Agent::consume() {
+  const auto consume_success = responses.consume([this] (C2Payload&& payload) {
+extractPayload(std::move(payload));
+  });
+  if (!consume_success) {
+extractPayload(C2Payload{ Operation::HEARTBEAT });
+  }
+  return 
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(C2RESPONSE_POLL_MS));
+}
+
+utils::optional C2Agent::fetchFlow(const std::string& uri) const {
+  if (!utils::StringUtils::startsWith(uri, "http") || protocol_.load() == 
nullptr) {
+// try to open the file
+utils::optional content = filesystem_->read(uri);
+if (content) {
+  return content;
+}
+  }
+  // couldn't open as file and we have no protocol to request the file from
+  if (protocol_.load() == nullptr) {
+return {};
+  }
+
+  std::string resolved_url = uri;
+  if (!utils::StringUtils::startsWith(uri, "http")) {
+std::stringstream adjusted_url;
+std::string base;
+if (configuration_->get(minifi::Configure::nifi_c2_flow_base_url, base)) {
+  adjusted_url << base;
+  if (!utils::StringUtils::endsWith(base, "/")) {
+adjusted_url << "/";
+  }
+  adjusted_url << uri;
+  resolved_url = adjusted_url.str();
+} else if (configuration_->get("c2.rest.url", base)) {
+  std::string host, protocol;
+  int port = -1;
+  utils::parse_url(, , , );
+  adjusted_url << protocol << host;
+  if (port > 0) {
+adjusted_url << ":" << port;
+  }
+  adjusted_url << "/c2/api/" << uri;
+  resolved_url = adjusted_url.str();
+}
+  }
+
+  C2Payload payload(Operation::TRANSFER, false, true);
+  C2Payload & = protocol_.load()->consumePayload(resolved_url, 
payload, RECEIVE, false);
+
+  auto raw_data = response.getRawData();
+  return std::string(raw_data.data(), raw_data.size());
+}
+
+bool C2Agent::handleConfigurationUpdate(const C2ContentResponse ) {
+  auto url = resp.operation_arguments.find("location");
+
+  std::string file_uri;
+  std::string configuration_str;
+
+  if (url == resp.operation_arguments.end()) {
+logger_->log_debug("Did not have location within %s", resp.ident);
+auto update_text = resp.operation_arguments.find("configuration_data");
+if (update_text == resp.operation_arguments.end()) {
+  logger_->log_debug("Neither the config file location nor the data is 
provided");
+  C2Payload response(Operation::ACKNOWLEDGE, 
state::UpdateState::SET_ERROR, resp.ident, false, true);
+  response.setRawData("Error while applying flow. Neither the config file 
location nor the data is provided.");
+  enqueue_c2_response(std::move(response));
+  return false;
+}
+configuration_str = update_text->second.to_string();
+  }
+
+  file_uri = url->second.to_string();
+  utils::optional optional_configuration_str = 
fetchFlow(file_uri);

Review comment:
   yes, previously "fetching" worked by creating a temporary file, writing 
into that, and returning the name of the file, if we are handling sensitive 
data (flow config which needs to be encrypted) persisting the raw data to 
however short amount of time seems dangerous, so now

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526034411



##
File path: libminifi/include/utils/file/PathUtils.h
##
@@ -62,6 +70,21 @@ inline bool isAbsolutePath(const char* const path) noexcept {
 #endif
 }
 
+inline utils::optional canonicalize(const std::string ) {

Review comment:
   it was in `FlowController::initializePaths` previously, its purpose is 
unclear to me





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526033553



##
File path: libminifi/src/FlowController.cpp
##
@@ -70,60 +71,33 @@ namespace apache {
 namespace nifi {
 namespace minifi {
 
-#define DEFAULT_CONFIG_NAME "conf/config.yml"
-
-FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo, 
std::shared_ptr configure,
-   std::unique_ptr 
flow_configuration, std::shared_ptr content_repo, 
const std::string name, bool headless_mode)
+FlowController::FlowController(std::shared_ptr 
provenance_repo, std::shared_ptr flow_file_repo,
+   std::shared_ptr configure, 
std::unique_ptr flow_configuration,
+   std::shared_ptr 
content_repo, const std::string name, bool headless_mode,
+   std::shared_ptr 
filesystem)
 : 
core::controller::ControllerServiceProvider(core::getClassName()),
-  root_(nullptr),
+  c2::C2Client(std::move(configure), std::move(provenance_repo), 
std::move(flow_file_repo),
+   std::move(content_repo), std::move(flow_configuration), 
std::move(filesystem)),
   running_(false),
   updating_(false),
-  c2_enabled_(true),
   initialized_(false),
-  provenance_repo_(provenance_repo),
-  flow_file_repo_(flow_file_repo),
   
controller_service_map_(std::make_shared()),
   thread_pool_(2, false, nullptr, "Flowcontroller threadpool"),
-  flow_configuration_(std::move(flow_configuration)),
-  configuration_(std::move(configure)),
-  content_repo_(content_repo),
   logger_(logging::LoggerFactory::getLogger()) {
-  if (provenance_repo == nullptr)
+  if (IsNullOrEmpty(provenance_repo_))

Review comment:
   the whole existence of `IsNullOrEmpty` is questionable to me, but its 
usage is ubiquitous, and since it was already used for `configuration_` I 
rewrote the others as well, I am not against getting rid of it, do you think we 
should do it, either only here, or in other places as well?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526030865



##
File path: libminifi/include/utils/file/PathUtils.h
##
@@ -17,12 +17,20 @@
 #ifndef LIBMINIFI_INCLUDE_UTILS_FILE_PATHUTILS_H_
 #define LIBMINIFI_INCLUDE_UTILS_FILE_PATHUTILS_H_
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include "utils/OptionalUtils.h"
+
+#ifdef _MSC_VER

Review comment:
   not clear to me, it was moved from `FlowController.cpp` (and defined the 
same way in `MainHelper.h`)





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526011840



##
File path: libminifi/include/utils/StringView.h
##
@@ -0,0 +1,122 @@
+/**
+ * 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.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class StringView {
+ public:
+  constexpr StringView(const char* begin, const char* end) : begin_(begin), 
end_(end) {}
+  explicit StringView(const char* str) : begin_(str), end_(begin_ + 
std::char_traits::length(str)) {}
+  explicit StringView(const std::string& str): begin_(&*str.begin()), 
end_(begin_ + str.length()) {}
+
+  constexpr const char* begin() const noexcept {
+return begin_;
+  }
+
+  constexpr const char* end() const noexcept {
+return end_;
+  }
+
+  constexpr bool empty() const noexcept {
+return begin_ == end_;
+  }
+
+  std::reverse_iterator rbegin() const noexcept {

Review comment:
   my understanding is that pointers are iterators (there is also partial 
specialization for `std::iterator_traits`), is there a utility in the STL that 
wraps raw pointers into something more "iterator-like"?





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526004650



##
File path: libminifi/include/utils/StringUtils.h
##
@@ -149,6 +150,17 @@ class StringUtils {
 return std::equal(endString.rbegin(), endString.rend(), value.rbegin());
   }
 
+  inline static bool startsWith(const std::string , const std::string & 
startString) {

Review comment:
   done, also moved `endsWith` into `StringView`





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526004415



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,192 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::simple_arguments_{
+{std::vector{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::flag_arguments_{
+{std::vector{"--help", "-h"},
+ "Prints this help message"},
+{std::vector{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+std::string Arguments::getHelp() {
+  std::stringstream ss;
+  ss << "Usage: " << "encrypt-config";
+  for (const auto& simple_arg : simple_arguments_) {
+ss << " ";
+if (!simple_arg.required) {
+  ss << "[";
+}
+ss << utils::StringUtils::join("|", simple_arg.names)
+<< " <" << simple_arg.value_name << ">";
+if (!simple_arg.required) {
+  ss << "]";
+}
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << " [" << utils::StringUtils::join("|", flag.names) << "]";
+  }
+  ss << std::endl;
+  for (const auto& simple_arg : simple_arguments_) {
+ss << "\t";
+ss << utils::StringUtils::join("|", simple_arg.names) << " : ";
+if (simple_arg.required) {
+  ss << "(required)";
+} else {
+  ss << "(optional)";
+}
+ss << " " << simple_arg.description;
+ss << std::endl;
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << "\t" << utils::StringUtils::join("|", flag.names) << " : "
+<< flag.description << std::endl;
+  }
+  return ss.str();
+}
+
+void Arguments::set(const std::string& key, const std::string& value) {
+  if (get(key)) {
+std::cerr << "Key is specified more that once \"" << key << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  simple_args_[key] = value;
+}
+
+void Arguments::set(const std::string& flag) {
+  if (isSet(flag)) {
+std::cerr << "Flag is specified more that once \"" << flag << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  flag_args_.insert(flag);
+}
+
+utils::optional Arguments::get(const std::string ) const {
+  utils::optional opt_arg = getSimpleArg(key);
+  if (!opt_arg) {
+return {};
+  }
+  for (const auto& name : opt_arg->names) {
+auto it = simple_args_.find(name);
+if (it != simple_args_.end()) {
+  return it->second;
+}
+  }
+  return {};
+}
+
+bool Arguments::isSet(const std::string ) const {
+  utils::optional opt_flag = getFlag(flag);
+  if (!opt_flag) {
+return false;
+  }
+  return std::any_of(opt_flag->names.begin(), opt_flag->names.end(), [&] 
(const std::string& name) {
+return flag_args_.find(name) != flag_args_.end();
+  });
+}
+
+Arguments Arguments::parse(int argc, char* argv[]) {
+  Arguments args;
+  for (int argIdx = 1; argIdx < argc; ++argIdx) {
+std::string key{argv[argIdx]};
+if (getFlag(key)) {
+  args.set(key);
+  continue;
+}
+if (!getSimpleArg(key)) {
+  std::cerr << "Unrecognized option: \"" << key << "\"" << std::endl;
+  std::cerr << getHelp();
+  std::exit(1);
+}
+if (argIdx == argc - 1) {
+  std::cerr << "No value specified for key \"" << key << "\"" << std::endl;
+  std::cerr << getHelp();
+  std::exit(1);
+}
+++argIdx;
+std::string value{argv[argIdx]};
+args.set(key, value);
+  }
+  if (args.isSet("-h")) {
+std::cout << getHelp();
+std::exit(0);
+  }
+  for (const auto& simple_arg : simple_arguments_) {
+if (simple_arg.required) {
+  bool found = false;
+  for (const auto& name : simple_arg.names) {
+if (args.get(name)) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {

Review comment:
   done

##
File path:

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526004228



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,192 @@
+/**
+ * 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 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::simple_arguments_{
+{std::vector{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::flag_arguments_{
+{std::vector{"--help", "-h"},
+ "Prints this help message"},
+{std::vector{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+std::string Arguments::getHelp() {
+  std::stringstream ss;
+  ss << "Usage: " << "encrypt-config";
+  for (const auto& simple_arg : simple_arguments_) {
+ss << " ";
+if (!simple_arg.required) {
+  ss << "[";
+}
+ss << utils::StringUtils::join("|", simple_arg.names)
+<< " <" << simple_arg.value_name << ">";
+if (!simple_arg.required) {
+  ss << "]";
+}
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << " [" << utils::StringUtils::join("|", flag.names) << "]";
+  }
+  ss << std::endl;
+  for (const auto& simple_arg : simple_arguments_) {
+ss << "\t";
+ss << utils::StringUtils::join("|", simple_arg.names) << " : ";
+if (simple_arg.required) {
+  ss << "(required)";
+} else {
+  ss << "(optional)";
+}
+ss << " " << simple_arg.description;
+ss << std::endl;
+  }
+  for (const auto& flag : flag_arguments_) {
+ss << "\t" << utils::StringUtils::join("|", flag.names) << " : "
+<< flag.description << std::endl;
+  }
+  return ss.str();
+}
+
+void Arguments::set(const std::string& key, const std::string& value) {
+  if (get(key)) {
+std::cerr << "Key is specified more that once \"" << key << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  simple_args_[key] = value;
+}
+
+void Arguments::set(const std::string& flag) {
+  if (isSet(flag)) {
+std::cerr << "Flag is specified more that once \"" << flag << "\"" << 
std::endl;
+std::cerr << getHelp();
+std::exit(1);
+  }
+  flag_args_.insert(flag);
+}
+
+utils::optional Arguments::get(const std::string ) const {
+  utils::optional opt_arg = getSimpleArg(key);
+  if (!opt_arg) {
+return {};
+  }
+  for (const auto& name : opt_arg->names) {
+auto it = simple_args_.find(name);
+if (it != simple_args_.end()) {
+  return it->second;
+}
+  }
+  return {};
+}
+
+bool Arguments::isSet(const std::string ) const {
+  utils::optional opt_flag = getFlag(flag);
+  if (!opt_flag) {
+return false;
+  }
+  return std::any_of(opt_flag->names.begin(), opt_flag->names.end(), [&] 
(const std::string& name) {
+return flag_args_.find(name) != flag_args_.end();
+  });
+}
+
+Arguments Arguments::parse(int argc, char* argv[]) {
+  Arguments args;
+  for (int argIdx = 1; argIdx < argc; ++argIdx) {
+std::string key{argv[argIdx]};
+if (getFlag(key)) {
+  args.set(key);
+  continue;
+}
+if (!getSimpleArg(key)) {
+  std::cerr << "Unrecognized option: \"" << key << "\"" << std::endl;
+  std::cerr << getHelp();
+  std::exit(1);
+}
+if (argIdx == argc - 1) {
+  std::cerr << "No value specified for key \"" << key << "\"" << std::endl;
+  std::cerr << getHelp();
+  std::exit(1);
+}
+++argIdx;
+std::string value{argv[argIdx]};
+args.set(key, value);
+  }
+  if (args.isSet("-h")) {
+std::cout << getHelp();
+std::exit(0);
+  }
+  for (const auto& simple_arg : simple_arguments_) {
+if (simple_arg.required) {
+  bool found = false;
+  for (const auto& name : simple_arg.names) {
+if (args.get(name)) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+std::cerr << "Missing required option [" <<

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526003991



##
File path: encrypt-config/EncryptConfig.cpp
##
@@ -90,33 +122,44 @@ std::string EncryptConfig::propertiesFilePath() const {
   MINIFI_PROPERTIES_FILE_NAME);
 }
 
-utils::crypto::Bytes EncryptConfig::getEncryptionKey() const {
+utils::crypto::EncryptionKeys EncryptConfig::getEncryptionKeys() const {
   encrypt_config::ConfigFile 
bootstrap_file{std::ifstream{bootstrapFilePath()}};
-  utils::optional key_from_bootstrap_file = 
bootstrap_file.getValue(ENCRYPTION_KEY_PROPERTY_NAME);
+  utils::optional decryption_key_hex = 
bootstrap_file.getValue(DECRYPTION_KEY_PROPERTY_NAME);
+  utils::optional encryption_key_hex = 
bootstrap_file.getValue(ENCRYPTION_KEY_PROPERTY_NAME);
+
+  utils::crypto::EncryptionKeys keys;
+  if (!decryption_key_hex || decryption_key_hex->empty()) {
+std::cout << "No decryption key was provided\n";

Review comment:
   removed log, hopefully replaced all references to a "deprecated 
key"/"decryption key" with "old key" or "old encryption key"

##
File path: encrypt-config/EncryptConfig.cpp
##
@@ -42,40 +42,72 @@ namespace nifi {
 namespace minifi {
 namespace encrypt_config {
 
-EncryptConfig::EncryptConfig(int argc, char* argv[]) : 
minifi_home_(parseMinifiHomeFromTheOptions(argc, argv)) {
+EncryptConfig::EncryptConfig(const std::string& minifi_home) : 
minifi_home_(minifi_home) {
   if (sodium_init() < 0) {
 throw std::runtime_error{"Could not initialize the libsodium library!"};
   }
+  keys_ = getEncryptionKeys();

Review comment:
   added comment





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526003406



##
File path: encrypt-config/EncryptConfig.cpp
##
@@ -42,40 +42,72 @@ namespace nifi {
 namespace minifi {
 namespace encrypt_config {
 
-EncryptConfig::EncryptConfig(int argc, char* argv[]) : 
minifi_home_(parseMinifiHomeFromTheOptions(argc, argv)) {
+EncryptConfig::EncryptConfig(const std::string& minifi_home) : 
minifi_home_(minifi_home) {
   if (sodium_init() < 0) {
 throw std::runtime_error{"Could not initialize the libsodium library!"};
   }
+  keys_ = getEncryptionKeys();
 }
 
-std::string EncryptConfig::parseMinifiHomeFromTheOptions(int argc, char* 
argv[]) {
-  if (argc >= 2) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-h") || (argstr == "--help")) {
-std::cout << USAGE_STRING << std::endl;
-std::exit(0);
-  }
-}
+EncryptConfig::EncryptionType EncryptConfig::encryptSensitiveProperties() 
const {
+  encryptSensitiveProperties(keys_);
+  if (keys_.decryption_key) {
+return EncryptionType::RE_ENCRYPT;
   }
+  return EncryptionType::ENCRYPT;
+}
 
-  if (argc >= 3) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-m") || (argstr == "--minifi-home")) {
-if (i+1 < argc) {
-  return std::string(argv[i+1]);
-}
-  }
-}
+void EncryptConfig::encryptFlowConfig() const {
+  encrypt_config::ConfigFile 
properties_file{std::ifstream{propertiesFilePath()}};
+  utils::optional config_path = 
properties_file.getValue(Configure::nifi_flow_configuration_file);
+  if (!config_path) {
+config_path = utils::file::PathUtils::resolve(minifi_home_, 
"conf/config.yml");

Review comment:
   moved these defaults to `libminifi/include/Defaults.h` and using that





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r526002968



##
File path: extensions/http-curl/tests/C2ConfigEncryption.cpp
##
@@ -0,0 +1,56 @@
+/**
+ *
+ * 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.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "utils/EncryptionProvider.h"
+
+int main(int argc, char **argv) {
+  const cmd_args args = parse_cmdline_args(argc, argv, "update");
+  TestController controller;
+  // copy config file to temporary location as it will get overridden
+  char tmp_format[] = "/var/tmp/c2.XX";
+  std::string home_path = controller.createTempDirectory(tmp_format);
+  std::string config_file = utils::file::FileUtils::concat_path(home_path, 
"config.yml");
+  utils::file::FileUtils::copy_file(args.test_file, config_file);
+  C2UpdateHandler handler(args.test_file);

Review comment:
   added comment, `C2UpdateHandler` is the mock c2 server, providing the 
original contents, while the encrypted file is persisted by `VerifyC2Update` 
(that one manages the flow) to the temporary `config_file`





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

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-11-18 Thread GitBox



adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r525902713



##
File path: encrypt-config/EncryptConfig.cpp
##
@@ -42,40 +42,72 @@ namespace nifi {
 namespace minifi {
 namespace encrypt_config {
 
-EncryptConfig::EncryptConfig(int argc, char* argv[]) : 
minifi_home_(parseMinifiHomeFromTheOptions(argc, argv)) {
+EncryptConfig::EncryptConfig(const std::string& minifi_home) : 
minifi_home_(minifi_home) {
   if (sodium_init() < 0) {
 throw std::runtime_error{"Could not initialize the libsodium library!"};
   }
+  keys_ = getEncryptionKeys();
 }
 
-std::string EncryptConfig::parseMinifiHomeFromTheOptions(int argc, char* 
argv[]) {
-  if (argc >= 2) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-h") || (argstr == "--help")) {
-std::cout << USAGE_STRING << std::endl;
-std::exit(0);
-  }
-}
+EncryptConfig::EncryptionType EncryptConfig::encryptSensitiveProperties() 
const {
+  encryptSensitiveProperties(keys_);
+  if (keys_.decryption_key) {
+return EncryptionType::RE_ENCRYPT;
   }
+  return EncryptionType::ENCRYPT;
+}
 
-  if (argc >= 3) {
-for (int i = 1; i < argc; ++i) {
-  std::string argstr(argv[i]);
-  if ((argstr == "-m") || (argstr == "--minifi-home")) {
-if (i+1 < argc) {
-  return std::string(argv[i+1]);
-}
-  }
-}
+void EncryptConfig::encryptFlowConfig() const {
+  encrypt_config::ConfigFile 
properties_file{std::ifstream{propertiesFilePath()}};
+  utils::optional config_path = 
properties_file.getValue(Configure::nifi_flow_configuration_file);
+  if (!config_path) {
+config_path = utils::file::PathUtils::resolve(minifi_home_, 
"conf/config.yml");
+std::cout << "Couldn't find path of configuration file, using default: \"" 
<< *config_path << "\"\n";
+  } else {
+config_path = utils::file::PathUtils::resolve(minifi_home_, *config_path);
+std::cout << "Encrypting flow configuration file: \"" << *config_path << 
"\"\n";
+  }
+  std::string config_content;
+  try {
+std::ifstream config_file{*config_path, std::ios::binary};
+config_file.exceptions(std::ios::failbit | std::ios::badbit);
+config_content = std::string{std::istreambuf_iterator(config_file), 
{}};
+  } catch (...) {
+std::cerr << "Error while reading flow configuration file \"" << 
*config_path << "\"\n";
+throw;
   }
+  try {
+utils::crypto::decrypt(config_content, keys_.encryption_key);
+std::cout << "Flow config file is already properly encrypted.\n";
+return;
+  } catch (const std::exception&) {}
 
-  throw std::runtime_error{USAGE_STRING};
-}
+  if (utils::crypto::isEncrypted(config_content)) {
+if (!keys_.decryption_key) {
+  std::cerr << "Config file is encrypted, but no deprecated key is set.\n";
+  std::exit(1);
+}
+std::cout << "Trying to decrypt flow config file using the deprecated key 
...\n";
+try {
+  config_content = utils::crypto::decrypt(config_content, 
*keys_.decryption_key);
+} catch (const std::exception&) {
+  std::cerr << "Flow config is encrypted, but couldn't be decrypted.\n";
+  std::exit(1);
+}
+  } else {
+std::cout << "Flow config file is not encrypted, using as-is.\n";

Review comment:
   it might be that an encrypted config is corrupted for whatever reason, 
and since we have no way of detecting that (short of trying to parse the yml) I 
figured it would raise some eyebrows if the user expects reencryption 





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

67 matches

Mail list logo