From d1428cd2aec6bc176895a27e30533c54a6a88244 Mon Sep 17 00:00:00 2001 From: Elcoid <47006046+Elcoid@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:52:05 -0400 Subject: [PATCH 1/2] sam: fix inconsistencies in version negotiation of sam handshake This commit fixes [issue #2570](https://github.com/PurpleI2P/i2pd/issues/2570). The [specification of the SAM protocol](https://i2p.net/en/docs/api/samv3/) says the following: - "As of version 3.1 (I2P 0.9.14), the MIN and MAX parameters are optional." - "SAM will always return the highest version possible given the MIN and MAX constraints" Given this, the following handshakes do not change: ``` printf "HELLO VERSION MIN=3.1 MAX=3.3\n" | nc -q 0 127.0.0.1 7656 HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MAX=3.3\n" | nc -q 0 127.0.0.1 7656 HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=31 MAX=31\n" | nc -q 0 127.0.0.1 7656 HELLO REPLY RESULT=OK VERSION=3.1 (periods are still optional) ``` But the following ones do (the responses before and after this commit are included): ``` printf "HELLO VERSION MAX=3.4\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=NOVERSION after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=3.0\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=OK VERSION=3.0 after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=3.1\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=OK VERSION=3.1 after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=2.9\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=NOVERSION after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=NOVERSION after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=3.3 MAX=3.1\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=OK VERSION=3.1 after: HELLO REPLY RESULT=NOVERSION printf "HELLO VERSION MIN=3.5 MAX=2.7\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=OK VERSION=2.7 after: HELLO REPLY RESULT=NOVERSION printf "HELLO VERSION MIN=2.7 MAX=3.5\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=NOVERSION after: HELLO REPLY RESULT=OK VERSION=3.3 printf "HELLO VERSION MIN=aab3vsg1df MAX=fd3gfbgf1bf\n" | nc -q 0 127.0.0.1 7656 before: HELLO REPLY RESULT=OK VERSION=3.1 after: HELLO REPLY RESULT=NOVERSION ``` I included a script to automatically test these handshakes. --- libi2pd_client/SAM.cpp | 49 ++++++++++++++++++++++++++------ tests/test-sam-version.sh | 59 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 tests/test-sam-version.sh diff --git a/libi2pd_client/SAM.cpp b/libi2pd_client/SAM.cpp index cdc662a3..63afce69 100644 --- a/libi2pd_client/SAM.cpp +++ b/libi2pd_client/SAM.cpp @@ -95,6 +95,15 @@ namespace client version *= 10; version += (ch - '0'); } + else if (ch == '.') + { + // no-op: skip periods + } + else + { + // if version contains other characters, return error + return -1; + } } return version; } @@ -131,7 +140,10 @@ namespace client if (!strcmp (m_Buffer, SAM_HANDSHAKE)) { int minVer = 0, maxVer = 0; - // try to find MIN and MAX, 3.0 if not found + bool verErr = 0; + // try to find MIN and MAX, MAX_SAM_VERSION if not found, + // since the highest possible version must be returned + // given the constraints if (separator) { separator++; @@ -143,14 +155,35 @@ namespace client if (!minVerStr.empty ()) minVer = ExtractVersion (minVerStr); } + // if parsing error or impossible version constraints + if (minVer == -1 || maxVer == -1 || (minVer && maxVer && minVer > maxVer)) + verErr = 1; // version negotiation - if (maxVer && maxVer <= MAX_SAM_VERSION) - m_Version = maxVer; - else if (minVer && minVer >= MIN_SAM_VERSION && minVer <= MAX_SAM_VERSION) - m_Version = minVer; - else if (!maxVer && !minVer) - m_Version = MIN_SAM_VERSION; - else + else if (maxVer && minVer) // if both constraints provided + { + if (maxVer < MIN_SAM_VERSION || minVer > MAX_SAM_VERSION) + verErr = 1; + else + m_Version = std::min(maxVer, MAX_SAM_VERSION); + } + else if (maxVer) // if only max provided + { + if (maxVer < MIN_SAM_VERSION) + verErr = 1; + else + m_Version = std::min(maxVer, MAX_SAM_VERSION); + } + else if (minVer) // if only min provided + { + if (minVer > MAX_SAM_VERSION) + verErr = 1; + else + m_Version = MAX_SAM_VERSION; + } + else // if neither min nor max is provided + m_Version = MAX_SAM_VERSION; + + if (verErr) { LogPrint (eLogError, "SAM: Handshake version mismatch ", minVer, " ", maxVer); SendMessageReply (SAM_HANDSHAKE_NOVERSION, true); diff --git a/tests/test-sam-version.sh b/tests/test-sam-version.sh new file mode 100644 index 00000000..c1f40a10 --- /dev/null +++ b/tests/test-sam-version.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash + +# This script tests the version negotiation in the SAM handshake. + +# Inputs and expected outputs +IN=() +EXP=() + +IN+=("MIN=3.1 MAX=3.3") +EXP+=("OK VERSION=3.3") + +IN+=("MAX=3.3") +EXP+=("OK VERSION=3.3") + +IN+=("MAX=3.4") +EXP+=("OK VERSION=3.3") + +IN+=("MIN=3.0") +EXP+=("OK VERSION=3.3") + +IN+=("MIN=3.1") +EXP+=("OK VERSION=3.3") + +IN+=("MIN=2.9") +EXP+=("OK VERSION=3.3") + +IN+=("") +EXP+=("OK VERSION=3.3") + +IN+=("MIN=3.3 MAX=3.1") +EXP+=("NOVERSION") + +IN+=("MIN=3.5 MAX=2.7") +EXP+=("NOVERSION") + +IN+=("MIN=2.7 MAX=3.5") +EXP+=("OK VERSION=3.3") + +IN+=("MIN=afddab3vsfdsg1df MAX=dsaaffdb3ggfgfbgf1bssbf") +EXP+=("NOVERSION") + +IN+=("MIN=31 MAX=31") +EXP+=("OK VERSION=3.1") + + +for i in $(seq 0 $((${#IN[@]} - 1))); do + printf "HELLO VERSION ${IN[$i]} - " + + # Observed output + OBS=$(printf "HELLO VERSION ${IN[$i]}\n" | nc -q 0 127.0.0.1 7656) + + if [ "$OBS" = "HELLO REPLY RESULT=${EXP[$i]}" ]; then + printf "OK\n" + else + printf "received $OBS\n" + fi +done + + From 61e18baf68d19649cd6506ee487b07d4712d4f34 Mon Sep 17 00:00:00 2001 From: Elcoid <47006046+Elcoid@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:30:18 -0400 Subject: [PATCH 2/2] sam: replace C-style 0/1 with C++-style false/true --- libi2pd_client/SAM.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libi2pd_client/SAM.cpp b/libi2pd_client/SAM.cpp index 63afce69..50cb8bf5 100644 --- a/libi2pd_client/SAM.cpp +++ b/libi2pd_client/SAM.cpp @@ -140,7 +140,7 @@ namespace client if (!strcmp (m_Buffer, SAM_HANDSHAKE)) { int minVer = 0, maxVer = 0; - bool verErr = 0; + bool verErr = false; // try to find MIN and MAX, MAX_SAM_VERSION if not found, // since the highest possible version must be returned // given the constraints @@ -157,26 +157,26 @@ namespace client } // if parsing error or impossible version constraints if (minVer == -1 || maxVer == -1 || (minVer && maxVer && minVer > maxVer)) - verErr = 1; + verErr = true; // version negotiation else if (maxVer && minVer) // if both constraints provided { if (maxVer < MIN_SAM_VERSION || minVer > MAX_SAM_VERSION) - verErr = 1; + verErr = true; else m_Version = std::min(maxVer, MAX_SAM_VERSION); } else if (maxVer) // if only max provided { if (maxVer < MIN_SAM_VERSION) - verErr = 1; + verErr = true; else m_Version = std::min(maxVer, MAX_SAM_VERSION); } else if (minVer) // if only min provided { if (minVer > MAX_SAM_VERSION) - verErr = 1; + verErr = true; else m_Version = MAX_SAM_VERSION; }