Abstract fd operations better in tool. Windows and POSIX implement very similar fd operations, but differ slightly: - ssize_t in POSIX is usually int on Windows. - POSIX needs EINTR retry loops. - Windows wants _open rather than open, etc. - POSIX fds and sockets are the same thing, while Windows sockets are HANDLEs and leaves fd as a C runtime construct. Rather than ad-hoc macros and redefinitions of ssize_t (which reportedly upset MINGW), add some actual abstractions. While I'm here, add a scoped file descriptor type. That still leaves recv/send which are only used in one file, so defined a socket_result_t for them. Change-Id: I17fca2a50c77191f573852bfd27553996e3e9c3f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41725 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/tool/CMakeLists.txt b/tool/CMakeLists.txt index 7f34017..7658713 100644 --- a/tool/CMakeLists.txt +++ b/tool/CMakeLists.txt
@@ -8,6 +8,7 @@ client.cc const.cc digest.cc + fd.cc file.cc generate_ed25519.cc genrsa.cc
diff --git a/tool/digest.cc b/tool/digest.cc index 742fa7f..4adaf8d 100644 --- a/tool/digest.cc +++ b/tool/digest.cc
@@ -37,7 +37,6 @@ OPENSSL_MSVC_PRAGMA(warning(pop)) #include <io.h> #define PATH_MAX MAX_PATH -typedef int ssize_t; #endif #include <openssl/digest.h> @@ -45,19 +44,6 @@ #include "internal.h" -struct close_delete { - void operator()(int *fd) { - BORINGSSL_CLOSE(*fd); - } -}; - -template<typename T, typename R, R (*func) (T*)> -struct func_delete { - void operator()(T* obj) { - func(obj); - } -}; - // Source is an awkward expression of a union type in C++: Stdin | File filename. struct Source { enum Type { @@ -79,37 +65,31 @@ static const char kStdinName[] = "standard input"; -// OpenFile opens the regular file named |filename| and sets |*out_fd| to be a -// file descriptor to it. Returns true on sucess or prints an error to stderr -// and returns false on error. -static bool OpenFile(int *out_fd, const std::string &filename) { - *out_fd = -1; - - int fd = BORINGSSL_OPEN(filename.c_str(), O_RDONLY | O_BINARY); - if (fd < 0) { +// OpenFile opens the regular file named |filename| and returns a file +// descriptor to it. +static ScopedFD OpenFile(const std::string &filename) { + ScopedFD fd = OpenFD(filename.c_str(), O_RDONLY | O_BINARY); + if (!fd) { fprintf(stderr, "Failed to open input file '%s': %s\n", filename.c_str(), strerror(errno)); - return false; + return ScopedFD(); } - std::unique_ptr<int, close_delete> scoped_fd(&fd); #if !defined(OPENSSL_WINDOWS) struct stat st; - if (fstat(fd, &st)) { + if (fstat(fd.get(), &st)) { fprintf(stderr, "Failed to stat input file '%s': %s\n", filename.c_str(), strerror(errno)); - return false; + return ScopedFD(); } if (!S_ISREG(st.st_mode)) { fprintf(stderr, "%s: not a regular file\n", filename.c_str()); - return false; + return ScopedFD(); } #endif - *out_fd = fd; - scoped_fd.release(); - return true; + return fd; } // SumFile hashes the contents of |source| with |md| and sets |*out_hex| to the @@ -119,16 +99,17 @@ // error. static bool SumFile(std::string *out_hex, const EVP_MD *md, const Source &source) { - std::unique_ptr<int, close_delete> scoped_fd; + ScopedFD scoped_fd; int fd; if (source.is_stdin()) { fd = 0; } else { - if (!OpenFile(&fd, source.filename())) { + scoped_fd = OpenFile(source.filename()); + if (!scoped_fd) { return false; } - scoped_fd.reset(&fd); + fd = scoped_fd.get(); } static const size_t kBufSize = 8192; @@ -141,21 +122,18 @@ } for (;;) { - ssize_t n; - - do { - n = BORINGSSL_READ(fd, buf.get(), kBufSize); - } while (n == -1 && errno == EINTR); - - if (n == 0) { - break; - } else if (n < 0) { + size_t n; + if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { fprintf(stderr, "Failed to read from %s: %s\n", source.is_stdin() ? kStdinName : source.filename().c_str(), strerror(errno)); return false; } + if (n == 0) { + break; + } + if (!EVP_DigestUpdate(ctx.get(), buf.get(), n)) { fprintf(stderr, "Failed to update hash.\n"); return false; @@ -221,25 +199,23 @@ // returns false. static bool Check(const CheckModeArguments &args, const EVP_MD *md, const Source &source) { - std::unique_ptr<FILE, func_delete<FILE, int, fclose>> scoped_file; FILE *file; + ScopedFILE scoped_file; if (source.is_stdin()) { file = stdin; } else { - int fd; - if (!OpenFile(&fd, source.filename())) { + ScopedFD fd = OpenFile(source.filename()); + if (!fd) { return false; } - file = BORINGSSL_FDOPEN(fd, "rb"); - if (!file) { + scoped_file = FDToFILE(std::move(fd), "rb"); + if (!scoped_file) { perror("fdopen"); - BORINGSSL_CLOSE(fd); return false; } - - scoped_file = std::unique_ptr<FILE, func_delete<FILE, int, fclose>>(file); + file = scoped_file.get(); } const size_t hex_size = EVP_MD_size(md) * 2;
diff --git a/tool/fd.cc b/tool/fd.cc new file mode 100644 index 0000000..2c27ccd --- /dev/null +++ b/tool/fd.cc
@@ -0,0 +1,105 @@ +/* Copyright (c) 2020, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#include <openssl/base.h> + +#include <errno.h> +#include <limits.h> +#include <stdio.h> + +#include <algorithm> + +#include "internal.h" + +#if defined(OPENSSL_WINDOWS) +#include <io.h> +#else +#include <fcntl.h> +#include <unistd.h> +#endif + + +ScopedFD OpenFD(const char *path, int flags) { +#if defined(OPENSSL_WINDOWS) + return ScopedFD(_open(path, flags)); +#else + int fd; + do { + fd = open(path, flags); + } while (fd == -1 && errno == EINTR); + return ScopedFD(fd); +#endif +} + +void CloseFD(int fd) { +#if defined(OPENSSL_WINDOWS) + _close(fd); +#else + close(fd); +#endif +} + +bool ReadFromFD(int fd, size_t *out_bytes_read, void *out, size_t num) { +#if defined(OPENSSL_WINDOWS) + // On Windows, the buffer must be at most |INT_MAX|. See + // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/read?view=vs-2019 + int ret = _read(fd, out, std::min(size_t{INT_MAX}, num)); +#else + ssize_t ret; + do { + ret = read(fd, out, num); + } while (ret == -1 && errno == EINVAL); +#endif + + if (ret < 0) { + *out_bytes_read = 0; + return false; + } + *out_bytes_read = ret; + return true; +} + +bool WriteToFD(int fd, size_t *out_bytes_written, const void *in, size_t num) { +#if defined(OPENSSL_WINDOWS) + // The documentation for |_write| does not say the buffer must be at most + // |INT_MAX|, but clamp it to |INT_MAX| instead of |UINT_MAX| in case. + int ret = _write(fd, in, std::min(size_t{INT_MAX}, num)); +#else + ssize_t ret; + do { + ret = write(fd, in, num); + } while (ret == -1 && errno == EINVAL); +#endif + + if (ret < 0) { + *out_bytes_written = 0; + return false; + } + *out_bytes_written = ret; + return true; +} + +ScopedFILE FDToFILE(ScopedFD fd, const char *mode) { + ScopedFILE ret; +#if defined(OPENSSL_WINDOWS) + ret.reset(_fdopen(fd.get(), mode)); +#else + ret.reset(fdopen(fd.get(), mode)); +#endif + // |fdopen| takes ownership of |fd| on success. + if (ret) { + fd.release(); + } + return ret; +}
diff --git a/tool/internal.h b/tool/internal.h index 4cace9d..eb9e4ba 100644 --- a/tool/internal.h +++ b/tool/internal.h
@@ -18,32 +18,17 @@ #include <openssl/base.h> #include <string> +#include <utility> #include <vector> -OPENSSL_MSVC_PRAGMA(warning(push)) // MSVC issues warning C4702 for unreachable code in its xtree header when // compiling with -D_HAS_EXCEPTIONS=0. See // https://connect.microsoft.com/VisualStudio/feedback/details/809962 +OPENSSL_MSVC_PRAGMA(warning(push)) OPENSSL_MSVC_PRAGMA(warning(disable: 4702)) - #include <map> - OPENSSL_MSVC_PRAGMA(warning(pop)) -#if defined(OPENSSL_WINDOWS) - #define BORINGSSL_OPEN _open - #define BORINGSSL_FDOPEN _fdopen - #define BORINGSSL_CLOSE _close - #define BORINGSSL_READ _read - #define BORINGSSL_WRITE _write -#else - #define BORINGSSL_OPEN open - #define BORINGSSL_FDOPEN fdopen - #define BORINGSSL_CLOSE close - #define BORINGSSL_READ read - #define BORINGSSL_WRITE write -#endif - struct FileCloser { void operator()(FILE *file) { fclose(file); @@ -52,6 +37,67 @@ using ScopedFILE = std::unique_ptr<FILE, FileCloser>; +// The following functions abstract between POSIX and Windows differences in +// file descriptor I/O functions. + +// CloseFD behaves like |close|. +void CloseFD(int fd); + +class ScopedFD { + public: + ScopedFD() {} + explicit ScopedFD(int fd) : fd_(fd) {} + ScopedFD(ScopedFD &&other) { *this = std::move(other); } + ScopedFD(const ScopedFD &) = delete; + ~ScopedFD() { reset(); } + + ScopedFD &operator=(const ScopedFD &) = delete; + ScopedFD &operator=(ScopedFD &&other) { + reset(); + fd_ = other.fd_; + other.fd_ = -1; + return *this; + } + + explicit operator bool() const { return fd_ >= 0; } + + int get() const { return fd_; } + + void reset() { + if (fd_ >= 0) { + CloseFD(fd_); + } + fd_ = -1; + } + + int release() { + int fd = fd_; + fd_ = -1; + return fd; + } + + private: + int fd_ = -1; +}; + +// OpenFD behaves like |open| but handles |EINTR| and works on Windows. +ScopedFD OpenFD(const char *path, int flags); + +// ReadFromFD reads up to |num| bytes from |fd| and writes the result to |out|. +// On success, it returns true and sets |*out_bytes_read| to the number of bytes +// read. Otherwise, it returns false and leaves an error in |errno|. On POSIX, +// it handles |EINTR| internally. +bool ReadFromFD(int fd, size_t *out_bytes_read, void *out, size_t num); + +// WriteToFD writes up to |num| bytes from |in| to |fd|. On success, it returns +// true and sets |*out_bytes_written| to the number of bytes written. Otherwise, +// it returns false and leaves an error in |errno|. On POSIX, it handles |EINTR| +// internally. +bool WriteToFD(int fd, size_t *out_bytes_written, const void *in, size_t num); + +// FDToFILE behaves like |fdopen|. +ScopedFILE FDToFILE(ScopedFD fd, const char *mode); + enum ArgumentType { kRequiredArgument, kOptionalArgument,
diff --git a/tool/pkcs12.cc b/tool/pkcs12.cc index a8ddb0e..3d8a1cd 100644 --- a/tool/pkcs12.cc +++ b/tool/pkcs12.cc
@@ -40,12 +40,6 @@ #include "internal.h" -#if defined(OPENSSL_WINDOWS) -typedef int read_result_t; -#else -typedef ssize_t read_result_t; -#endif - static const struct argument kArguments[] = { { "-dump", kOptionalArgument, @@ -65,51 +59,52 @@ return false; } - int fd = BORINGSSL_OPEN(args_map["-dump"].c_str(), O_RDONLY); - if (fd < 0) { + ScopedFD fd = OpenFD(args_map["-dump"].c_str(), O_RDONLY); + if (!fd) { perror("open"); return false; } struct stat st; - if (fstat(fd, &st)) { + if (fstat(fd.get(), &st)) { perror("fstat"); - BORINGSSL_CLOSE(fd); return false; } const size_t size = st.st_size; std::unique_ptr<uint8_t[]> contents(new uint8_t[size]); - read_result_t n; size_t off = 0; - do { - n = BORINGSSL_READ(fd, &contents[off], size - off); - if (n >= 0) { - off += static_cast<size_t>(n); + while (off < size) { + size_t bytes_read; + if (!ReadFromFD(fd.get(), &bytes_read, contents.get() + off, size - off)) { + perror("read"); + return false; } - } while ((n > 0 && off < size) || (n == -1 && errno == EINTR)); - - if (off != size) { - perror("read"); - BORINGSSL_CLOSE(fd); - return false; + if (bytes_read == 0) { + fprintf(stderr, "Unexpected EOF\n"); + return false; + } + off += bytes_read; } - BORINGSSL_CLOSE(fd); - printf("Enter password: "); fflush(stdout); char password[256]; off = 0; - do { - n = BORINGSSL_READ(0, &password[off], sizeof(password) - 1 - off); - if (n >= 0) { - off += static_cast<size_t>(n); + while (off < sizeof(password) - 1) { + size_t bytes_read; + if (!ReadFromFD(0, &bytes_read, password + off, + sizeof(password) - 1 - off)) { + perror("read"); + return false; } - } while ((n > 0 && OPENSSL_memchr(password, '\n', off) == NULL && - off < sizeof(password) - 1) || - (n == -1 && errno == EINTR)); + + off += bytes_read; + if (bytes_read == 0 || OPENSSL_memchr(password, '\n', off) != nullptr) { + break; + } + } char *newline = reinterpret_cast<char *>(OPENSSL_memchr(password, '\n', off)); if (newline == NULL) {
diff --git a/tool/transport_common.cc b/tool/transport_common.cc index 7c5e962..88e9169 100644 --- a/tool/transport_common.cc +++ b/tool/transport_common.cc
@@ -55,7 +55,6 @@ #include <ws2tcpip.h> OPENSSL_MSVC_PRAGMA(warning(pop)) -typedef int ssize_t; OPENSSL_MSVC_PRAGMA(comment(lib, "Ws2_32.lib")) #endif @@ -68,7 +67,10 @@ #include "transport_common.h" -#if !defined(OPENSSL_WINDOWS) +#if defined(OPENSSL_WINDOWS) +using socket_result_t = int; +#else +using socket_result_t = ssize_t; static int closesocket(int sock) { return close(sock); } @@ -739,12 +741,13 @@ return true; } - ssize_t n; - do { - n = BORINGSSL_WRITE(1, buffer, ssl_ret); - } while (n == -1 && errno == EINTR); + size_t n; + if (!WriteToFD(1, &n, buffer, ssl_ret)) { + fprintf(stderr, "Error writing to stdout.\n"); + return false; + } - if (n != ssl_ret) { + if (n != static_cast<size_t>(ssl_ret)) { fprintf(stderr, "Short write to stderr.\n"); return false; } @@ -786,7 +789,7 @@ return false; } - ssize_t n; + socket_result_t n; do { n = recv(sock_, &buf_[buf_len_], sizeof(buf_) - buf_len_, 0); } while (n == -1 && errno == EINTR); @@ -871,7 +874,7 @@ size_t done = 0; while (done < data_len) { - ssize_t n; + socket_result_t n; do { n = send(sock, &data[done], data_len - done, 0); } while (n == -1 && errno == EINTR);