The NTP Detector is broken

Description

The current implementation of the NTP detector requires specify the IP address that must be used to validate the message returned by the NTP server.

This doesn't make any sense because that IP address must be taken from the class that uses the detector (which contains the information of the node that is being processed). Otherwise, when defining the detector on the foreign sources, it will work only on one specific node from the requisition even if the desired result is to discover the NTP server across several nodes from the requisition.

This information is actually inside the NtpClient class used by the NtpDetector, so the easiest way is to define a private variable of the NtpClient inside the NtpDetector and then use it on the validation procedure to verify the package returned by the server.

But, validate the IP is the only thing validated by the detector which in my opinion is not enough, we should verify that the package is a valid NTP package in order to be sure that we are talking with a real NTP server.

Acceptance / Success Criteria

None

Attachments

1
  • 05 Dec 2012, 05:52 PM

Lucidchart Diagrams

Activity

Show:

Alejandro Galue December 7, 2012 at 9:43 AM

The proposed solution is very similar to the attached patch but include a much better validation.

Committed on revision 55617ce8baf3409d4a240e09304555a71dee77b3 for 1.10

Alejandro Galue December 5, 2012 at 5:52 PM

Attached is the proposed solution for your review.

Alejandro Galue December 5, 2012 at 5:47 PM

Right now, you can see that the NTP server is responding but as the IP is not the same, the detector returns that the service was not detected:

2012-12-05 11:36:28,735 DEBUG [scanExecutor-5] NtpClient: Address: /10.0.2.15, port: 123, timeout: 2000 2012-12-05 11:36:28,735 INFO [scanExecutor-5] NtpDetector: isServiceDetected: Attempting to connect to address: 10.0.2.15, port: 123, attempt: #1 2012-12-05 11:36:28,735 INFO [scanExecutor-5] ClientConversation: Sending Request Leap indicator: 0 Version: 3 Mode: 3 Stratum: 0 Poll: 0 Precision: 0 (1E0 seconds) Root delay: 0.00 ms Root dispersion: 0.00 ms Reference identifier: ^@^@^@^@ Reference timestamp: 0 Originate timestamp: 0 Receive timestamp: 0 Transmit timestamp: 05-Dec-2012 11:36:28.735000 2012-12-05 11:36:28,735 INFO [scanExecutor-5] ClientConversation: Received Response java.net.DatagramPacket@4617d1d2 2012-12-05 11:36:28,735 INFO [scanExecutor-5] IpInterfaceScan$1: Attempted to detect service NTP on address 10.0.2.15: false

If the parameter named ipToValidate is added to the detector definition, the detection passes and the service is added only to the node with that IP address (which seems to be odd for me).

I made the proper changes and besides validate the IP address from the NtpClient class, I've added a validation of one of the parameters returned by a valid NTP package, just to be sure that the package was sent by a real NTP server, and it works fine.

Fixed

Details

Assignee

Reporter

Labels

Fix versions

Affects versions

Priority

PagerDuty

Created December 5, 2012 at 5:39 PM
Updated January 27, 2017 at 4:20 PM
Resolved December 7, 2012 at 9:43 AM

Flag notifications