Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/iocore/net/SSLSNIConfig.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ NamedElement::operator=(NamedElement &&other)
if (this != &other) {
match = std::move(other.match);
inbound_port_ranges = std::move(other.inbound_port_ranges);
rank = other.rank;
Copy link
Copy Markdown
Contributor Author

@bryancall bryancall May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug fix. ⬆️

The rest of the changes are more tests and an updated config.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense

}
return *this;
}
Expand Down
13 changes: 13 additions & 0 deletions src/iocore/net/unit_tests/sni_conf_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,16 @@ sni:
http2_buffer_water_mark: 256
- fqdn: foo.bar.com
http2: false

# test with mixed-case
- fqdn: "MiXeDcAsE.foo.com"
http2: true
http2_buffer_water_mark: 256
inbound_port_ranges: 31337

# test with mixed-case glob
- fqdn: "*.MiXeDcAsE.com"
http2: false

# test glob in the middle, this will be an exact match
- fqdn: "cat.*.com"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion around the #9736, we concluded don't allow this. This should be error.

  1. Allow single left-most * in the fqdn field

https://docs.trafficserver.apache.org/en/latest/admin-guide/files/sni.yaml.en.html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

80 changes: 68 additions & 12 deletions src/iocore/net/unit_tests/test_SSLSNIConfig.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,78 +41,134 @@ TEST_CASE("Test SSLSNIConfig")

SECTION("The config does not match any SNIs for someport.com:577")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 577)};
auto const &actions{params.get("someport.com", 577)};
CHECK(!actions.first);
}

SECTION("The config does not match any SNIs for someport.com:808")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 808)};
auto const &actions{params.get("someport.com", 808)};
CHECK(!actions.first);
}

SECTION("The config does not match any SNIs for oneport.com:1")
{
auto const &actions{params.get({"oneport.com", std::strlen("oneport.com")}, 1)};
auto const &actions{params.get("oneport.com", 1)};
CHECK(!actions.first);
}

SECTION("The config does match an SNI for oneport.com:433")
{
auto const &actions{params.get({"oneport.com", std::strlen("oneport.com")}, 433)};
auto const &actions{params.get("oneport.com", 433)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}

SECTION("The config matches an SNI for allports.com")
{
auto const &actions{params.get({"allports.com", std::strlen("allports.com")}, 1)};
auto const &actions{params.get("allports.com", 1)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}

SECTION("The config matches an SNI for someport.com:1")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 1)};
auto const &actions{params.get("someport.com", 1)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 3);
}

SECTION("The config matches an SNI for someport.com:433")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 433)};
auto const &actions{params.get("someport.com", 433)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 3);
}

SECTION("The config matches an SNI for someport:8080")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 8080)};
auto const &actions{params.get("someport.com", 8080)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}

SECTION("The config matches an SNI for someport:65535")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 65535)};
auto const &actions{params.get("someport.com", 65535)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}

SECTION("The config matches an SNI for someport:482")
{
auto const &actions{params.get({"someport.com", std::strlen("someport.com")}, 482)};
auto const &actions{params.get("someport.com", 482)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 3);
}

SECTION("Matching order")
{
std::string_view target = "foo.bar.com";
auto const &actions{params.get(target, 443)};
auto const &actions{params.get("foo.bar.com", 443)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 5); ///< three H2 config + early data + fqdn
}

SECTION("Test mixed-case")
{
auto const &actions{params.get("SoMePoRt.CoM", 65535)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}

SECTION("Test mixed-case with wildcard in yaml config")
{
auto const &actions{params.get("AnYtHiNg.BaR.CoM", 443)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 4);
// verify the capture group
REQUIRE(actions.second._fqdn_wildcard_captured_groups->at(0) == "AnYtHiNg");
}

SECTION("Test mixed-case in yaml config")
{
auto const &actions{params.get("mixedcase.foo.com", 31337)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 4);
}

SECTION("Test mixed-case glob in yaml config")
{
auto const &actions{params.get("FoO.mixedcase.com", 443)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 3);
// verify the capture group
REQUIRE(actions.second._fqdn_wildcard_captured_groups->at(0) == "FoO");
}

SECTION("Test empty SNI does not match")
{
auto const &actions{params.get("", 443)};
CHECK(!actions.first);
}

SECTION("Test SNI with special characters does not match")
{
auto const &actions{params.get("some$port.com", 443)};
CHECK(!actions.first);
}

SECTION("Test with invalid glob in the middle in yaml config (e.g. cat.*.com) does not match")
{
auto const &actions{params.get("cat.dog.com", 443)};
REQUIRE(!actions.first);
}

SECTION("Test with invalid glob in the middle in yaml config (e.g. cat.*.com) does an exact match")
{
auto const &actions{params.get("cat.*.com", 443)};
REQUIRE(actions.first);
REQUIRE(actions.first->size() == 2);
}
}

TEST_CASE("SNIConfig reconfigure callback is invoked")
Expand Down
2 changes: 1 addition & 1 deletion src/iocore/net/unit_tests/test_YamlSNIConfig.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ TEST_CASE("YamlSNIConfig sets port ranges appropriately")
FAIL(errorstream.str());
}
REQUIRE(zret.is_ok());
REQUIRE(conf.items.size() == 7);
REQUIRE(conf.items.size() == 10);

SECTION("If no ports were specified, port range should contain all ports.")
{
Expand Down