From b791b6765c656d289b01ce0997ec08f62b402b2f Mon Sep 17 00:00:00 2001 From: Jorge Antonio Diaz-Benito Soriano Date: Fri, 20 Oct 2023 15:59:05 +0200 Subject: [PATCH] Fix multiple bugs in NTP packet serialization --- .../tidal/networktime/internal/NtpExchanger.kt | 6 +++--- .../tidal/networktime/internal/NtpPacket.kt | 18 +++++++++--------- .../internal/NtpPacketDeserializer.kt | 6 +++--- .../internal/NtpPacketSerializer.kt | 14 ++++++++++---- .../tidal/networktime/internal/SyncPeriodic.kt | 5 +++-- .../tidal/networktime/internal/SyncSingular.kt | 8 ++++---- .../commonMain/kotlin/root/MainViewModel.kt | 1 - 7 files changed, 32 insertions(+), 26 deletions(-) diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpExchanger.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpExchanger.kt index 08d85ef5..320c14f4 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpExchanger.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpExchanger.kt @@ -18,13 +18,13 @@ internal class NtpExchanger( ): NtpExchangeResult? { val ntpUdpSocketOperations = NtpUdpSocketOperations() val requestPacket = NtpPacket( - versionNumber = ntpVersion.toByte(), - mode = NTP_MODE_CLIENT.toByte(), + versionNumber = ntpVersion.toInt(), + mode = NTP_MODE_CLIENT, ) return try { ntpUdpSocketOperations.prepareSocket(queryTimeout.inWholeMilliseconds) val requestTime = referenceClock.referenceEpochTime - val buffer = ntpPacketSerializer(requestPacket.copy(originateEpochTimestamp = requestTime)) + val buffer = ntpPacketSerializer(requestPacket.copy(transmitEpochTimestamp = requestTime)) ntpUdpSocketOperations.exchangePacketInPlace( buffer, address, diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacket.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacket.kt index dc417d16..f986bb9e 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacket.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacket.kt @@ -4,19 +4,19 @@ import kotlin.time.Duration import kotlin.time.Duration.Companion.days internal data class NtpPacket( - val leapIndicator: Byte = 0, - val versionNumber: Byte = 0, - val mode: Byte = 0, + val leapIndicator: Int = 0, + val versionNumber: Int = 0, + val mode: Int = 0, val stratum: Byte = 0, val poll: Byte = 0, val precision: Byte = 0, - val rootDelay: Duration = Duration.ZERO, - val rootDispersion: Duration = Duration.ZERO, + val rootDelay: Duration = Duration.INFINITE, + val rootDispersion: Duration = Duration.INFINITE, val referenceIdentifier: Int = 0, - val referenceEpochTimestamp: Duration = Duration.ZERO, - val originateEpochTimestamp: Duration = Duration.ZERO, - val receiveEpochTimestamp: Duration = Duration.ZERO, - val transmitEpochTimestamp: Duration = Duration.ZERO, + val referenceEpochTimestamp: Duration = Duration.INFINITE, + val originateEpochTimestamp: Duration = Duration.INFINITE, + val receiveEpochTimestamp: Duration = Duration.INFINITE, + val transmitEpochTimestamp: Duration = Duration.INFINITE, ) { init { // Check sizes of fields whose type does not match their corresponding size in the actual packet diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketDeserializer.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketDeserializer.kt index b63f5ff4..dfa498d0 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketDeserializer.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketDeserializer.kt @@ -9,9 +9,9 @@ internal class NtpPacketDeserializer { operator fun invoke(bytes: ByteArray): NtpPacket { var index = 0 return NtpPacket( - ((bytes[index++].toInt() shl 8) + bytes[index++]).toByte(), - ((bytes[index++].toInt() shl 16) + (bytes[index++].toInt() shl 24) + bytes[index++]).toByte(), - ((bytes[index++].toInt() shl 16) + (bytes[index++].toInt() shl 24) + bytes[index++]).toByte(), + (bytes[index++].toInt() shl 8) + bytes[index++], + (bytes[index++].toInt() shl 16) + (bytes[index++].toInt() shl 24) + bytes[index++], + (bytes[index++].toInt() shl 16) + (bytes[index++].toInt() shl 24) + bytes[index++], bytes[index++], bytes[index++], bytes[index++], diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketSerializer.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketSerializer.kt index b0d8125d..a0f3890a 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketSerializer.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/NtpPacketSerializer.kt @@ -1,12 +1,12 @@ package com.tidal.networktime.internal +import kotlin.random.Random import kotlin.time.Duration -internal class NtpPacketSerializer { +internal class NtpPacketSerializer(private val random: Random) { operator fun invoke(ntpPacket: NtpPacket) = with(ntpPacket) { byteArrayOf( - ((leapIndicator.toInt() and 0b11) or (versionNumber.toInt() and 0b110)).toByte(), - ((versionNumber.toInt() and 0b1) or (mode.toInt() and 0b111)).toByte(), + ((leapIndicator shl 6) or (versionNumber shl 3) or mode).toByte(), stratum, poll, precision, @@ -25,6 +25,9 @@ internal class NtpPacketSerializer { private val Duration.asIntervalToNtpInterval: ByteArray get() { + if (this == Duration.INFINITE) { + return ByteArray(4) + } val wholeSeconds = inWholeSeconds val fraction = (inWholeMilliseconds - wholeSeconds * 1_000) * (1 shl 16) / 1_000 return byteArrayOf( @@ -37,6 +40,9 @@ internal class NtpPacketSerializer { private val Duration.asEpochTimestampToNtpEpochTimestamp: ByteArray get() { + if (this == Duration.INFINITE) { + return ByteArray(8) + } val seconds = (this + NtpPacket.NTP_EPOCH_OFFSET_WITH_EPOCH).inWholeSeconds val fraction = (inWholeMilliseconds - inWholeSeconds * 1_000) * 0b100000000000000000000000000000000 / @@ -49,7 +55,7 @@ internal class NtpPacketSerializer { (fraction shr 24).toByte(), (fraction shr 16).toByte(), (fraction shr 8).toByte(), - fraction.toByte(), + random.nextBytes(1).single(), ) } } diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncPeriodic.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncPeriodic.kt index 1e3cbe8a..37c059aa 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncPeriodic.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncPeriodic.kt @@ -16,11 +16,12 @@ internal class SyncPeriodic( HttpClientFactory()() { install(HttpTimeout) }, DnsOverHttpsResponseParser(), ), + random: Random = Random.Default, private val ntpExchanger: NtpExchanger = NtpExchanger( referenceClock, - NtpPacketSerializer(), + NtpPacketSerializer(random), NtpPacketDeserializer(), - Random.Default, + random, ), ) { suspend operator fun invoke() { diff --git a/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncSingular.kt b/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncSingular.kt index 817d6681..42647716 100644 --- a/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncSingular.kt +++ b/library/src/commonMain/kotlin/com/tidal/networktime/internal/SyncSingular.kt @@ -26,11 +26,11 @@ internal class SyncSingular( .sortedBy { it.clockOffset } .run { if (isEmpty()) { - null + return } else { this[size / 2] } - } ?: return + } mutableState.synchronizationResult = SynchronizationResult( selectedResult.run { timeMeasured + clockOffset }, referenceClock.referenceEpochTime, @@ -47,7 +47,7 @@ internal class SyncSingular( }, lookupTimeout, ).map { address -> - (1..queriesPerResolvedAddress).map { + (1..queriesPerResolvedAddress).mapNotNull { val ret = ntpExchanger( address, queryTimeout, @@ -62,7 +62,7 @@ internal class SyncSingular( ) delay(waitBetweenResolvedAddressQueries) ret - }.filterNotNull() + } .takeIf { it.isNotEmpty() } ?.minBy { it.roundTripDelay } } diff --git a/samples/shared/src/commonMain/kotlin/root/MainViewModel.kt b/samples/shared/src/commonMain/kotlin/root/MainViewModel.kt index 841cd773..42991981 100644 --- a/samples/shared/src/commonMain/kotlin/root/MainViewModel.kt +++ b/samples/shared/src/commonMain/kotlin/root/MainViewModel.kt @@ -1,6 +1,5 @@ package root -import com.tidal.networktime.DnsLookupStrategy import com.tidal.networktime.NTPServer import com.tidal.networktime.SNTPClient import kotlinx.coroutines.GlobalScope