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.
This commit is contained in:
Elcoid committed 2026-09-25 10:52:05 -04:00
1 parent 54f51044e5
commit d1428cd2ae
2 files changed
+100 -8

No files matched your search

+41 -8
View File
@@ -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);