fgerlits commented on code in PR #1370: URL: https://github.com/apache/nifi-minifi-cpp/pull/1370#discussion_r947938125
########## libminifi/src/utils/net/SslServer.cpp: ########## @@ -0,0 +1,91 @@ +/** + * 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 "utils/net/SslServer.h" + +namespace org::apache::nifi::minifi::utils::net { + +SslSession::SslSession(asio::io_context& io_context, asio::ssl::context& context, utils::ConcurrentQueue<Message>& concurrent_queue, + std::optional<size_t> max_queue_size, std::shared_ptr<core::logging::Logger> logger) + : concurrent_queue_(concurrent_queue), + max_queue_size_(max_queue_size), Review Comment: minor, but `max_queue_size` could be moved instead of copied ########## PROCESSORS.md: ########## @@ -1254,13 +1254,15 @@ With parsing disabled all message will be routed to the success relationship, bu In the list below, the names of required properties appear in bold. Any other properties (not in bold) are considered optional. The table also indicates any default values, and whether a property supports the NiFi Expression Language. -| Name | Default Value | Allowable Values | Description | -|---------------------------|---------------|------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| Listening Port | 514 | | The port for Syslog communication. (Well-known ports (0-1023) require root access) | -| Protocol | UDP | UDP<br>TCP<br> | The protocol for Syslog communication. | -| Parse Messages | false | false<br>true | Indicates if the processor should parse the Syslog messages. If set to false, each outgoing FlowFile will only contain the sender, protocol, and port, and no additional attributes. | -| Max Batch Size | 500 | | The maximum number of Syslog events to process at a time. | -| Max Size of Message Queue | 10000 | | Maximum number of Syslog messages allowed to be buffered before processing them when the processor is triggered. If the buffer is full, the message is ignored. If set to zero the buffer is unlimited. | +| Name | Default Value | Allowable Values | Description | +|---------------------------|---------------|----------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Listening Port | 514 | | The port for Syslog communication. (Well-known ports (0-1023) require root access) | +| Protocol | UDP | UDP<br>TCP<br> | The protocol for Syslog communication. | +| Parse Messages | false | false<br>true | Indicates if the processor should parse the Syslog messages. If set to false, each outgoing FlowFile will only contain the sender, protocol, and port, and no additional attributes. | +| Max Batch Size | 500 | | The maximum number of Syslog events to process at a time. | +| Max Size of Message Queue | 10000 | | Maximum number of Syslog messages allowed to be buffered before processing them when the processor is triggered. If the buffer is full, the message is ignored. If set to zero the buffer is unlimited. | +| SSL Context Service | | | The Controller Service to use in order to obtain an SSL Context. If this property is set, messages will be received over a secure connection. | Review Comment: In `ListenSyslog.cpp`, there is one more sentence in the description. Should that be included here? ########## libminifi/test/Utils.h: ########## @@ -135,3 +142,37 @@ struct FlowFileQueueTestAccessor { FIELD_ACCESSOR(load_task_); FIELD_ACCESSOR(queue_); }; + +bool sendMessagesViaSSL(const std::vector<std::string_view>& contents, uint64_t port, const std::string& ca_cert_path, const std::optional<minifi::utils::net::SslData>& ssl_data = std::nullopt) { + asio::ssl::context ctx(asio::ssl::context::sslv23); + ctx.load_verify_file(ca_cert_path); + if (ssl_data) { + ctx.set_verify_mode(asio::ssl::verify_peer); + ctx.use_certificate_file(ssl_data->cert_loc, asio::ssl::context::pem); + ctx.use_private_key_file(ssl_data->key_loc, asio::ssl::context::pem); Review Comment: Do we also need to set the password callback? ########## libminifi/src/utils/net/SslServer.cpp: ########## @@ -0,0 +1,91 @@ +/** + * 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 "utils/net/SslServer.h" + +namespace org::apache::nifi::minifi::utils::net { + +SslSession::SslSession(asio::io_context& io_context, asio::ssl::context& context, utils::ConcurrentQueue<Message>& concurrent_queue, + std::optional<size_t> max_queue_size, std::shared_ptr<core::logging::Logger> logger) + : concurrent_queue_(concurrent_queue), + max_queue_size_(max_queue_size), + logger_(std::move(logger)), + socket_(io_context, context) { +} + +ssl_socket::lowest_layer_type& SslSession::getSocket() { + return socket_.lowest_layer(); +} + +void SslSession::start() { + socket_.async_handshake(asio::ssl::stream_base::server, + [this, self = shared_from_this()](const std::error_code& error_code) { + if (error_code) { + logger_->log_error("Error occured during SSL handshake: (%d) %s", error_code.value(), error_code.message()); + return; + } + asio::async_read_until(socket_, + buffer_, + '\n', + [self](const auto& error_code, size_t) -> void { + self->handleReadUntilNewLine(error_code); Review Comment: Why do we need `self` as a capture? Could we use `this` here instead? ########## extensions/standard-processors/tests/unit/ListenSyslogTests.cpp: ########## @@ -434,4 +434,35 @@ TEST_CASE("ListenSyslog max queue and max batch size test", "[ListenSyslog]") { CHECK(controller.trigger().at(ListenSyslog::Success).empty()); } -} // namespace org::apache::nifi::minifi::processors::testing +TEST_CASE("Test ListenSyslog via TCP with SSL connection", "[ListenSyslog]") { + const auto listen_syslog = std::make_shared<ListenSyslog>("ListenSyslog"); + + SingleProcessorTestController controller{listen_syslog}; + auto ssl_context_service = controller.plan->addController("SSLContextService", "SSLContextService"); + REQUIRE(controller.plan->setProperty(ssl_context_service, controllers::SSLContextService::CACertificate.getName(), + minifi::utils::file::FileUtils::get_executable_dir() + "/resources/ca_cert.crt")); Review Comment: Does this work on Windows? Probably better to use `utils::file::concat_path()`. ########## PROCESSORS.md: ########## @@ -1303,11 +1305,26 @@ Listens for incoming TCP connections and reads data from each connection using a In the list below, the names of required properties appear in bold. Any other properties (not in bold) are considered optional. The table also indicates any default values, and whether a property supports the NiFi Expression Language. -| Name | Default Value | Allowable Values | Description | -|-------------------------------|---------------|------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| **Listening Port** | | | The port to listen on for communication. | -| **Max Batch Size** | 500 | | The maximum number of messages to process at a time. | -| **Max Size of Message Queue** | 10000 | | Maximum number of messages allowed to be buffered before processing them when the processor is triggered. If the buffer is full, the message is ignored. If set to zero the buffer is unlimited. | +| Name | Default Value | Allowable Values | Description | +|-------------------------------|---------------|----------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **Listening Port** | | | The port to listen on for communication. | +| **Max Batch Size** | 500 | | The maximum number of messages to process at a time. | +| **Max Size of Message Queue** | 10000 | | Maximum number of messages allowed to be buffered before processing them when the processor is triggered. If the buffer is full, the message is ignored. If set to zero the buffer is unlimited. | +| SSL Context Service | | | The Controller Service to use in order to obtain an SSL Context. If this property is set, messages will be received over a secure connection. | +| Client Auth | NONE | NONE<br/>WANT<br/>REQUIRED | The client authentication policy to use for the SSL Context. Only used if an SSL Context Service is provided. | + +### Relationships + +| Name | Description | +|---------|--------------------------------------------------------------------| +| success | Messages received successfully will be sent out this relationship. | + +### Output Attributes + +| Attribute | Description | Requirements | +|--------------------------|--------------------------------------------------------------------|------------------------| +| _tcp.port_ | The sending host of the messages. | - | +| _tcp.sender_ | The sending port the messages were received. | - | Review Comment: The descriptions look like they should be switched. Also, there are some whitespace issues in line 1327. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
