Defer writing the shim settings.
This is prefactoring for a coming change to the shim that will write
handoff and handback messages (which are serialized SSLConnection
objects) to the transcript.
This breaks the slightly tenuous ordering between the runner and the
shim. Fix the runner to wait until the shim has exited before
appending the transcript.
Change-Id: Iae34d28ec1addfe3ec4f3c77008248fe5530687c
Reviewed-on: https://boringssl-review.googlesource.com/27184
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/fuzz/refresh_ssl_corpora.sh b/fuzz/refresh_ssl_corpora.sh
index 6db5562..d49f574 100755
--- a/fuzz/refresh_ssl_corpora.sh
+++ b/fuzz/refresh_ssl_corpora.sh
@@ -70,8 +70,8 @@
no_fuzzer_mode_shim=$(readlink -f \
"$no_fuzzer_mode_build_dir/ssl/test/bssl_shim")
-fuzzer_mode_transcripts=$(mktemp -d '/tmp/boringssl-transcript.XXXXXX')
-no_fuzzer_mode_transcripts=$(mktemp -d '/tmp/boringssl-transcript.XXXXXX')
+fuzzer_mode_transcripts=$(mktemp -d '/tmp/boringssl-transcript-fuzzer-mode.XXXXXX')
+no_fuzzer_mode_transcripts=$(mktemp -d '/tmp/boringssl-transcript-no-fuzzer-mode.XXXXXX')
echo Recording fuzzer-mode transcripts
(cd ../ssl/test/runner/ && go test \
@@ -84,7 +84,7 @@
(cd ../ssl/test/runner/ && go test \
-shim-path "$no_fuzzer_mode_shim" \
-transcript-dir "$no_fuzzer_mode_transcripts" \
- -deterministic) || true
+ -deterministic)
# Minimize the existing corpora.
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 824dc94..f1531bd 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1849,66 +1849,85 @@
return true;
}
-static bool WriteSettings(int i, const TestConfig *config,
- const SSL_SESSION *session) {
- if (config->write_settings.empty()) {
+// SettingsWriter writes fuzzing inputs to a file if |write_settings| is set.
+struct SettingsWriter {
+ public:
+ SettingsWriter() {}
+
+ // Init initializes the writer for a new connection, given by |i|. Each
+ // connection gets a unique output file.
+ bool Init(int i, const TestConfig *config, SSL_SESSION *session) {
+ if (config->write_settings.empty()) {
+ return true;
+ }
+ // Treat write_settings as a path prefix for each connection in the run.
+ char buf[DECIMAL_SIZE(int)];
+ snprintf(buf, sizeof(buf), "%d", i);
+ path_ = config->write_settings + buf;
+
+ if (!CBB_init(cbb_.get(), 64)) {
+ return false;
+ }
+
+ if (session != nullptr) {
+ uint8_t *data;
+ size_t len;
+ if (!SSL_SESSION_to_bytes(session, &data, &len)) {
+ return false;
+ }
+ bssl::UniquePtr<uint8_t> free_data(data);
+ CBB child;
+ if (!CBB_add_u16(cbb_.get(), kSessionTag) ||
+ !CBB_add_u24_length_prefixed(cbb_.get(), &child) ||
+ !CBB_add_bytes(&child, data, len) ||
+ !CBB_flush(cbb_.get())) {
+ return false;
+ }
+ }
+
+ if (config->is_server &&
+ (config->require_any_client_certificate || config->verify_peer) &&
+ !CBB_add_u16(cbb_.get(), kRequestClientCert)) {
+ return false;
+ }
+
+ if (config->tls13_variant != 0 &&
+ (!CBB_add_u16(cbb_.get(), kTLS13Variant) ||
+ !CBB_add_u8(cbb_.get(),
+ static_cast<uint8_t>(config->tls13_variant)))) {
+ return false;
+ }
+
return true;
}
- // Treat write_settings as a path prefix for each connection in the run.
- char buf[DECIMAL_SIZE(int)];
- snprintf(buf, sizeof(buf), "%d", i);
- std::string path = config->write_settings + buf;
+ // Commit writes the buffered data to disk.
+ bool Commit() {
+ if (path_.empty()) {
+ return true;
+ }
- bssl::ScopedCBB cbb;
- if (!CBB_init(cbb.get(), 64)) {
- return false;
- }
-
- if (session != nullptr) {
- uint8_t *data;
- size_t len;
- if (!SSL_SESSION_to_bytes(session, &data, &len)) {
+ uint8_t *settings;
+ size_t settings_len;
+ if (!CBB_add_u16(cbb_.get(), kDataTag) ||
+ !CBB_finish(cbb_.get(), &settings, &settings_len)) {
return false;
}
- bssl::UniquePtr<uint8_t> free_data(data);
- CBB child;
- if (!CBB_add_u16(cbb.get(), kSessionTag) ||
- !CBB_add_u24_length_prefixed(cbb.get(), &child) ||
- !CBB_add_bytes(&child, data, len) ||
- !CBB_flush(cbb.get())) {
+ bssl::UniquePtr<uint8_t> free_settings(settings);
+
+ using ScopedFILE = std::unique_ptr<FILE, decltype(&fclose)>;
+ ScopedFILE file(fopen(path_.c_str(), "w"), fclose);
+ if (!file) {
return false;
}
+
+ return fwrite(settings, settings_len, 1, file.get()) == 1;
}
- if (config->is_server &&
- (config->require_any_client_certificate || config->verify_peer) &&
- !CBB_add_u16(cbb.get(), kRequestClientCert)) {
- return false;
- }
-
- if (config->tls13_variant != 0 &&
- (!CBB_add_u16(cbb.get(), kTLS13Variant) ||
- !CBB_add_u8(cbb.get(), static_cast<uint8_t>(config->tls13_variant)))) {
- return false;
- }
-
- uint8_t *settings;
- size_t settings_len;
- if (!CBB_add_u16(cbb.get(), kDataTag) ||
- !CBB_finish(cbb.get(), &settings, &settings_len)) {
- return false;
- }
- bssl::UniquePtr<uint8_t> free_settings(settings);
-
- using ScopedFILE = std::unique_ptr<FILE, decltype(&fclose)>;
- ScopedFILE file(fopen(path.c_str(), "w"), fclose);
- if (!file) {
- return false;
- }
-
- return fwrite(settings, settings_len, 1, file.get()) == 1;
-}
+ private:
+ std::string path_;
+ bssl::ScopedCBB cbb_;
+};
static bssl::UniquePtr<SSL> NewSSL(SSL_CTX *ssl_ctx, const TestConfig *config,
SSL_SESSION *session, bool is_resume,
@@ -2708,12 +2727,18 @@
}
bssl::UniquePtr<SSL_SESSION> offer_session = std::move(session);
- if (!WriteSettings(i, config, offer_session.get())) {
+ SettingsWriter writer;
+ if (!writer.Init(i, config, offer_session.get())) {
fprintf(stderr, "Error writing settings.\n");
return 1;
}
- if (!DoConnection(&session, ssl_ctx.get(), config, &retry_config, is_resume,
- offer_session.get())) {
+ bool ok = DoConnection(&session, ssl_ctx.get(), config, &retry_config,
+ is_resume, offer_session.get());
+ if (!writer.Commit()) {
+ fprintf(stderr, "Error writing settings.\n");
+ return 1;
+ }
+ if (!ok) {
fprintf(stderr, "Connection %d failed.\n", i + 1);
ERR_print_errors_fp(stderr);
return 1;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ce20d22..866ad53 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -482,21 +482,23 @@
var testCases []testCase
-func writeTranscript(test *testCase, path string, data []byte) {
+func appendTranscript(path string, data []byte) error {
if len(data) == 0 {
- return
+ return nil
}
settings, err := ioutil.ReadFile(path)
if err != nil {
- fmt.Fprintf(os.Stderr, "Error reading %s: %s.\n", path, err)
- return
+ if !os.IsNotExist(err) {
+ return err
+ }
+ // If the shim aborted before writing a file, use a default
+ // settings block, so the transcript is still somewhat valid.
+ settings = []byte{0, 0} // kDataTag
}
settings = append(settings, data...)
- if err := ioutil.WriteFile(path, settings, 0644); err != nil {
- fmt.Fprintf(os.Stderr, "Error writing %s: %s\n", path, err)
- }
+ return ioutil.WriteFile(path, settings, 0644)
}
// A timeoutConn implements an idle timeout on each Read and Write operation.
@@ -523,7 +525,7 @@
return t.Conn.Write(b)
}
-func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, transcriptPrefix string, num int) error {
+func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, transcripts *[][]byte, num int) error {
if !test.noSessionCache {
if config.ClientSessionCache == nil {
config.ClientSessionCache = NewLRUClientSessionCache(1)
@@ -575,10 +577,13 @@
if *flagDebug {
defer connDebug.WriteTo(os.Stdout)
}
- if len(transcriptPrefix) != 0 {
+ if len(*transcriptDir) != 0 {
defer func() {
- path := transcriptPrefix + strconv.Itoa(num)
- writeTranscript(test, path, connDebug.Transcript())
+ if num == len(*transcripts) {
+ *transcripts = append(*transcripts, connDebug.Transcript())
+ } else {
+ panic("transcripts are out of sync")
+ }
}()
}
@@ -1118,6 +1123,7 @@
}
var transcriptPrefix string
+ var transcripts [][]byte
if len(*transcriptDir) != 0 {
protocol := "tls"
if test.protocol == dtls {
@@ -1176,7 +1182,7 @@
conn, err := acceptOrWait(listener, waitChan)
if err == nil {
- err = doExchange(test, &config, conn, false /* not a resumption */, transcriptPrefix, 0)
+ err = doExchange(test, &config, conn, false /* not a resumption */, &transcripts, 0)
conn.Close()
}
@@ -1196,7 +1202,7 @@
var connResume net.Conn
connResume, err = acceptOrWait(listener, waitChan)
if err == nil {
- err = doExchange(test, &resumeConfig, connResume, true /* resumption */, transcriptPrefix, i+1)
+ err = doExchange(test, &resumeConfig, connResume, true /* resumption */, &transcripts, i+1)
connResume.Close()
}
}
@@ -1217,6 +1223,14 @@
waitTimeout.Stop()
}
+ // Now that the shim has exitted, all the settings files have been
+ // written. Append the saved transcripts.
+ for i, transcript := range transcripts {
+ if err := appendTranscript(transcriptPrefix+strconv.Itoa(i), transcript); err != nil {
+ return err
+ }
+ }
+
var isValgrindError, mustFail bool
if exitError, ok := childErr.(*exec.ExitError); ok {
switch exitError.Sys().(syscall.WaitStatus).ExitStatus() {
