fix(url): parse ?srp-compat= as 0|1, not "legacy"
The original librist convention for URL parameters that select from a
small fixed enum is numeric values (aes-type, congestion-control,
timing-mode, reflector, and as of the previous commit profile).
?srp-compat= was added in v0.2.18-rc1 with a symbolic alias ("legacy")
and a permissive parser that mapped any unrecognised value, including
a typo of the alias, to 0 (compliant). Tightened to numeric-only
before v0.2.18 final.
The parser now accepts only 0 or 1. Out-of-range, non-numeric, and
trailing-garbage values cause rist_parse_address2 to return non-zero
with stderr output.
Tests
-----
- test/rist/unit/test_url_srp_compat_parse.c (new): two accepted forms
plus six rejected forms.
- meson test --suite unit: 6/6 OK (was 5/5).
- meson test --suite url-profile: 4/4 OK.
- meson test (full): 49 OK, 11 Expected Fail, 0 unexpected.
This commit is contained in:
@@ -55,7 +55,7 @@ SRP interoperability:
|
||||
the PAD) will not handshake with 0.2.16+. Upgrading both ends
|
||||
to 0.2.16+ remains the recommended fix.
|
||||
- New URL parameter for transitional interop:
|
||||
?srp-compat=legacy
|
||||
?srp-compat=1
|
||||
On both ends of the SRP exchange, this opts the SRP wire format
|
||||
back into the pre-0.2.16 unpadded form. Must be set on BOTH
|
||||
sides; mixed configuration will still fail. An explicit WARN
|
||||
@@ -63,7 +63,7 @@ SRP interoperability:
|
||||
as an escape hatch while older deployments are upgraded.
|
||||
- When an SRP handshake fails, the authenticator (and the client
|
||||
on M2 mismatch) now emits a single INFO hint pointing at the
|
||||
srp-compat=legacy escape hatch. The hint is advisory only —
|
||||
srp-compat=1 escape hatch. The hint is advisory only —
|
||||
the SRP-6a transcript binds M1 to the authenticator's wire-mode
|
||||
B, so there is no purely-local way to distinguish "wrong
|
||||
password" from "peer is on the other wire format" once the
|
||||
|
||||
@@ -28,7 +28,7 @@
|
||||
#define RIST_URL_PARAM_KEEPALIVE_INT "keepalive-interval"
|
||||
#define RIST_URL_PARAM_SRP_USERNAME "username"
|
||||
#define RIST_URL_PARAM_SRP_PASSWORD "password"
|
||||
#define RIST_URL_PARAM_SRP_COMPAT "srp-compat" //"legacy" or "1" => pre-0.2.16 SRP wire format
|
||||
#define RIST_URL_PARAM_SRP_COMPAT "srp-compat" //0|1 (1 => pre-0.2.16 SRP wire format)
|
||||
/* Less common URL parameters */
|
||||
#define RIST_URL_PARAM_BUFFER_SIZE_MIN "buffer-min"
|
||||
#define RIST_URL_PARAM_BUFFER_SIZE_MAX "buffer-max"
|
||||
|
||||
+2
-2
@@ -432,7 +432,7 @@ struct librist_crypto_srp_authenticator_ctx {
|
||||
uint8_t m2[SHA256_DIGEST_LENGTH];
|
||||
|
||||
bool correct_hashing_init;
|
||||
bool legacy_pad; //pre-0.2.16 unpadded u/k (srp-compat=legacy)
|
||||
bool legacy_pad; //pre-0.2.16 unpadded u/k (srp-compat=1)
|
||||
};
|
||||
|
||||
struct librist_crypto_srp_authenticator_ctx *librist_crypto_srp_authenticator_ctx_create(const char* n_hex, const char *g_hex, const uint8_t *v_bytes, size_t v_len, const uint8_t *s_bytes, size_t s_len, bool correct, bool legacy_pad) {
|
||||
@@ -739,7 +739,7 @@ struct librist_crypto_srp_client_ctx {
|
||||
uint8_t m1[SHA256_DIGEST_LENGTH];
|
||||
|
||||
bool correct_hashing_init;
|
||||
bool legacy_pad; //pre-0.2.16 unpadded u/k (srp-compat=legacy)
|
||||
bool legacy_pad; //pre-0.2.16 unpadded u/k (srp-compat=1)
|
||||
};
|
||||
|
||||
int librist_crypto_srp_client_write_A_bytes(struct librist_crypto_srp_client_ctx *ctx, uint8_t *A_buf, size_t A_buf_len) {
|
||||
|
||||
+7
-7
@@ -111,7 +111,7 @@ struct eapsrp_ctx
|
||||
|
||||
bool eapversion3;//EAPv3 signalled. old libRIST used v2, so use this to ensure compat with broken hashing
|
||||
|
||||
bool srp_legacy_pad; //srp-compat=legacy URL opt-in
|
||||
bool srp_legacy_pad; //srp-compat=1 URL opt-in
|
||||
bool srp_legacy_peer_warned; //one-shot latch for the M1/M2 hint
|
||||
};
|
||||
|
||||
@@ -349,10 +349,10 @@ static int process_eap_request_srp_server_validator(struct eapsrp_ctx *ctx, uint
|
||||
ctx->srp_legacy_peer_warned = true;
|
||||
if (ctx->srp_legacy_pad) {
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_INFO,
|
||||
EAP_LOG_PREFIX" Hint: this side is configured for srp-compat=legacy. If the server is running librist 0.2.16+ (PAD-compliant, the default), drop ?srp-compat=legacy on both sides.\n");
|
||||
EAP_LOG_PREFIX" Hint: this side is configured with srp-compat=1. If the server is running librist 0.2.16+ (PAD-compliant, the default), drop ?srp-compat=1 on both sides.\n");
|
||||
} else {
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_INFO,
|
||||
EAP_LOG_PREFIX" Hint: if the server is running librist 0.2.15 or earlier, the SRP wire format changed in 0.2.16 for RFC 5054 / TR-06-2 compliance. To interoperate with an older server, add ?srp-compat=legacy on BOTH URLs. Otherwise check the password.\n");
|
||||
EAP_LOG_PREFIX" Hint: if the server is running librist 0.2.15 or earlier, the SRP wire format changed in 0.2.16 for RFC 5054 / TR-06-2 compliance. To interoperate with an older server, add ?srp-compat=1 on BOTH URLs. Otherwise check the password.\n");
|
||||
}
|
||||
}
|
||||
ctx->authentication_state = EAP_AUTH_STATE_FAILED;
|
||||
@@ -573,11 +573,11 @@ static int process_eap_response_client_validator(struct eapsrp_ctx *ctx, size_t
|
||||
ctx->srp_legacy_peer_warned = true;
|
||||
if (ctx->srp_legacy_pad) {
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_INFO,
|
||||
EAP_LOG_PREFIX" Hint: this side is configured for srp-compat=legacy. If %s is running librist 0.2.16+ (PAD-compliant, the default), drop ?srp-compat=legacy on both sides.\n",
|
||||
EAP_LOG_PREFIX" Hint: this side is configured with srp-compat=1. If %s is running librist 0.2.16+ (PAD-compliant, the default), drop ?srp-compat=1 on both sides.\n",
|
||||
ctx->ip_string);
|
||||
} else {
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_INFO,
|
||||
EAP_LOG_PREFIX" Hint: if %s is running librist 0.2.15 or earlier, the SRP wire format changed in 0.2.16 for RFC 5054 / TR-06-2 compliance. To interoperate with an older peer, add ?srp-compat=legacy on BOTH URLs. Otherwise check the password.\n",
|
||||
EAP_LOG_PREFIX" Hint: if %s is running librist 0.2.15 or earlier, the SRP wire format changed in 0.2.16 for RFC 5054 / TR-06-2 compliance. To interoperate with an older peer, add ?srp-compat=1 on BOTH URLs. Otherwise check the password.\n",
|
||||
ctx->ip_string);
|
||||
}
|
||||
}
|
||||
@@ -1150,7 +1150,7 @@ int rist_enable_eap_srp_2(struct rist_peer *peer, const char *username, const ch
|
||||
ctx->srp_legacy_pad = (peer->config.srp_compat_legacy != 0);
|
||||
if (ctx->srp_legacy_pad)
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_WARN,
|
||||
EAP_LOG_PREFIX"SRP legacy compat mode ACTIVE on this authenticator (srp-compat=legacy). Wire format is the pre-0.2.16 unpadded form — NOT TR-06-2 / RFC 5054 compliant. For transitional interop only.\n");
|
||||
EAP_LOG_PREFIX"SRP legacy compat mode ACTIVE on this authenticator (srp-compat=1). Wire format is the pre-0.2.16 unpadded form — NOT TR-06-2 / RFC 5054 compliant. For transitional interop only.\n");
|
||||
peer->eap_ctx = ctx;
|
||||
struct rist_peer *child = peer->child;
|
||||
peer->eap_authentication_state = 1;
|
||||
@@ -1185,7 +1185,7 @@ int rist_enable_eap_srp_2(struct rist_peer *peer, const char *username, const ch
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_INFO, EAP_LOG_PREFIX"EAP Authentication enabled, role = authenticatee\n");
|
||||
if (ctx->srp_legacy_pad)
|
||||
rist_log_priv2(ctx->config.logging_settings, RIST_LOG_WARN,
|
||||
EAP_LOG_PREFIX"SRP legacy compat mode ACTIVE on this client (srp-compat=legacy). Wire format is the pre-0.2.16 unpadded form — NOT TR-06-2 / RFC 5054 compliant. For transitional interop only.\n");
|
||||
EAP_LOG_PREFIX"SRP legacy compat mode ACTIVE on this client (srp-compat=1). Wire format is the pre-0.2.16 unpadded form — NOT TR-06-2 / RFC 5054 compliant. For transitional interop only.\n");
|
||||
ctx->eapversion3 = true;
|
||||
if (!peer->multicast_receiver)
|
||||
_librist_proto_eap_start(ctx);
|
||||
|
||||
+8
-2
@@ -192,8 +192,14 @@ int parse_url_options(const char* url, struct rist_peer_config *output_peer_conf
|
||||
} else if (strcmp( url_params[i].key, RIST_URL_PARAM_SRP_PASSWORD) == 0) {
|
||||
strncpy((void *)output_peer_config->srp_password, val, 256 -1);
|
||||
} else if (strcmp( url_params[i].key, RIST_URL_PARAM_SRP_COMPAT) == 0) {
|
||||
output_peer_config->srp_compat_legacy =
|
||||
(strcmp(val, "legacy") == 0 || strcmp(val, "1") == 0) ? 1 : 0;
|
||||
char *endp = NULL;
|
||||
long temp = strtol(val, &endp, 10);
|
||||
if (endp == val || *endp != '\0' || (temp != 0 && temp != 1)) {
|
||||
ret = -1;
|
||||
fprintf(stderr, "Invalid srp-compat '%s'; expected 0|1\n", val);
|
||||
continue;
|
||||
}
|
||||
output_peer_config->srp_compat_legacy = (int)temp;
|
||||
} else if (strcmp( url_params[i].key, RIST_URL_PARAM_CNAME ) == 0) {
|
||||
strncpy((void *)output_peer_config->cname, val, 128-1);
|
||||
} else if (strcmp( url_params[i].key, RIST_URL_PARAM_AES_TYPE ) == 0) {
|
||||
|
||||
@@ -55,4 +55,11 @@ test_url_profile_override = executable('test_url_profile_override',
|
||||
'test_url_profile_override.c',
|
||||
include_directories : inc,
|
||||
link_with : librist)
|
||||
test('url_profile_override', test_url_profile_override, suite:['unit'])
|
||||
test('url_profile_override', test_url_profile_override, suite:['unit'])
|
||||
|
||||
# ?srp-compat= URL parsing exercised through the public rist_parse_address2 API.
|
||||
test_url_srp_compat_parse = executable('test_url_srp_compat_parse',
|
||||
'test_url_srp_compat_parse.c',
|
||||
include_directories : inc,
|
||||
link_with : librist)
|
||||
test('url_srp_compat_parse', test_url_srp_compat_parse, suite:['unit'])
|
||||
@@ -437,7 +437,7 @@ static void test_srp_correct_hashing_client_verify_M2(void **state) {
|
||||
|
||||
/* Fresh authenticator+client pair using only the public SRP API. These
|
||||
* tests do not depend on the deterministic fixture above (a/b are random
|
||||
* each run) and exercise the srp-compat=legacy plumbing end-to-end.
|
||||
* each run) and exercise the srp-compat=1 plumbing end-to-end.
|
||||
*
|
||||
* Four exchanges on the 2048-bit NG_DEFAULT group:
|
||||
* 1. Both PAD (default) → handshake succeeds (verify_m1 == 0)
|
||||
@@ -445,7 +445,7 @@ static void test_srp_correct_hashing_client_verify_M2(void **state) {
|
||||
* 3. Authenticator PAD, client LEGACY → handshake fails (verify_m1 != 0)
|
||||
* 4. Authenticator LEGACY, client PAD → handshake fails (verify_m1 != 0)
|
||||
*
|
||||
* Cases 3 and 4 are the "operator forgot to set srp-compat on one side"
|
||||
* Cases 3 and 4 are the "operator forgot to set srp-compat=1 on one side"
|
||||
* scenarios. They MUST fail at M1 (otherwise the legacy-bypass would be
|
||||
* leaking through), but we do not attempt to detect the cross-mode case
|
||||
* cryptographically — that is impossible without changing the wire
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
/* librist. SPDX-License-Identifier: BSD-2-Clause
|
||||
*
|
||||
* Intentionally avoids cmocka so it runs anywhere the public library
|
||||
* does. */
|
||||
|
||||
#include "librist/librist.h"
|
||||
#include "librist/peer.h"
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
static int expect_legacy(const char *url, int want)
|
||||
{
|
||||
struct rist_peer_config *cfg = NULL;
|
||||
int ret = rist_parse_address2(url, &cfg);
|
||||
if (ret != 0 || cfg == NULL) {
|
||||
fprintf(stderr,
|
||||
"FAIL: rist_parse_address2(%s) ret=%d cfg=%p\n",
|
||||
url, ret, (void *)cfg);
|
||||
if (cfg)
|
||||
rist_peer_config_free2(&cfg);
|
||||
return 1;
|
||||
}
|
||||
if (cfg->srp_compat_legacy != want) {
|
||||
fprintf(stderr,
|
||||
"FAIL: %s -> srp_compat_legacy=%d (want %d)\n",
|
||||
url, cfg->srp_compat_legacy, want);
|
||||
rist_peer_config_free2(&cfg);
|
||||
return 1;
|
||||
}
|
||||
rist_peer_config_free2(&cfg);
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int expect_parse_error(const char *url)
|
||||
{
|
||||
struct rist_peer_config *cfg = NULL;
|
||||
int ret = rist_parse_address2(url, &cfg);
|
||||
if (ret == 0) {
|
||||
fprintf(stderr,
|
||||
"FAIL: %s expected parse error, got success\n", url);
|
||||
if (cfg)
|
||||
rist_peer_config_free2(&cfg);
|
||||
return 1;
|
||||
}
|
||||
if (cfg)
|
||||
rist_peer_config_free2(&cfg);
|
||||
return 0;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int failures = 0;
|
||||
|
||||
failures += expect_legacy("rist://@127.0.0.1:1234", 0);
|
||||
failures += expect_legacy("rist://@127.0.0.1:1234?srp-compat=0", 0);
|
||||
failures += expect_legacy("rist://@127.0.0.1:1234?srp-compat=1", 1);
|
||||
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=legacy");
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=auto");
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=2");
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=-1");
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=1foo");
|
||||
failures += expect_parse_error("rist://@127.0.0.1:1234?srp-compat=garbage");
|
||||
|
||||
if (failures == 0) {
|
||||
printf("OK\n");
|
||||
return 0;
|
||||
}
|
||||
fprintf(stderr, "Total failures: %d\n", failures);
|
||||
return 1;
|
||||
}
|
||||
Reference in New Issue
Block a user