diff options
author | agl@chromium.org <agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-26 19:44:09 +0000 |
---|---|---|
committer | agl@chromium.org <agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-26 19:44:09 +0000 |
commit | d7a9328f5d93b8877d2e7ff34093537b39a3108b (patch) | |
tree | abcf828ee491da156b15ae3581d82d2de0b14baf /net | |
parent | d6a2b644891c820fa50203789dee80ef636bf1e8 (diff) | |
download | chromium_src-d7a9328f5d93b8877d2e7ff34093537b39a3108b.zip chromium_src-d7a9328f5d93b8877d2e7ff34093537b39a3108b.tar.gz chromium_src-d7a9328f5d93b8877d2e7ff34093537b39a3108b.tar.bz2 |
Fix the case where the browser livelocks if we cannot open a file.
If one tries to upload a file that one doesn't have read access to,
the browser livelocks. It tries to read from the file, gets nothing
but spins forever because it knows that it hasn't finished reading.
To address this, firstly we add a check at stat() time to make sure
that we can read the file. However, this doesn't take care of the case
where the access() call was incorrect, or the permissions have changed
under us. In this case, we replace the missing file with NULs.
(Land attempt three: first in r39446, reverted in r39448. Second in
r39899, reverted in r39901.)
http://codereview.chromium.org/541022
BUG=30850
TEST=Try to upload a file that isn't readable (i.e. /etc/shadow). The resulting upload should be a 0 byte file.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40146 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/base/file_stream.h | 13 | ||||
-rw-r--r-- | net/base/file_stream_posix.cc | 32 | ||||
-rw-r--r-- | net/base/file_stream_win.cc | 41 | ||||
-rw-r--r-- | net/base/upload_data.cc | 72 | ||||
-rw-r--r-- | net/base/upload_data.h | 47 | ||||
-rw-r--r-- | net/base/upload_data_stream.cc | 50 | ||||
-rw-r--r-- | net/base/upload_data_stream.h | 4 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_nss.cc | 6 | ||||
-rw-r--r-- | net/third_party/nss/README.google | 3 | ||||
-rw-r--r-- | net/third_party/nss/nss.gyp | 7 | ||||
-rw-r--r-- | net/third_party/nss/patches/falsestart.patch | 319 | ||||
-rw-r--r-- | net/third_party/nss/ssl/ssl.h | 11 | ||||
-rw-r--r-- | net/third_party/nss/ssl/ssl3con.c | 20 | ||||
-rw-r--r-- | net/third_party/nss/ssl/ssl3gthr.c | 13 | ||||
-rw-r--r-- | net/third_party/nss/ssl/sslimpl.h | 3 | ||||
-rw-r--r-- | net/third_party/nss/ssl/sslsecur.c | 6 | ||||
-rw-r--r-- | net/third_party/nss/ssl/sslsock.c | 11 |
17 files changed, 589 insertions, 69 deletions
diff --git a/net/base/file_stream.h b/net/base/file_stream.h index 6b7f3dc..def87bc 100644 --- a/net/base/file_stream.h +++ b/net/base/file_stream.h @@ -43,13 +43,24 @@ class FileStream { // Note that if there are any pending async operations, they'll be aborted. void Close(); + // Release performs the same actions as Close, but doesn't actually close the + // underlying PlatformFile. + void Release(); + // Call this method to open the FileStream. The remaining methods // cannot be used unless this method returns OK. If the file cannot be // opened then an error code is returned. // open_flags is a bitfield of base::PlatformFileFlags int Open(const FilePath& path, int open_flags); - // Returns true if Open succeeded and Close has not been called. + // Calling this method is functionally the same as constructing the object + // with the non-default constructor. This method can only be used if the + // FileSteam isn't currently open (i.e. was constructed with the default + // constructor). + int Open(base::PlatformFile file, int open_flags); + + // Returns true if Open succeeded and neither Close nor Release have been + // called. bool IsOpen() const; // Adjust the position from where data is read. Upon success, the stream diff --git a/net/base/file_stream_posix.cc b/net/base/file_stream_posix.cc index a4c5b3c..e947a6e 100644 --- a/net/base/file_stream_posix.cc +++ b/net/base/file_stream_posix.cc @@ -298,13 +298,8 @@ FileStream::FileStream() } FileStream::FileStream(base::PlatformFile file, int flags) - : file_(file), - open_flags_(flags) { - // If the file handle is opened with base::PLATFORM_FILE_ASYNC, we need to - // make sure we will perform asynchronous File IO to it. - if (flags & base::PLATFORM_FILE_ASYNC) { - async_context_.reset(new AsyncContext()); - } + : file_(base::kInvalidPlatformFileValue) { + Open(file, flags); } FileStream::~FileStream() { @@ -323,6 +318,12 @@ void FileStream::Close() { } } +void FileStream::Release() { + // Abort any existing asynchronous operations. + async_context_.reset(); + file_ = base::kInvalidPlatformFileValue; +} + int FileStream::Open(const FilePath& path, int open_flags) { if (IsOpen()) { DLOG(FATAL) << "File is already open!"; @@ -344,6 +345,21 @@ int FileStream::Open(const FilePath& path, int open_flags) { return OK; } +int FileStream::Open(base::PlatformFile file, int open_flags) { + if (IsOpen()) { + DLOG(FATAL) << "File is already open!"; + return ERR_UNEXPECTED; + } + + open_flags_ = open_flags; + file_ = file; + + if (open_flags & base::PLATFORM_FILE_ASYNC) + async_context_.reset(new AsyncContext()); + + return OK; +} + bool FileStream::IsOpen() const { return file_ != base::kInvalidPlatformFileValue; } @@ -445,7 +461,7 @@ int64 FileStream::Truncate(int64 bytes) { if (!IsOpen()) return ERR_UNEXPECTED; - // We better be open for reading. + // We better be open for writing. DCHECK(open_flags_ & base::PLATFORM_FILE_WRITE); // Seek to the position to truncate from. diff --git a/net/base/file_stream_win.cc b/net/base/file_stream_win.cc index cec6a9d..e1928a7 100644 --- a/net/base/file_stream_win.cc +++ b/net/base/file_stream_win.cc @@ -120,16 +120,10 @@ FileStream::FileStream() open_flags_(0) { } -FileStream::FileStream(base::PlatformFile file, int flags) - : file_(file), - open_flags_(flags) { - // If the file handle is opened with base::PLATFORM_FILE_ASYNC, we need to - // make sure we will perform asynchronous File IO to it. - if (flags & base::PLATFORM_FILE_ASYNC) { - async_context_.reset(new AsyncContext(this)); - MessageLoopForIO::current()->RegisterIOHandler(file_, - async_context_.get()); - } +FileStream::FileStream(base::PlatformFile file, int open_flags) + : file_(INVALID_HANDLE_VALUE), + open_flags_(0) { + Open(file, open_flags); } FileStream::~FileStream() { @@ -147,6 +141,13 @@ void FileStream::Close() { } } +void FileStream::Release() { + if (file_ != INVALID_HANDLE_VALUE) + CancelIo(file_); + async_context_.reset(); + file_ = INVALID_HANDLE_VALUE; +} + int FileStream::Open(const FilePath& path, int open_flags) { if (IsOpen()) { DLOG(FATAL) << "File is already open!"; @@ -170,6 +171,26 @@ int FileStream::Open(const FilePath& path, int open_flags) { return OK; } +int FileStream::Open(base::PlatformFile file, int open_flags) { + if (IsOpen()) { + DLOG(FATAL) << "File is already open!"; + return ERR_UNEXPECTED; + } + + open_flags_ = open_flags; + file_ = file; + + // If the file handle is opened with base::PLATFORM_FILE_ASYNC, we need to + // make sure we will perform asynchronous File IO to it. + if (open_flags_ & base::PLATFORM_FILE_ASYNC) { + async_context_.reset(new AsyncContext(this)); + MessageLoopForIO::current()->RegisterIOHandler(file_, + async_context_.get()); + } + + return OK; +} + bool FileStream::IsOpen() const { return file_ != INVALID_HANDLE_VALUE; } diff --git a/net/base/upload_data.cc b/net/base/upload_data.cc index 0496873..a23b25e 100644 --- a/net/base/upload_data.cc +++ b/net/base/upload_data.cc @@ -5,6 +5,7 @@ #include "net/base/upload_data.h" #include "base/file_util.h" +#include "base/platform_file.h" #include "base/logging.h" namespace net { @@ -17,28 +18,67 @@ uint64 UploadData::GetContentLength() const { return len; } -uint64 UploadData::Element::GetContentLength() const { - if (override_content_length_) - return content_length_; +void UploadData::CloseFiles() { + std::vector<Element>::iterator it = elements_.begin(); + for (; it != elements_.end(); ++it) { + if (it->type() == TYPE_FILE) + it->Close(); + } +} + +base::PlatformFile UploadData::Element::platform_file() const { + DCHECK(type_ == TYPE_FILE) << "platform_file on non-file Element"; + + return file_; +} + +void UploadData::Element::Close() { + DCHECK(type_ == TYPE_FILE) << "Close on non-file Element"; + + if (file_ != base::kInvalidPlatformFileValue) + base::ClosePlatformFile(file_); + file_ = base::kInvalidPlatformFileValue; +} + +void UploadData::Element::SetToFilePathRange(const FilePath& path, + uint64 offset, + uint64 length) { + type_ = TYPE_FILE; + file_range_offset_ = 0; + file_range_length_ = 0; - if (type_ == TYPE_BYTES) - return static_cast<uint64>(bytes_.size()); + Close(); - DCHECK(type_ == TYPE_FILE); + if (offset + length < offset) { + LOG(ERROR) << "Upload file offset and length overflow 64-bits. Ignoring."; + return; + } - // TODO(darin): This size calculation could be out of sync with the state of - // the file when we get around to reading it. We should probably find a way - // to lock the file or somehow protect against this error condition. + base::PlatformFile file = base::CreatePlatformFile( + path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, NULL); + if (file == base::kInvalidPlatformFileValue) { + // This case occurs when the user selects a file that isn't readable. + file_path_= path; + return; + } - int64 length = 0; - if (!file_util::GetFileSize(file_path_, &length)) - return 0; + uint64 file_size; + if (!base::GetPlatformFileSize(file, &file_size)) { + base::ClosePlatformFile(file); + return; + } - if (file_range_offset_ >= static_cast<uint64>(length)) - return 0; // range is beyond eof + if (offset > file_size) { + base::ClosePlatformFile(file); + return; + } + if (offset + length > file_size) + length = file_size - offset; - // compensate for the offset and clip file_range_length_ to eof - return std::min(length - file_range_offset_, file_range_length_); + file_ = file; + file_path_ = path; + file_range_offset_ = offset; + file_range_length_ = length; } } // namespace net diff --git a/net/base/upload_data.h b/net/base/upload_data.h index 3aab835..d01b435 100644 --- a/net/base/upload_data.h +++ b/net/base/upload_data.h @@ -9,6 +9,7 @@ #include "base/basictypes.h" #include "base/file_path.h" +#include "base/platform_file.h" #include "base/ref_counted.h" #include "testing/gtest/include/gtest/gtest_prod.h" @@ -26,7 +27,8 @@ class UploadData : public base::RefCounted<UploadData> { class Element { public: Element() : type_(TYPE_BYTES), file_range_offset_(0), - file_range_length_(kuint64max), + file_range_length_(0), + file_(base::kInvalidPlatformFileValue), override_content_length_(false) { } @@ -45,19 +47,42 @@ class UploadData : public base::RefCounted<UploadData> { SetToFilePathRange(path, 0, kuint64max); } - void SetToFilePathRange(const FilePath& path, - uint64 offset, uint64 length) { - type_ = TYPE_FILE; - file_path_ = path; - file_range_offset_ = offset; - file_range_length_ = length; - } + void SetToFilePathRange(const FilePath& path, uint64 offset, uint64 length); // Returns the byte-length of the element. For files that do not exist, 0 // is returned. This is done for consistency with Mozilla. - uint64 GetContentLength() const; + uint64 GetContentLength() const { + if (override_content_length_) + return content_length_; + + if (type_ == TYPE_BYTES) { + return bytes_.size(); + } else { + return file_range_length_; + } + } + + // For a TYPE_FILE, return a handle to the file. The caller does not take + // ownership and should not close the file handle. + base::PlatformFile platform_file() const; + + // For a TYPE_FILE, this closes the file handle. It's a fatal error to call + // platform_file() after this. + void Close(); private: + // type_ == TYPE_BYTES: + // bytes_ is valid + // type_ == TYPE_FILE: + // file_path_ should always be valid. + // + // platform_file() may be invalid, in which case file_range_* are 0 and + // file_ is invalid. This occurs when we cannot open the requested file. + // + // Else, then file_range_* are within range of the length of the file + // that we found when opening the file. Also, the sum of offset and + // length will not overflow a uint64. file_ will be handle to the file. + // Allows tests to override the result of GetContentLength. void SetContentLength(uint64 content_length) { override_content_length_ = true; @@ -69,6 +94,7 @@ class UploadData : public base::RefCounted<UploadData> { FilePath file_path_; uint64 file_range_offset_; uint64 file_range_length_; + base::PlatformFile file_; bool override_content_length_; uint64 content_length_; @@ -109,6 +135,9 @@ class UploadData : public base::RefCounted<UploadData> { elements_.swap(*elements); } + // CloseFiles closes the file handles of all Elements of type TYPE_FILE. + void CloseFiles(); + // Identifies a particular upload instance, which is used by the cache to // formulate a cache key. This value should be unique across browser // sessions. A value of 0 is used to indicate an unspecified identifier. diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc index 221ef28..9fca29e 100644 --- a/net/base/upload_data_stream.cc +++ b/net/base/upload_data_stream.cc @@ -10,7 +10,7 @@ namespace net { -UploadDataStream::UploadDataStream(const UploadData* data) +UploadDataStream::UploadDataStream(UploadData* data) : data_(data), buf_(new IOBuffer(kBufSize)), buf_len_(0), @@ -67,18 +67,19 @@ void UploadDataStream::FillBuf() { } else { DCHECK(element.type() == UploadData::TYPE_FILE); - if (!next_element_stream_.IsOpen()) { - int flags = base::PLATFORM_FILE_OPEN | - base::PLATFORM_FILE_READ; - int rv = next_element_stream_.Open(element.file_path(), flags); - // If the file does not exist, that's technically okay.. we'll just - // upload an empty file. This is for consistency with Mozilla. - DLOG_IF(WARNING, rv != OK) << "Failed to open \"" - << element.file_path().value() - << "\" for reading: " << rv; - - next_element_remaining_ = 0; // Default to reading nothing. - if (rv == OK) { + if (element.file_range_length() == 0) { + // If we failed to open the file, then the length is set to zero. The + // length used when calculating the POST size was also zero. This + // matches the behaviour of Mozilla. + next_element_remaining_ = 0; + } else { + if (!next_element_stream_.IsOpen()) { + // We ignore the return value of Open becuase we've already checked + // !IsOpen, above. + int flags = base::PLATFORM_FILE_READ | + base::PLATFORM_FILE_WRITE; + next_element_stream_.Open(element.platform_file(), flags); + uint64 offset = element.file_range_offset(); if (offset && next_element_stream_.Seek(FROM_BEGIN, offset) < 0) { DLOG(WARNING) << "Failed to seek \"" << element.file_path().value() @@ -90,11 +91,18 @@ void UploadDataStream::FillBuf() { } int rv = 0; - int count = static_cast<int>(std::min( - static_cast<uint64>(size_remaining), next_element_remaining_)); - if (count > 0 && - (rv = next_element_stream_.Read(buf_->data() + buf_len_, - count, NULL)) > 0) { + if (next_element_remaining_ > 0) { + int count = + static_cast<int>(std::min(next_element_remaining_, + static_cast<uint64>(size_remaining))); + rv = next_element_stream_.Read(buf_->data() + buf_len_, count, NULL); + if (rv < 1) { + // If the file was truncated between the time that we opened it and + // now, or if we got an error on reading, then we pad with NULs. + memset(buf_->data() + buf_len_, 0, count); + rv = count; + } + buf_len_ += rv; next_element_remaining_ -= rv; } else { @@ -105,12 +113,14 @@ void UploadDataStream::FillBuf() { if (advance_to_next_element) { ++next_element_; next_element_offset_ = 0; - next_element_stream_.Close(); + next_element_stream_.Release(); } } - if (next_element_ == end && !buf_len_) + if (next_element_ == end && !buf_len_) { eof_ = true; + data_->CloseFiles(); + } } } // namespace net diff --git a/net/base/upload_data_stream.h b/net/base/upload_data_stream.h index 0dd7dd3..9df5db0 100644 --- a/net/base/upload_data_stream.h +++ b/net/base/upload_data_stream.h @@ -14,7 +14,7 @@ class IOBuffer; class UploadDataStream { public: - explicit UploadDataStream(const UploadData* data); + explicit UploadDataStream(UploadData* data); ~UploadDataStream(); // Returns the stream's buffer and buffer length. @@ -42,7 +42,7 @@ class UploadDataStream { // left to fill the buffer with. void FillBuf(); - const UploadData* data_; + UploadData* data_; // This buffer is filled with data to be uploaded. The data to be sent is // always at the front of the buffer. If we cannot send all of the buffer at diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 311b054..da7f90a 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -380,6 +380,12 @@ int SSLClientSocketNSS::InitializeSSLOptions() { LOG(INFO) << "SSL_ENABLE_DEFLATE failed. Old system nss?"; #endif +#ifdef SSL_ENABLE_FALSE_START + rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_FALSE_START, PR_TRUE); + if (rv != SECSuccess) + LOG(INFO) << "SSL_ENABLE_FALSE_START failed. Old system nss?"; +#endif + #ifdef SSL_ENABLE_RENEGOTIATION // We allow servers to request renegotiation. Since we're a client, // prohibiting this is rather a waste of time. Only servers are in a position diff --git a/net/third_party/nss/README.google b/net/third_party/nss/README.google index ce46e1a..291e176 100644 --- a/net/third_party/nss/README.google +++ b/net/third_party/nss/README.google @@ -9,6 +9,9 @@ Patches: patches/nextproto.patch http://codereview.chromium.org/415005 + * False start support + patches/falsestart.patch + * Commenting out a couple of functions because they need NSS symbols which may not exist in the system NSS library. patches/versionskew.patch diff --git a/net/third_party/nss/nss.gyp b/net/third_party/nss/nss.gyp index 5511066..1e7ef49 100644 --- a/net/third_party/nss/nss.gyp +++ b/net/third_party/nss/nss.gyp @@ -126,6 +126,13 @@ }, }], ], + 'configurations': { + 'Debug_Base': { + 'defines': [ + 'DEBUG', + ], + }, + }, }, ], } diff --git a/net/third_party/nss/patches/falsestart.patch b/net/third_party/nss/patches/falsestart.patch new file mode 100644 index 0000000..ad41b20e --- /dev/null +++ b/net/third_party/nss/patches/falsestart.patch @@ -0,0 +1,319 @@ +diff --git a/mozilla/security/nss/cmd/strsclnt/strsclnt.c b/mozilla/security/nss/cmd/strsclnt/strsclnt.c +index c266644..1f71434 100644 +--- a/mozilla/security/nss/cmd/strsclnt/strsclnt.c ++++ b/mozilla/security/nss/cmd/strsclnt/strsclnt.c +@@ -162,6 +162,7 @@ static PRBool disableLocking = PR_FALSE; + static PRBool ignoreErrors = PR_FALSE; + static PRBool enableSessionTickets = PR_FALSE; + static PRBool enableCompression = PR_FALSE; ++static PRBool enableFalseStart = PR_FALSE; + + PRIntervalTime maxInterval = PR_INTERVAL_NO_TIMEOUT; + +@@ -197,7 +198,8 @@ Usage(const char *progName) + " -U means enable throttling up threads\n" + " -B bypasses the PKCS11 layer for SSL encryption and MACing\n" + " -u enable TLS Session Ticket extension\n" +- " -z enable compression\n", ++ " -z enable compression\n" ++ " -g enable false start\n", + progName); + exit(1); + } +@@ -1244,6 +1246,12 @@ client_main( + errExit("SSL_OptionSet SSL_ENABLE_DEFLATE"); + } + ++ if (enableFalseStart) { ++ rv = SSL_OptionSet(model_sock, SSL_ENABLE_FALSE_START, PR_TRUE); ++ if (rv != SECSuccess) ++ errExit("SSL_OptionSet SSL_ENABLE_FALSE_START"); ++ } ++ + SSL_SetURL(model_sock, hostName); + + SSL_AuthCertificateHook(model_sock, mySSLAuthCertificate, +@@ -1354,7 +1362,7 @@ main(int argc, char **argv) + + + optstate = PL_CreateOptState(argc, argv, +- "23BC:DNP:TUW:a:c:d:f:in:op:qst:uvw:z"); ++ "23BC:DNP:TUW:a:c:d:f:gin:op:qst:uvw:z"); + while ((status = PL_GetNextOpt(optstate)) == PL_OPT_OK) { + switch(optstate->option) { + +@@ -1384,6 +1392,8 @@ main(int argc, char **argv) + + case 'f': fileName = optstate->value; break; + ++ case 'g': enableFalseStart = PR_TRUE; break; ++ + case 'i': ignoreErrors = PR_TRUE; break; + + case 'n': nickName = PL_strdup(optstate->value); break; +diff --git a/mozilla/security/nss/cmd/tstclnt/tstclnt.c b/mozilla/security/nss/cmd/tstclnt/tstclnt.c +index c15a0ad..55684e6 100644 +--- a/mozilla/security/nss/cmd/tstclnt/tstclnt.c ++++ b/mozilla/security/nss/cmd/tstclnt/tstclnt.c +@@ -225,6 +225,7 @@ static void Usage(const char *progName) + fprintf(stderr, "%-20s Renegotiate N times (resuming session if N>1).\n", "-r N"); + fprintf(stderr, "%-20s Enable the session ticket extension.\n", "-u"); + fprintf(stderr, "%-20s Enable compression.\n", "-z"); ++ fprintf(stderr, "%-20s Enable false start.\n", "-g"); + fprintf(stderr, "%-20s Letter(s) chosen from the following list\n", + "-c ciphers"); + fprintf(stderr, +@@ -521,6 +522,7 @@ int main(int argc, char **argv) + int useExportPolicy = 0; + int enableSessionTickets = 0; + int enableCompression = 0; ++ int enableFalseStart = 0; + PRSocketOptionData opt; + PRNetAddr addr; + PRPollDesc pollset[2]; +@@ -551,7 +553,7 @@ int main(int argc, char **argv) + } + + optstate = PL_CreateOptState(argc, argv, +- "23BSTW:a:c:d:fh:m:n:op:qr:suvw:xz"); ++ "23BSTW:a:c:d:fgh:m:n:op:qr:suvw:xz"); + while ((optstatus = PL_GetNextOpt(optstate)) == PL_OPT_OK) { + switch (optstate->option) { + case '?': +@@ -578,6 +580,8 @@ int main(int argc, char **argv) + + case 'c': cipherString = PORT_Strdup(optstate->value); break; + ++ case 'g': enableFalseStart = 1; break; ++ + case 'd': certDir = PORT_Strdup(optstate->value); break; + + case 'f': clientSpeaksFirst = PR_TRUE; break; +@@ -863,7 +867,14 @@ int main(int argc, char **argv) + SECU_PrintError(progName, "error enabling compression"); + return 1; + } +- ++ ++ /* enable false start. */ ++ rv = SSL_OptionSet(s, SSL_ENABLE_FALSE_START, enableFalseStart); ++ if (rv != SECSuccess) { ++ SECU_PrintError(progName, "error enabling false start"); ++ return 1; ++ } ++ + SSL_SetPKCS11PinArg(s, &pwdata); + + SSL_AuthCertificateHook(s, SSL_AuthCertificate, (void *)handle); +diff --git a/mozilla/security/nss/lib/ssl/ssl.h b/mozilla/security/nss/lib/ssl/ssl.h +index e285ab4..bd1bfd3 100644 +--- a/mozilla/security/nss/lib/ssl/ssl.h ++++ b/mozilla/security/nss/lib/ssl/ssl.h +@@ -128,6 +128,17 @@ SSL_IMPORT PRFileDesc *SSL_ImportFD(PRFileDesc *model, PRFileDesc *fd); + /* Renegotiation Info (RI) */ + /* extension in ALL handshakes. */ + /* default: off */ ++#define SSL_ENABLE_FALSE_START 22 /* Enable SSL false start (off by */ ++ /* default, applies only to */ ++ /* clients). False start is a */ ++/* mode where an SSL client will start sending application data before */ ++/* verifying the server's Finished message. This means that we could end up */ ++/* sending data to an imposter. However, the data will be encrypted and */ ++/* only the true server can derive the session key. Thus, so long as the */ ++/* cipher isn't broken this is safe. Because of this, False Start will only */ ++/* occur on RSA or DH ciphersuites where the cipher's key length is >= 80 */ ++/* bits. The advantage of False Start is that it saves a round trip for */ ++/* client-speaks-first protocols when performing a full handshake. */ + + #ifdef SSL_DEPRECATED_FUNCTION + /* Old deprecated function names */ +diff --git a/mozilla/security/nss/lib/ssl/ssl3con.c b/mozilla/security/nss/lib/ssl/ssl3con.c +index 6b37c4f..f073431 100644 +--- a/mozilla/security/nss/lib/ssl/ssl3con.c ++++ b/mozilla/security/nss/lib/ssl/ssl3con.c +@@ -5656,7 +5656,17 @@ ssl3_RestartHandshakeAfterCertReq(sslSocket * ss, + return rv; + } + +- ++PRBool ++ssl3_CanFalseStart(sslSocket *ss) { ++ return ss->opt.enableFalseStart && ++ !ss->sec.isServer && ++ !ss->ssl3.hs.isResuming && ++ ss->ssl3.cwSpec && ++ ss->ssl3.cwSpec->cipher_def->secret_key_size >= 10 && ++ (ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_rsa || ++ ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_dh || ++ ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_ecdh); ++} + + /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete + * ssl3 Server Hello Done message. +@@ -5728,6 +5738,12 @@ ssl3_HandleServerHelloDone(sslSocket *ss) + ss->ssl3.hs.ws = wait_new_session_ticket; + else + ss->ssl3.hs.ws = wait_change_cipher; ++ ++ /* Do the handshake callback for sslv3 here. */ ++ if (ss->handshakeCallback != NULL && ssl3_CanFalseStart(ss)) { ++ (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); ++ } ++ + return SECSuccess; + + loser: +@@ -8468,7 +8484,7 @@ xmit_loser: + ss->ssl3.hs.ws = idle_handshake; + + /* Do the handshake callback for sslv3 here. */ +- if (ss->handshakeCallback != NULL) { ++ if (ss->handshakeCallback != NULL && !ssl3_CanFalseStart(ss)) { + (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); + } + +diff --git a/mozilla/security/nss/lib/ssl/ssl3gthr.c b/mozilla/security/nss/lib/ssl/ssl3gthr.c +index bdd2958..28fe154 100644 +--- a/mozilla/security/nss/lib/ssl/ssl3gthr.c ++++ b/mozilla/security/nss/lib/ssl/ssl3gthr.c +@@ -188,6 +188,7 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) + { + SSL3Ciphertext cText; + int rv; ++ PRBool canFalseStart = PR_FALSE; + + PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) ); + do { +@@ -207,7 +208,17 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) + if (rv < 0) { + return ss->recvdCloseNotify ? 0 : rv; + } +- } while (ss->ssl3.hs.ws != idle_handshake && ss->gs.buf.len == 0); ++ ++ if (ss->opt.enableFalseStart) { ++ ssl_GetSSL3HandshakeLock(ss); ++ canFalseStart = (ss->ssl3.hs.ws == wait_change_cipher || ++ ss->ssl3.hs.ws == wait_new_session_ticket) && ++ ssl3_CanFalseStart(ss); ++ ssl_ReleaseSSL3HandshakeLock(ss); ++ } ++ } while (ss->ssl3.hs.ws != idle_handshake && ++ !canFalseStart && ++ ss->gs.buf.len == 0); + + ss->gs.readOffset = 0; + ss->gs.writeOffset = ss->gs.buf.len; +diff --git a/mozilla/security/nss/lib/ssl/sslimpl.h b/mozilla/security/nss/lib/ssl/sslimpl.h +index 7581b98..00f0ce2 100644 +--- a/mozilla/security/nss/lib/ssl/sslimpl.h ++++ b/mozilla/security/nss/lib/ssl/sslimpl.h +@@ -333,6 +333,7 @@ typedef struct sslOptionsStr { + unsigned int enableDeflate : 1; /* 19 */ + unsigned int enableRenegotiation : 2; /* 20-21 */ + unsigned int requireSafeNegotiation : 1; /* 22 */ ++ unsigned int enableFalseStart : 1; /* 23 */ + } sslOptions; + + typedef enum { sslHandshakingUndetermined = 0, +@@ -1250,6 +1251,8 @@ extern void ssl_SetAlwaysBlock(sslSocket *ss); + + extern SECStatus ssl_EnableNagleDelay(sslSocket *ss, PRBool enabled); + ++extern PRBool ssl3_CanFalseStart(sslSocket *ss); ++ + #define SSL_LOCK_READER(ss) if (ss->recvLock) PZ_Lock(ss->recvLock) + #define SSL_UNLOCK_READER(ss) if (ss->recvLock) PZ_Unlock(ss->recvLock) + #define SSL_LOCK_WRITER(ss) if (ss->sendLock) PZ_Lock(ss->sendLock) +diff --git a/mozilla/security/nss/lib/ssl/sslsecur.c b/mozilla/security/nss/lib/ssl/sslsecur.c +index 8f79135..4dc0475 100644 +--- a/mozilla/security/nss/lib/ssl/sslsecur.c ++++ b/mozilla/security/nss/lib/ssl/sslsecur.c +@@ -148,6 +148,12 @@ ssl_Do1stHandshake(sslSocket *ss) + ss->gs.readOffset = 0; + break; + } ++ if (ss->version >= SSL_LIBRARY_VERSION_3_0 && ++ (ss->ssl3.hs.ws == wait_change_cipher || ++ ss->ssl3.hs.ws == wait_new_session_ticket) && ++ ssl3_CanFalseStart(ss)) { ++ break; ++ } + rv = (*ss->handshake)(ss); + ++loopCount; + /* This code must continue to loop on SECWouldBlock, +diff --git a/mozilla/security/nss/lib/ssl/sslsock.c b/mozilla/security/nss/lib/ssl/sslsock.c +index aab48d6..40f633a 100644 +--- a/mozilla/security/nss/lib/ssl/sslsock.c ++++ b/mozilla/security/nss/lib/ssl/sslsock.c +@@ -183,6 +183,7 @@ static sslOptions ssl_defaults = { + PR_FALSE, /* enableDeflate */ + 2, /* enableRenegotiation (default: requires extension) */ + PR_FALSE, /* requireSafeNegotiation */ ++ PR_FALSE, /* enableFalseStart */ + }; + + sslSessionIDLookupFunc ssl_sid_lookup; +@@ -728,6 +729,10 @@ SSL_OptionSet(PRFileDesc *fd, PRInt32 which, PRBool on) + ss->opt.requireSafeNegotiation = on; + break; + ++ case SSL_ENABLE_FALSE_START: ++ ss->opt.enableFalseStart = on; ++ break; ++ + default: + PORT_SetError(SEC_ERROR_INVALID_ARGS); + rv = SECFailure; +@@ -791,6 +796,7 @@ SSL_OptionGet(PRFileDesc *fd, PRInt32 which, PRBool *pOn) + on = ss->opt.enableRenegotiation; break; + case SSL_REQUIRE_SAFE_NEGOTIATION: + on = ss->opt.requireSafeNegotiation; break; ++ case SSL_ENABLE_FALSE_START: on = ss->opt.enableFalseStart; break; + + default: + PORT_SetError(SEC_ERROR_INVALID_ARGS); +@@ -841,6 +847,7 @@ SSL_OptionGetDefault(PRInt32 which, PRBool *pOn) + case SSL_REQUIRE_SAFE_NEGOTIATION: + on = ssl_defaults.requireSafeNegotiation; + break; ++ case SSL_ENABLE_FALSE_START: on = ssl_defaults.enableFalseStart; break; + + default: + PORT_SetError(SEC_ERROR_INVALID_ARGS); +@@ -984,6 +991,10 @@ SSL_OptionSetDefault(PRInt32 which, PRBool on) + ssl_defaults.requireSafeNegotiation = on; + break; + ++ case SSL_ENABLE_FALSE_START: ++ ssl_defaults.enableFalseStart = on; ++ break; ++ + default: + PORT_SetError(SEC_ERROR_INVALID_ARGS); + return SECFailure; +diff --git a/mozilla/security/nss/tests/ssl/sslstress.txt b/mozilla/security/nss/tests/ssl/sslstress.txt +index 9a3aae8..c2a5c76 100644 +--- a/mozilla/security/nss/tests/ssl/sslstress.txt ++++ b/mozilla/security/nss/tests/ssl/sslstress.txt +@@ -42,9 +42,11 @@ + noECC 0 _ -c_1000_-C_A Stress SSL2 RC4 128 with MD5 + noECC 0 _ -c_1000_-C_c_-T Stress SSL3 RC4 128 with MD5 + noECC 0 _ -c_1000_-C_c Stress TLS RC4 128 with MD5 ++ noECC 0 _ -c_1000_-C_c_-h Stress TLS RC4 128 with MD5 (false start) + noECC 0 -u -2_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket) + noECC 0 -z -2_-c_1000_-C_c_-z Stress TLS RC4 128 with MD5 (compression) + noECC 0 -u_-z -2_-c_1000_-C_c_-u_-z Stress TLS RC4 128 with MD5 (session ticket, compression) ++ noECC 0 -u_-z -2_-c_1000_-C_c_-u_-z_-h Stress TLS RC4 128 with MD5 (session ticket, compression, false start) + SNI 0 -u_-a_Host-sni.Dom -2_-3_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket, SNI) + + # +@@ -55,7 +57,9 @@ + noECC 0 -r_-r -c_100_-C_c_-N_-n_TestUser Stress TLS RC4 128 with MD5 (no reuse, client auth) + noECC 0 -r_-r_-u -2_-c_100_-C_c_-n_TestUser_-u Stress TLS RC4 128 with MD5 (session ticket, client auth) + noECC 0 -r_-r_-z -2_-c_100_-C_c_-n_TestUser_-z Stress TLS RC4 128 with MD5 (compression, client auth) ++ noECC 0 -r_-r_-z -2_-c_100_-C_c_-n_TestUser_-z_-h Stress TLS RC4 128 with MD5 (compression, client auth, false start) + noECC 0 -r_-r_-u_-z -2_-c_100_-C_c_-n_TestUser_-u_-z Stress TLS RC4 128 with MD5 (session ticket, compression, client auth) ++ noECC 0 -r_-r_-u_-z -2_-c_100_-C_c_-n_TestUser_-u_-z_-h Stress TLS RC4 128 with MD5 (session ticket, compression, client auth, false start) + SNI 0 -r_-r_-u_-a_Host-sni.Dom -2_-3_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket, SNI, client auth, default virt host) + SNI 0 -r_-r_-u_-a_Host-sni.Dom_-k_Host-sni.Dom -2_-3_-c_1000_-C_c_-u_-a_Host-sni.Dom Stress TLS RC4 128 with MD5 (session ticket, SNI, client auth, change virt host) + diff --git a/net/third_party/nss/ssl/ssl.h b/net/third_party/nss/ssl/ssl.h index b0e77df..0bc02f8 100644 --- a/net/third_party/nss/ssl/ssl.h +++ b/net/third_party/nss/ssl/ssl.h @@ -128,6 +128,17 @@ SSL_IMPORT PRFileDesc *SSL_ImportFD(PRFileDesc *model, PRFileDesc *fd); /* Renegotiation Info (RI) */ /* extension in ALL handshakes. */ /* default: off */ +#define SSL_ENABLE_FALSE_START 22 /* Enable SSL false start (off by */ + /* default, applies only to */ + /* clients). False start is a */ +/* mode where an SSL client will start sending application data before */ +/* verifying the server's Finished message. This means that we could end up */ +/* sending data to an imposter. However, the data will be encrypted and */ +/* only the true server can derive the session key. Thus, so long as the */ +/* cipher isn't broken this is safe. Because of this, False Start will only */ +/* occur on RSA or DH ciphersuites where the cipher's key length is >= 80 */ +/* bits. The advantage of False Start is that it saves a round trip for */ +/* client-speaks-first protocols when performing a full handshake. */ #ifdef SSL_DEPRECATED_FUNCTION /* Old deprecated function names */ diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c index 545e51e..24dc01c 100644 --- a/net/third_party/nss/ssl/ssl3con.c +++ b/net/third_party/nss/ssl/ssl3con.c @@ -5657,7 +5657,17 @@ ssl3_RestartHandshakeAfterCertReq(sslSocket * ss, return rv; } - +PRBool +ssl3_CanFalseStart(sslSocket *ss) { + return ss->opt.enableFalseStart && + !ss->sec.isServer && + !ss->ssl3.hs.isResuming && + ss->ssl3.cwSpec && + ss->ssl3.cwSpec->cipher_def->secret_key_size >= 10 && + (ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_rsa || + ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_dh || + ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_ecdh); +} /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete * ssl3 Server Hello Done message. @@ -5735,6 +5745,12 @@ ssl3_HandleServerHelloDone(sslSocket *ss) ss->ssl3.hs.ws = wait_new_session_ticket; else ss->ssl3.hs.ws = wait_change_cipher; + + /* Do the handshake callback for sslv3 here. */ + if (ss->handshakeCallback != NULL && ssl3_CanFalseStart(ss)) { + (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); + } + return SECSuccess; loser: @@ -8509,7 +8525,7 @@ xmit_loser: ss->ssl3.hs.ws = idle_handshake; /* Do the handshake callback for sslv3 here. */ - if (ss->handshakeCallback != NULL) { + if (ss->handshakeCallback != NULL && !ssl3_CanFalseStart(ss)) { (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); } diff --git a/net/third_party/nss/ssl/ssl3gthr.c b/net/third_party/nss/ssl/ssl3gthr.c index bdd2958..28fe154 100644 --- a/net/third_party/nss/ssl/ssl3gthr.c +++ b/net/third_party/nss/ssl/ssl3gthr.c @@ -188,6 +188,7 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) { SSL3Ciphertext cText; int rv; + PRBool canFalseStart = PR_FALSE; PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) ); do { @@ -207,7 +208,17 @@ ssl3_GatherCompleteHandshake(sslSocket *ss, int flags) if (rv < 0) { return ss->recvdCloseNotify ? 0 : rv; } - } while (ss->ssl3.hs.ws != idle_handshake && ss->gs.buf.len == 0); + + if (ss->opt.enableFalseStart) { + ssl_GetSSL3HandshakeLock(ss); + canFalseStart = (ss->ssl3.hs.ws == wait_change_cipher || + ss->ssl3.hs.ws == wait_new_session_ticket) && + ssl3_CanFalseStart(ss); + ssl_ReleaseSSL3HandshakeLock(ss); + } + } while (ss->ssl3.hs.ws != idle_handshake && + !canFalseStart && + ss->gs.buf.len == 0); ss->gs.readOffset = 0; ss->gs.writeOffset = ss->gs.buf.len; diff --git a/net/third_party/nss/ssl/sslimpl.h b/net/third_party/nss/ssl/sslimpl.h index 0658d2c..a800d56 100644 --- a/net/third_party/nss/ssl/sslimpl.h +++ b/net/third_party/nss/ssl/sslimpl.h @@ -338,6 +338,7 @@ typedef struct sslOptionsStr { unsigned int enableDeflate : 1; /* 19 */ unsigned int enableRenegotiation : 2; /* 20-21 */ unsigned int requireSafeNegotiation : 1; /* 22 */ + unsigned int enableFalseStart : 1; /* 23 */ } sslOptions; typedef enum { sslHandshakingUndetermined = 0, @@ -1266,6 +1267,8 @@ extern void ssl_SetAlwaysBlock(sslSocket *ss); extern SECStatus ssl_EnableNagleDelay(sslSocket *ss, PRBool enabled); +extern PRBool ssl3_CanFalseStart(sslSocket *ss); + #define SSL_LOCK_READER(ss) if (ss->recvLock) PZ_Lock(ss->recvLock) #define SSL_UNLOCK_READER(ss) if (ss->recvLock) PZ_Unlock(ss->recvLock) #define SSL_LOCK_WRITER(ss) if (ss->sendLock) PZ_Lock(ss->sendLock) diff --git a/net/third_party/nss/ssl/sslsecur.c b/net/third_party/nss/ssl/sslsecur.c index 80c2ba6..a8184478 100644 --- a/net/third_party/nss/ssl/sslsecur.c +++ b/net/third_party/nss/ssl/sslsecur.c @@ -148,6 +148,12 @@ ssl_Do1stHandshake(sslSocket *ss) ss->gs.readOffset = 0; break; } + if (ss->version >= SSL_LIBRARY_VERSION_3_0 && + (ss->ssl3.hs.ws == wait_change_cipher || + ss->ssl3.hs.ws == wait_new_session_ticket) && + ssl3_CanFalseStart(ss)) { + break; + } rv = (*ss->handshake)(ss); ++loopCount; /* This code must continue to loop on SECWouldBlock, diff --git a/net/third_party/nss/ssl/sslsock.c b/net/third_party/nss/ssl/sslsock.c index 722fe60..c4611a0 100644 --- a/net/third_party/nss/ssl/sslsock.c +++ b/net/third_party/nss/ssl/sslsock.c @@ -184,6 +184,7 @@ static sslOptions ssl_defaults = { PR_FALSE, /* enableDeflate */ 2, /* enableRenegotiation (default: requires extension) */ PR_FALSE, /* requireSafeNegotiation */ + PR_FALSE, /* enableFalseStart */ }; sslSessionIDLookupFunc ssl_sid_lookup; @@ -733,6 +734,10 @@ SSL_OptionSet(PRFileDesc *fd, PRInt32 which, PRBool on) ss->opt.requireSafeNegotiation = on; break; + case SSL_ENABLE_FALSE_START: + ss->opt.enableFalseStart = on; + break; + default: PORT_SetError(SEC_ERROR_INVALID_ARGS); rv = SECFailure; @@ -796,6 +801,7 @@ SSL_OptionGet(PRFileDesc *fd, PRInt32 which, PRBool *pOn) on = ss->opt.enableRenegotiation; break; case SSL_REQUIRE_SAFE_NEGOTIATION: on = ss->opt.requireSafeNegotiation; break; + case SSL_ENABLE_FALSE_START: on = ss->opt.enableFalseStart; break; default: PORT_SetError(SEC_ERROR_INVALID_ARGS); @@ -846,6 +852,7 @@ SSL_OptionGetDefault(PRInt32 which, PRBool *pOn) case SSL_REQUIRE_SAFE_NEGOTIATION: on = ssl_defaults.requireSafeNegotiation; break; + case SSL_ENABLE_FALSE_START: on = ssl_defaults.enableFalseStart; break; default: PORT_SetError(SEC_ERROR_INVALID_ARGS); @@ -989,6 +996,10 @@ SSL_OptionSetDefault(PRInt32 which, PRBool on) ssl_defaults.requireSafeNegotiation = on; break; + case SSL_ENABLE_FALSE_START: + ssl_defaults.enableFalseStart = on; + break; + default: PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; |