Message source is missing for UDP transport based inputs #5293

bernd · 2018-11-19T14:49:49Z

Expected Behavior

Log messages coming in via UDP transports should have a source field.

Current Behavior

Since the big netty upgrade from 3 to 4, all messages that arrive via UDP netty transports do not have a source field anymore.

Possible Solution

Our RawMessageHandler is trying to get the remote address from the netty channel. This works for TCP channels but doesn't work for UDP channels. For UDP channels ctx.channel().remoteAddress() always returns null.

graylog2-server/graylog2-server/src/main/java/org/graylog2/inputs/transports/netty/RawMessageHandler.java

Lines 40 to 45 in 12f456e

    
           protected void channelRead0(ChannelHandlerContext ctx, ByteBuf msg) throws Exception { 
        
               final byte[] bytes = new byte[msg.readableBytes()]; 
        
               msg.readBytes(bytes); 
        
               final RawMessage raw = new RawMessage(bytes, (InetSocketAddress) ctx.channel().remoteAddress()); 
        
               input.processRawMessage(raw); 
        
           }

The remote address for UDP datagrams can be obtained via DatagramPacket#sender(). The problem is, that our DatagramPacketHandler and NetflowMessageAggregationHandler only pass on the actual payload byte buffer to subsequent netty handlers. That means we cannot get to the datagram packet sender in the RawMessageHandler later.

One possible solution could be to create a small container object that holds the payload byte buffer and the remote address instead of just passing on the payload byte buffer to subsequent netty handlers.
Another option might be to use netty channel attributes to set the remote address when still having access to the datagram packet and then read it in the RawMessageHandler. But that might be problematic because the same netty channel is used for all incoming packets so we might run into race conditions.

Steps to Reproduce (for bugs)

Create raw UDP input
Send a UDP message via echo udp test | ncat -u 127.0.0.1 5555
Search for the message
Check "source" field. It's set to "unknown"

Context

This basically breaks all messages that are coming in via UDP and which don't set a custom source!

Your Environment

Graylog Version: 3.0.0-alpha.4

The text was updated successfully, but these errors were encountered:

dennisoelkers · 2018-11-19T14:58:31Z

This is most probably also the underlying cause of #5264.

bernd · 2018-11-19T15:03:04Z

This is most probably also the underlying cause of #5264.

@dennisoelkers Yes! Thanks for the heads up!

jam49 · 2019-01-08T22:24:07Z

This also appears broken for TCP syslog inputs in 2.5.1, works fine in 2.4.x; source attribute is blank. reverting to 2.4.6 fixes the problem.

This fixes an issue that has been introduced with the big Netty 4.1 update in PR #4397. In Netty 3.x we always passed a "MessageEvent" through the channel pipeline and got the remote address from that object. Since this object doesn't exist anymore in Netty 4.x, we only pass the message payload's "ByteBuf" through the pipeline and rely on the "Channel#getRemoteAddress" method to always return the remote address object. The problem is that this does indeed work for TCP based channels but it doesn't work for UDP based channels. For UDP channels the "#getRemoteAddress()" method always returns "null". This is probably due to the connection-less nature of UDP. For UDP transports Netty only creates a single channel. For TCP transports there is one channel per TCP connection To fix this we need to get our hands on the remote address when we create the "RawMessage" object at the very end of the Netty pipeline. Since we only pass the message payload "ByteBuf" through the Netty pipeline, we could previously reuse several classes for TCP and UDP transports because they were basically the same. For UDP transports we now need to carry the remote address through the pipeline by using a "AddressedEnvelope" (available in Netty) that takes a payload and a sender/receiver object. That means we have to create a few UDP specific - or rather "AddressedEnvelope" specific - pipeline handlers because the shared ones only know how to handle "ByteBuf" messages. This PR moves some shared code out of the "NettyTransport" class up to "AbstractTcpTransport" and "UdpTransport" so we can customize the pipeline for the two different payload types. It also creates new message aggregation handlers for the "AddressedEnvelope" objects. In the future we can probably refactor this to share some more code, but for 3.0 I tried to change as few as possible. The TCP pipeline is basically unchanged apart from the "AbstractTcpTransport" change. Fixes #5264 Fixes #5293

This fixes an issue that has been introduced with the big Netty 4.1 update in PR #4397. In Netty 3.x we always passed a "MessageEvent" through the channel pipeline and got the remote address from that object. Since this object doesn't exist anymore in Netty 4.x, we only pass the message payload's "ByteBuf" through the pipeline and rely on the "Channel#getRemoteAddress" method to always return the remote address object. The problem is that this does indeed work for TCP based channels but it doesn't work for UDP based channels. For UDP channels the "#getRemoteAddress()" method always returns "null". This is probably due to the connection-less nature of UDP. For UDP transports Netty only creates a single channel. For TCP transports there is one channel per TCP connection To fix this we need to get our hands on the remote address when we create the "RawMessage" object at the very end of the Netty pipeline. Since we only pass the message payload "ByteBuf" through the Netty pipeline, we could previously reuse several classes for TCP and UDP transports because they were basically the same. For UDP transports we now need to carry the remote address through the pipeline by using a "AddressedEnvelope" (available in Netty) that takes a payload and a sender/receiver object. That means we have to create a few UDP specific - or rather "AddressedEnvelope" specific - pipeline handlers because the shared ones only know how to handle "ByteBuf" messages. This PR moves some shared code out of the "NettyTransport" class up to "AbstractTcpTransport" and "UdpTransport" so we can customize the pipeline for the two different payload types. It also creates new message aggregation handlers for the "AddressedEnvelope" objects. In the future we can probably refactor this to share some more code, but for 3.0 I tried to change as few as possible. The TCP pipeline is basically unchanged apart from the "AbstractTcpTransport" change. Fixes #5264 Fixes #5293 **Note:** This needs to be cherry-picked into 3.0

This fixes an issue that has been introduced with the big Netty 4.1 update in PR #4397. In Netty 3.x we always passed a "MessageEvent" through the channel pipeline and got the remote address from that object. Since this object doesn't exist anymore in Netty 4.x, we only pass the message payload's "ByteBuf" through the pipeline and rely on the "Channel#getRemoteAddress" method to always return the remote address object. The problem is that this does indeed work for TCP based channels but it doesn't work for UDP based channels. For UDP channels the "#getRemoteAddress()" method always returns "null". This is probably due to the connection-less nature of UDP. For UDP transports Netty only creates a single channel. For TCP transports there is one channel per TCP connection To fix this we need to get our hands on the remote address when we create the "RawMessage" object at the very end of the Netty pipeline. Since we only pass the message payload "ByteBuf" through the Netty pipeline, we could previously reuse several classes for TCP and UDP transports because they were basically the same. For UDP transports we now need to carry the remote address through the pipeline by using a "AddressedEnvelope" (available in Netty) that takes a payload and a sender/receiver object. That means we have to create a few UDP specific - or rather "AddressedEnvelope" specific - pipeline handlers because the shared ones only know how to handle "ByteBuf" messages. This PR moves some shared code out of the "NettyTransport" class up to "AbstractTcpTransport" and "UdpTransport" so we can customize the pipeline for the two different payload types. It also creates new message aggregation handlers for the "AddressedEnvelope" objects. In the future we can probably refactor this to share some more code, but for 3.0 I tried to change as few as possible. The TCP pipeline is basically unchanged apart from the "AbstractTcpTransport" change. Fixes #5264 Fixes #5293 **Note:** This needs to be cherry-picked into 3.0 (cherry picked from commit 375e618)

…5519) This fixes an issue that has been introduced with the big Netty 4.1 update in PR #4397. In Netty 3.x we always passed a "MessageEvent" through the channel pipeline and got the remote address from that object. Since this object doesn't exist anymore in Netty 4.x, we only pass the message payload's "ByteBuf" through the pipeline and rely on the "Channel#getRemoteAddress" method to always return the remote address object. The problem is that this does indeed work for TCP based channels but it doesn't work for UDP based channels. For UDP channels the "#getRemoteAddress()" method always returns "null". This is probably due to the connection-less nature of UDP. For UDP transports Netty only creates a single channel. For TCP transports there is one channel per TCP connection To fix this we need to get our hands on the remote address when we create the "RawMessage" object at the very end of the Netty pipeline. Since we only pass the message payload "ByteBuf" through the Netty pipeline, we could previously reuse several classes for TCP and UDP transports because they were basically the same. For UDP transports we now need to carry the remote address through the pipeline by using a "AddressedEnvelope" (available in Netty) that takes a payload and a sender/receiver object. That means we have to create a few UDP specific - or rather "AddressedEnvelope" specific - pipeline handlers because the shared ones only know how to handle "ByteBuf" messages. This PR moves some shared code out of the "NettyTransport" class up to "AbstractTcpTransport" and "UdpTransport" so we can customize the pipeline for the two different payload types. It also creates new message aggregation handlers for the "AddressedEnvelope" objects. In the future we can probably refactor this to share some more code, but for 3.0 I tried to change as few as possible. The TCP pipeline is basically unchanged apart from the "AbstractTcpTransport" change. Fixes #5264 Fixes #5293 **Note:** This needs to be cherry-picked into 3.0 (cherry picked from commit 375e618)

bernd added bug blocker If not finished by release date, the release will be postponed. labels Nov 19, 2018

bernd added this to the 3.0.0 milestone Nov 19, 2018

bernd self-assigned this Nov 19, 2018

bernd mentioned this issue Nov 19, 2018

Syslog UDP input does not set gl2_remote_ip in Graylog 3.0 #5264

Closed

jalogisch added the triaged label Dec 10, 2018

bernd added the #M > 1d <= 3d label Jan 3, 2019

bernd mentioned this issue Jan 15, 2019

Fix missing "source" field for UDP transport based messages #5512

Merged

kroepke closed this as completed in #5512 Jan 15, 2019

bernd mentioned this issue Jan 16, 2019

Fix missing "source" field for UDP transport based messages (#5512) #5519

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Message source is missing for UDP transport based inputs #5293

Message source is missing for UDP transport based inputs #5293

bernd commented Nov 19, 2018

dennisoelkers commented Nov 19, 2018

bernd commented Nov 19, 2018

jam49 commented Jan 8, 2019 •

edited

Loading

Message source is missing for UDP transport based inputs #5293

Message source is missing for UDP transport based inputs #5293

Comments

bernd commented Nov 19, 2018

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce (for bugs)

Context

Your Environment

dennisoelkers commented Nov 19, 2018

bernd commented Nov 19, 2018

jam49 commented Jan 8, 2019 • edited Loading

jam49 commented Jan 8, 2019 •

edited

Loading