You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/03/28 10:22:11 UTC
[GitHub] [nifi] pgyori opened a new pull request #5908: NIFI-9838: Authorized Subject/Issuer DN Pattern properties added to ListenTCPRecord processor
pgyori opened a new pull request #5908:
URL: https://github.com/apache/nifi/pull/5908
https://issues.apache.org/jira/browse/NIFI-9838
<!--
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.
-->
#### Description of PR
Implements NIFI-9838. ListenTCPRecord processor has two new properties: 'Authorized Subject DN Pattern' and 'Authorized Subject DN Pattern'. The processor adds the values of these properties to the outgoing flowfiles as attributes. The processor also adds the client certificate's Subject DN and Issuer DN values to the outgoing flowfiles as attributes in case of secure connections.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
- [ ] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] Have you verified that the full build is successful on JDK 8?
- [ ] Have you verified that the full build is successful on JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
- [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory closed pull request #5908: NIFI-9838: ListenTCPRecord adds client certificate's Subject and Issuer DNs to flowfiles as attributes
Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #5908:
URL: https://github.com/apache/nifi/pull/5908
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pgyori commented on pull request #5908: NIFI-9838: ListenTCPRecord adds client certificate's Subject and Issuer DNs to flowfiles as attributes
Posted by GitBox <gi...@apache.org>.
pgyori commented on pull request #5908:
URL: https://github.com/apache/nifi/pull/5908#issuecomment-1080837384
Thank you @exceptionfactory ! Special thanks for carefully reading through the documentation I wrote, and for the recommended clarifications. I applied all your 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.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #5908: NIFI-9838: ListenTCPRecord adds client certificate's Subject and Issuer DNs to flowfiles as attributes
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #5908:
URL: https://github.com/apache/nifi/pull/5908#discussion_r836493435
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -460,4 +480,23 @@ private SocketChannelRecordReader pollForSocketRecordReader() {
private String getRemoteAddress(final SocketChannelRecordReader socketChannelRecordReader) {
return socketChannelRecordReader.getRemoteAddress() == null ? "null" : socketChannelRecordReader.getRemoteAddress().toString();
}
+
+ private void addClientCertificateDNsToAttributes(final Map<String, String> attributes, final SocketChannelRecordReader socketRecordReader)
+ throws SSLPeerUnverifiedException {
+ if (socketRecordReader instanceof SSLSocketChannelRecordReader) {
+ SSLSocketChannelRecordReader sslSocketRecordReader = (SSLSocketChannelRecordReader) socketRecordReader;
+ SSLSession sslSession = sslSocketRecordReader.getSession();
+ try {
+ Certificate[] certificates = sslSession.getPeerCertificates();
+ if (certificates.length > 0) {
+ X509Certificate certificate = (X509Certificate) certificates[0];
+ attributes.put(CLIENT_CERTIFICATE_SUBJECT_DN_ATTRIBUTE, certificate.getSubjectDN().toString());
+ attributes.put(CLIENT_CERTIFICATE_ISSUER_DN_ATTRIBUTE, certificate.getIssuerDN().toString());
+ }
+ } catch (SSLPeerUnverifiedException peerUnverifiedException) {
+ getLogger().debug("Peer not authenticated, " + CLIENT_CERTIFICATE_SUBJECT_DN_ATTRIBUTE + " and " +
+ CLIENT_CERTIFICATE_ISSUER_DN_ATTRIBUTE + " are not added to the outgoing FlowFile.");
Review comment:
It would be helpful to include the remote address and the exception for troubleshooting. Logging the attribute names is not necessary, so recommend the following approach:
```suggestion
getLogger().debug("Remote Peer [{}] not verified: client certificates not provided", socketRecordReader. getRemoteAddress(), peerUnverifiedException);
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -460,4 +480,23 @@ private SocketChannelRecordReader pollForSocketRecordReader() {
private String getRemoteAddress(final SocketChannelRecordReader socketChannelRecordReader) {
return socketChannelRecordReader.getRemoteAddress() == null ? "null" : socketChannelRecordReader.getRemoteAddress().toString();
}
+
+ private void addClientCertificateDNsToAttributes(final Map<String, String> attributes, final SocketChannelRecordReader socketRecordReader)
Review comment:
Recommend shortening to the following:
```suggestion
private void addClientCertificateAttributes(final Map<String, String> attributes, final SocketChannelRecordReader socketRecordReader)
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
+ "issuer and subject are added to the outgoing FlowFiles as attributes. " +
+ "The processor does not perform authorization based on Distinguished Name values, but since these values " +
+ "are attached to the outgoing FlowFiles, authorization can be implemented in the downstream flow using a RouteOnAttribute processor.")
@WritesAttributes({
@WritesAttribute(attribute="tcp.sender", description="The host that sent the data."),
@WritesAttribute(attribute="tcp.port", description="The port that the processor accepted the connection on."),
@WritesAttribute(attribute="record.count", description="The number of records written to the flow file."),
- @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file.")
+ @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file."),
+ @WritesAttribute(attribute="client.certificate.issuer.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
+ "Certificate Authority that issued the client's certificate " +
+ "is attached to the FlowFile as the value of the " +
+ "'client.certificate.issuer.dn' attribute."),
+ @WritesAttribute(attribute="client.certificate.subject.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
+ "client certificate's owner (subject) is attached to the FlowFile " +
+ "as the value of the client.certificate.subject.dn attribute")
Review comment:
```suggestion
"client certificate's owner (subject) is attached to the FlowFile.")
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
+ "issuer and subject are added to the outgoing FlowFiles as attributes. " +
+ "The processor does not perform authorization based on Distinguished Name values, but since these values " +
+ "are attached to the outgoing FlowFiles, authorization can be implemented in the downstream flow using a RouteOnAttribute processor.")
Review comment:
Recommend leaving off the specific processor reference and making it more generic:
```suggestion
"are attached to the outgoing FlowFiles, authorization can be implemented based on these attributes.")
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
Review comment:
The client certificate is only available for mutual TLS, and it would not be available when client authenticated is disabled. Recommend the following wording for clarity:
```suggestion
"When connected clients present certificates for mutual TLS authentication, the Distinguished Names of the client certificate's " +
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
+ "issuer and subject are added to the outgoing FlowFiles as attributes. " +
+ "The processor does not perform authorization based on Distinguished Name values, but since these values " +
+ "are attached to the outgoing FlowFiles, authorization can be implemented in the downstream flow using a RouteOnAttribute processor.")
@WritesAttributes({
@WritesAttribute(attribute="tcp.sender", description="The host that sent the data."),
@WritesAttribute(attribute="tcp.port", description="The port that the processor accepted the connection on."),
@WritesAttribute(attribute="record.count", description="The number of records written to the flow file."),
- @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file.")
+ @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file."),
+ @WritesAttribute(attribute="client.certificate.issuer.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
+ "Certificate Authority that issued the client's certificate " +
+ "is attached to the FlowFile as the value of the " +
+ "'client.certificate.issuer.dn' attribute."),
+ @WritesAttribute(attribute="client.certificate.subject.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
Review comment:
```suggestion
@WritesAttribute(attribute="client.certificate.subject.dn", description="For connections using mutual TLS, the Distinguished Name of the " +
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
+ "issuer and subject are added to the outgoing FlowFiles as attributes. " +
+ "The processor does not perform authorization based on Distinguished Name values, but since these values " +
+ "are attached to the outgoing FlowFiles, authorization can be implemented in the downstream flow using a RouteOnAttribute processor.")
@WritesAttributes({
@WritesAttribute(attribute="tcp.sender", description="The host that sent the data."),
@WritesAttribute(attribute="tcp.port", description="The port that the processor accepted the connection on."),
@WritesAttribute(attribute="record.count", description="The number of records written to the flow file."),
- @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file.")
+ @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file."),
+ @WritesAttribute(attribute="client.certificate.issuer.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
Review comment:
Recommend adjust the wording for clarity:
```suggestion
@WritesAttribute(attribute="client.certificate.issuer.dn", description="For connections using mutual TLS, the Distinguished Name of the " +
```
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenTCPRecord.java
##########
@@ -87,14 +93,28 @@
"If the read times out, or if any other error is encountered when reading, the connection will be closed, and any records " +
"read up to that point will be handled according to the configured Read Error Strategy (Discard or Transfer). In cases where " +
"clients are keeping a connection open, the concurrent tasks for the processor should be adjusted to match the Max Number of " +
- "TCP Connections allowed, so that there is a task processing each connection.")
+ "TCP Connections allowed, so that there is a task processing each connection. " +
+ "The processor can be configured to use an SSL Context Service to only allow secure connections. " +
+ "When a client connects to the processor using secure connection, the Distinguished Names of the client certificate's " +
+ "issuer and subject are added to the outgoing FlowFiles as attributes. " +
+ "The processor does not perform authorization based on Distinguished Name values, but since these values " +
+ "are attached to the outgoing FlowFiles, authorization can be implemented in the downstream flow using a RouteOnAttribute processor.")
@WritesAttributes({
@WritesAttribute(attribute="tcp.sender", description="The host that sent the data."),
@WritesAttribute(attribute="tcp.port", description="The port that the processor accepted the connection on."),
@WritesAttribute(attribute="record.count", description="The number of records written to the flow file."),
- @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file.")
+ @WritesAttribute(attribute="mime.type", description="The mime-type of the writer used to write the records to the flow file."),
+ @WritesAttribute(attribute="client.certificate.issuer.dn", description="If data is sent via secure connection, the Distinguished Name of the " +
+ "Certificate Authority that issued the client's certificate " +
+ "is attached to the FlowFile as the value of the " +
+ "'client.certificate.issuer.dn' attribute."),
Review comment:
It seems unnecessary to repeat the attribute name:
```suggestion
"is attached to the FlowFile."),
```
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org