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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
