OERV-RUNTIME/boringssl
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Pass IPv6 vs IPv4 down to the shim

Browse Source 
The runner currently tries to listen on IPv6 and then falls back to IPv4
on error. The shim does the same. If they pick different ones, this
breaks down.

Normally, fallback happens because the system doesn't have IPv6, and
both sides will make the same decision. But if binding to IPv6 fails for
other reasons, they may mismatch. We're observing them fail due to what
seems to port exhaustion. When this happens, shim and runner don't
agree on the same address family.

Instead, just tell the shim which address to connect to. This doesn't
fix the underlying port exhaustion problem, but it does seem to fix the
flakes. Although given we are still exhausting ports and falling back to
IPv4, it doesn't truly fix it. Later CLs will address port exhaustion by
using a single server port.

This changes the runner <-> shim protocol, but hopefully in a fairly
obvious way that others using BoGo can easily follow. It shouldn't break
our cross-version tests because we keep runner and shim at the same
versio there.

To avoid needing to make an incompatible change to the shim <->
handshaker protocol, which would impact our cross-version tests, this
introduces a mechanism for the shim omit flags when talking to the
handshaker. The handshaker doesn't need to know how to connect to the
runner.

Also print the error string on Windows. Sadly this is a bit tedious.

Change-Id: Ic8bda9a854a115c206c05a659a2e34f544b844a6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60885
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin
2023-06-19 18:54:09 +00:00
committed by Boringssl LUCI CQ
parent 8ead3f5314
commit e33257fa67
6 changed files with 134 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ssl/test/PORTING.md
+9 -7
View File
@@ -20,14 +20,16 @@ The test runner integrates with the TLS stack under test through a
“shim”: a command line program which encapsulates the stack. By
default, the shim points to the BoringSSL shim in the same source
tree, but any program can be supplied via the `-shim-path` flag. The
runner opens up a server socket and provides the shim with a `-port`
argument that points to that socket. The shim always connects to the
runner as a TCP client even when acting as a TLS server. For DTLS,
there is a small framing layer that gives packet boundaries over
TCP. The shim can also pass a variety of command line arguments which
are used to configure the stack under test. These can be found at
`test_config.cc`.
runner opens up a server socket and provides the shim with `-port` and
optional `-ipv6` arguments.
The shim should connect to loopback as a TCP client on the specified
port, using IPv6 if `-ipv6` is specified and IPv4 otherwise. This is
true even when the shim is speaking DTLS or acting as a TLS server.
For DTLS, there is a small framing layer that gives packet boundaries
over TCP. The shim can also pass a variety of command line arguments
which are used to configure the stack under test. These can be found at
`test_config.cc`.
The shim reports success by exiting with a `0` error code and failure by
reporting a non-zero error code and generally sending a textual error
ssl/test/bssl_shim.cc
+58 -46
View File
@@ -77,7 +77,20 @@ static void PrintSocketError(const char *func) {
}
#else
static void PrintSocketError(const char *func) {
fprintf(stderr, "%s: %d\n", func, WSAGetLastError());
int error = WSAGetLastError();
char *buffer;
DWORD len = FormatMessageA(
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, 0, error, 0,
reinterpret_cast<char *>(&buffer), 0, nullptr);
std::string msg = "unknown error";
if (len > 0) {
msg.assign(buffer, len);
while (!msg.empty() && (msg.back() == '\r' || msg.back() == '\n')) {
msg.resize(msg.size() - 1);
}
}
LocalFree(buffer);
fprintf(stderr, "%s: %s (%d)\n", func, msg.c_str(), error);
}
#endif
@@ -93,56 +106,55 @@ struct Free {
}
};
// Connect returns a new socket connected to localhost on |port| or -1 on
// error.
static int Connect(uint16_t port) {
for (int af : { AF_INET6, AF_INET }) {
int sock = socket(af, SOCK_STREAM, 0);
if (sock == -1) {
PrintSocketError("socket");
// Connect returns a new socket connected to the runner, or -1 on error.
static int Connect(const TestConfig *config) {
sockaddr_storage addr;
socklen_t addr_len = 0;
if (config->ipv6) {
sockaddr_in6 sin6;
OPENSSL_memset(&sin6, 0, sizeof(sin6));
sin6.sin6_family = AF_INET6;
sin6.sin6_port = htons(config->port);
if (!inet_pton(AF_INET6, "::1", &sin6.sin6_addr)) {
PrintSocketError("inet_pton");
return -1;
}
int nodelay = 1;
if (setsockopt(sock, IPPROTO_TCP, TCP_NODELAY,
reinterpret_cast<const char*>(&nodelay), sizeof(nodelay)) != 0) {
PrintSocketError("setsockopt");
closesocket(sock);
addr_len = sizeof(sin6);
memcpy(&addr, &sin6, addr_len);
} else {
sockaddr_in sin;
OPENSSL_memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = htons(config->port);
if (!inet_pton(AF_INET, "127.0.0.1", &sin.sin_addr)) {
PrintSocketError("inet_pton");
return -1;
}
sockaddr_storage ss;
OPENSSL_memset(&ss, 0, sizeof(ss));
ss.ss_family = af;
socklen_t len = 0;
if (af == AF_INET6) {
sockaddr_in6 *sin6 = (sockaddr_in6 *) &ss;
len = sizeof(*sin6);
sin6->sin6_port = htons(port);
if (!inet_pton(AF_INET6, "::1", &sin6->sin6_addr)) {
PrintSocketError("inet_pton");
closesocket(sock);
return -1;
}
} else if (af == AF_INET) {
sockaddr_in *sin = (sockaddr_in *) &ss;
len = sizeof(*sin);
sin->sin_port = htons(port);
if (!inet_pton(AF_INET, "127.0.0.1", &sin->sin_addr)) {
PrintSocketError("inet_pton");
closesocket(sock);
return -1;
}
}
if (connect(sock, reinterpret_cast<const sockaddr*>(&ss), len) == 0) {
return sock;
}
closesocket(sock);
addr_len = sizeof(sin);
memcpy(&addr, &sin, addr_len);
}
PrintSocketError("connect");
return -1;
int sock = socket(addr.ss_family, SOCK_STREAM, 0);
if (sock == -1) {
PrintSocketError("socket");
return -1;
}
int nodelay = 1;
if (setsockopt(sock, IPPROTO_TCP, TCP_NODELAY,
reinterpret_cast<const char *>(&nodelay),
sizeof(nodelay)) != 0) {
PrintSocketError("setsockopt");
closesocket(sock);
return -1;
}
if (connect(sock, reinterpret_cast<const sockaddr *>(&addr), addr_len) != 0) {
PrintSocketError("connect");
closesocket(sock);
return -1;
}
return sock;
}
class SocketCloser {
@@ -775,7 +787,7 @@ static bool DoConnection(bssl::UniquePtr<SSL_SESSION> *out_session,
#endif
}
int sock = Connect(config->port);
int sock = Connect(config);
if (sock == -1) {
return false;
}
ssl/test/handshake_util.cc
+3 -3
View File
@@ -398,9 +398,9 @@ static bool StartHandshaker(ScopedProcess *out, ScopedFD *out_control,
if (is_resume) {
args.push_back(kResumeFlag);
}
// config->argv omits argv[0].
for (int j = 0; j < config->argc; ++j) {
args.push_back(config->argv[j]);
// config->handshaker_args omits argv[0].
for (const char *arg : config->handshaker_args) {
args.push_back(arg);
}
args.push_back(nullptr);
ssl/test/runner/runner.go
+13 -6
View File
@@ -1260,25 +1260,32 @@ type shimProcess struct {
func newShimProcess(shimPath string, flags []string, env []string) (*shimProcess, error) {
shim := new(shimProcess)
var err error
useIPv6 := true
shim.listener, err = net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv6loopback})
if err != nil {
useIPv6 = false
shim.listener, err = net.ListenTCP("tcp4", &net.TCPAddr{IP: net.IP{127, 0, 0, 1}})
}
if err != nil {
return nil, err
}
flags = append([]string{"-port", strconv.Itoa(shim.listener.Addr().(*net.TCPAddr).Port)}, flags...)
cmdFlags := []string{"-port", strconv.Itoa(shim.listener.Addr().(*net.TCPAddr).Port)}
if useIPv6 {
cmdFlags = append(cmdFlags, "-ipv6")
}
cmdFlags = append(cmdFlags, flags...)
if *useValgrind {
shim.cmd = valgrindOf(false, shimPath, flags...)
shim.cmd = valgrindOf(false, shimPath, cmdFlags...)
} else if *useGDB {
shim.cmd = gdbOf(shimPath, flags...)
shim.cmd = gdbOf(shimPath, cmdFlags...)
} else if *useLLDB {
shim.cmd = lldbOf(shimPath, flags...)
shim.cmd = lldbOf(shimPath, cmdFlags...)
} else if *useRR {
shim.cmd = rrOf(shimPath, flags...)
shim.cmd = rrOf(shimPath, cmdFlags...)
} else {
shim.cmd = exec.Command(shimPath, flags...)
shim.cmd = exec.Command(shimPath, cmdFlags...)
}
shim.cmd.Stdin = os.Stdin
shim.cmd.Stdout = &shim.stdout
ssl/test/test_config.cc
+49 -20
View File
@@ -46,12 +46,18 @@ namespace {
struct Flag {
const char *name;
bool has_param;
// skip_handshaker, if true, causes this flag to be skipped when
// forwarding flags to the handshaker. This should be used with flags
// that only impact connecting to the runner.
bool skip_handshaker;
// If |has_param| is false, |param| will be nullptr.
std::function<bool(TestConfig *config, const char *param)> set_param;
};
Flag BoolFlag(const char *name, bool TestConfig::*field) {
return Flag{name, false, [=](TestConfig *config, const char *) -> bool {
Flag BoolFlag(const char *name, bool TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, false, skip_handshaker,
[=](TestConfig *config, const char *) -> bool {
config->*field = true;
return true;
}};
@@ -91,15 +97,19 @@ bool StringToInt(T *out, const char *str) {
}
template <typename T>
Flag IntFlag(const char *name, T TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
Flag IntFlag(const char *name, T TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
return StringToInt(&(config->*field), param);
}};
}
template <typename T>
Flag IntVectorFlag(const char *name, std::vector<T> TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
Flag IntVectorFlag(const char *name, std::vector<T> TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
T value;
if (!StringToInt(&value, param)) {
return false;
@@ -109,8 +119,10 @@ Flag IntVectorFlag(const char *name, std::vector<T> TestConfig::*field) {
}};
}
Flag StringFlag(const char *name, std::string TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
Flag StringFlag(const char *name, std::string TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
config->*field = param;
return true;
}};
@@ -119,8 +131,10 @@ Flag StringFlag(const char *name, std::string TestConfig::*field) {
// TODO(davidben): When we can depend on C++17 or Abseil, switch this to
// std::optional or absl::optional.
Flag OptionalStringFlag(const char *name,
std::unique_ptr<std::string> TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
std::unique_ptr<std::string> TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
(config->*field).reset(new std::string(param));
return true;
}};
@@ -143,15 +157,19 @@ bool DecodeBase64(std::string *out, const std::string &in) {
return true;
}
Flag Base64Flag(const char *name, std::string TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
Flag Base64Flag(const char *name, std::string TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
return DecodeBase64(&(config->*field), param);
}};
}
Flag Base64VectorFlag(const char *name,
std::vector<std::string> TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
std::vector<std::string> TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
std::string value;
if (!DecodeBase64(&value, param)) {
return false;
@@ -163,8 +181,10 @@ Flag Base64VectorFlag(const char *name,
Flag StringPairVectorFlag(
const char *name,
std::vector<std::pair<std::string, std::string>> TestConfig::*field) {
return Flag{name, true, [=](TestConfig *config, const char *param) -> bool {
std::vector<std::pair<std::string, std::string>> TestConfig::*field,
bool skip_handshaker = false) {
return Flag{name, true, skip_handshaker,
[=](TestConfig *config, const char *param) -> bool {
const char *comma = strchr(param, ',');
if (!comma) {
return false;
@@ -178,7 +198,8 @@ Flag StringPairVectorFlag(
std::vector<Flag> SortedFlags() {
std::vector<Flag> flags = {
IntFlag("-port", &TestConfig::port),
IntFlag("-port", &TestConfig::port, /*skip_handshaker=*/true),
BoolFlag("-ipv6", &TestConfig::ipv6, /*skip_handshaker=*/true),
BoolFlag("-server", &TestConfig::is_server),
BoolFlag("-dtls", &TestConfig::is_dtls),
BoolFlag("-quic", &TestConfig::is_quic),
@@ -428,11 +449,10 @@ bool ParseConfig(int argc, char **argv, bool is_shim,
TestConfig *out_initial,
TestConfig *out_resume,
TestConfig *out_retry) {
out_initial->argc = out_resume->argc = out_retry->argc = argc;
out_initial->argv = out_resume->argv = out_retry->argv = argv;
for (int i = 0; i < argc; i++) {
bool skip = false;
const char *name = argv[i];
const char *arg = argv[i];
const char *name = arg;
// -on-shim and -on-handshaker prefixes enable flags only on the shim or
// handshaker.
@@ -473,6 +493,13 @@ bool ParseConfig(int argc, char **argv, bool is_shim,
param = argv[i];
}
if (!flag->skip_handshaker) {
out_initial->handshaker_args.push_back(arg);
if (flag->has_param) {
out_initial->handshaker_args.push_back(param);
}
}
if (!skip) {
if (out != nullptr) {
if (!flag->set_param(out, param)) {
@@ -491,6 +518,8 @@ bool ParseConfig(int argc, char **argv, bool is_shim,
}
}
out_resume->handshaker_args = out_initial->handshaker_args;
out_retry->handshaker_args = out_initial->handshaker_args;
return true;
}
ssl/test/test_config.h
+2 -2
View File
@@ -26,6 +26,7 @@
struct TestConfig {
int port = 0;
bool ipv6 = false;
bool is_server = false;
bool is_dtls = false;
bool is_quic = false;
@@ -197,8 +198,7 @@ struct TestConfig {
bool fips_202205 = false;
bool wpa_202304 = false;
int argc;
char **argv;
std::vector<const char*> handshaker_args;
bssl::UniquePtr<SSL_CTX> SetupCtx(SSL_CTX *old_ctx) const;