Bug 1360574 - Detect & handle connection auth at the http/2 layer. r?mcmanus
This is also the non-broken way to fix
bug 1346392. Instead of waiting
until the auth handler gets its hands on things, we break layering a bit
and inspect the response headers as soon as we decompress them to see if
there's any connection-oriented auth being requested. If there is, we
treat the situation as if we got a RST_STREAM or GOAWAY with
HTTP_1_1_REQUIRED.
We were able to re-purpose the NS_ERROR_ABORT code path that was
previously used with an inappropriate HTTP status code when talking to
an HTTPS proxy over http/2, as that usage was removed a while back from
the stream, though we still had the (dead) code in the session to handle
the stream giving us that return value. The error code was changed to
NS_ERROR_NET_RESET, however, to give a better description of what's
going on.
MozReview-Commit-ID: DLjOIIiXGrV
--- a/netwerk/protocol/http/Http2Compression.cpp
+++ b/netwerk/protocol/http/Http2Compression.cpp
@@ -12,16 +12,17 @@
#define LOG(args) LOG5(args)
#undef LOG_ENABLED
#define LOG_ENABLED() LOG5_ENABLED()
#include "Http2Compression.h"
#include "Http2HuffmanIncoming.h"
#include "Http2HuffmanOutgoing.h"
#include "mozilla/StaticPtr.h"
+#include "nsCharSeparatedTokenizer.h"
#include "nsHttpHandler.h"
namespace mozilla {
namespace net {
static nsDeque *gStaticHeaders = nullptr;
class HpackStaticTableReporter final : public nsIMemoryReporter
@@ -447,16 +448,22 @@ Http2Decompressor::DecodeHeaderBlock(con
// This is an http-level error that we can handle by resetting the stream
// in the upper layers. Let's note that we saw this, then continue
// decompressing until we either hit the end of the header block or find a
// hard failure. That way we won't get an inconsistent compression state
// with the server.
softfail_rv = rv;
rv = NS_OK;
+ } else if (rv == NS_ERROR_NET_RESET) {
+ // This happens when we detect connection-based auth being requested in
+ // the response headers. We'll paper over it for now, and the session will
+ // handle this as if it received RST_STREAM with HTTP_1_1_REQUIRED.
+ softfail_rv = rv;
+ rv = NS_OK;
}
}
if (NS_FAILED(rv)) {
return rv;
}
return softfail_rv;
@@ -513,16 +520,33 @@ Http2Decompressor::DecodeInteger(uint32_
chainBit = mData[mOffset] & 0x80;
accum += (mData[mOffset] & 0x7f) * factor;
++mOffset;
factor = factor * 128;
}
return NS_OK;
}
+static bool
+HasConnectionBasedAuth(const nsACString& headerValue)
+{
+ nsCCharSeparatedTokenizer t(headerValue, '\n');
+ while (t.hasMoreTokens()) {
+ const nsDependentCSubstring& authMethod = t.nextToken();
+ if (authMethod.LowerCaseEqualsLiteral("ntlm")) {
+ return true;
+ }
+ if (authMethod.LowerCaseEqualsLiteral("negotiate")) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
nsresult
Http2Decompressor::OutputHeader(const nsACString &name, const nsACString &value)
{
// exclusions
if (!mIsPush &&
(name.EqualsLiteral("connection") ||
name.EqualsLiteral("host") ||
name.EqualsLiteral("keep-alive") ||
@@ -608,16 +632,30 @@ Http2Decompressor::OutputHeader(const ns
LOG(("Http2Decompressor::OutputHeader %s %s", name.BeginReading(),
value.BeginReading()));
mSeenNonColonHeader = true;
mOutput->Append(name);
mOutput->AppendLiteral(": ");
mOutput->Append(value);
mOutput->AppendLiteral("\r\n");
+
+ // Need to check if the server is going to try to speak connection-based auth
+ // with us. If so, we need to kill this via h2, and dial back with http/1.1.
+ // Technically speaking, the server should've just reset or goaway'd us with
+ // HTTP_1_1_REQUIRED, but there are some busted servers out there, so we need
+ // to check on our own to work around them.
+ if (name.EqualsLiteral("www-authenticate") ||
+ name.EqualsLiteral("proxy-authenticate")) {
+ if (HasConnectionBasedAuth(value)) {
+ LOG3(("Http2Decompressor %p connection-based auth found in %s", this,
+ name.BeginReading()));
+ return NS_ERROR_NET_RESET;
+ }
+ }
return NS_OK;
}
nsresult
Http2Decompressor::OutputHeader(uint32_t index)
{
// NWGH - make this < index
// bounds check
@@ -952,28 +990,30 @@ Http2Decompressor::DoLiteralWithIncremen
// this starts with 01 bit pattern
MOZ_ASSERT((mData[mOffset] & 0xC0) == 0x40);
nsAutoCString name, value;
nsresult rv = DoLiteralInternal(name, value, 6);
if (NS_SUCCEEDED(rv)) {
rv = OutputHeader(name, value);
}
- if (NS_FAILED(rv)) {
+ // Let NET_RESET continue on so that we don't get out of sync, as it is just
+ // used to kill the stream, not the session.
+ if (NS_FAILED(rv) && rv != NS_ERROR_NET_RESET) {
return rv;
}
uint32_t room = nvPair(name, value).Size();
if (room > mMaxBuffer) {
ClearHeaderTable();
LOG(("HTTP decompressor literal with index not inserted due to size %u %s %s\n",
room, name.get(), value.get()));
LOG(("Decompressor state after ClearHeaderTable"));
DumpState();
- return NS_OK;
+ return rv;
}
MakeRoom(room, "decompressor");
// Incremental Indexing implicitly adds a row to the header table.
mHeaderTable.AddElement(name, value);
uint32_t currentSize = mHeaderTable.ByteCount();
@@ -984,17 +1024,17 @@ Http2Decompressor::DoLiteralWithIncremen
uint32_t currentCount = mHeaderTable.VariableLength();
if (currentCount > mPeakCount) {
mPeakCount = currentCount;
}
LOG(("HTTP decompressor literal with index 0 %s %s\n",
name.get(), value.get()));
- return NS_OK;
+ return rv;
}
nsresult
Http2Decompressor::DoLiteralNeverIndexed()
{
// This starts with 0001 bit pattern
MOZ_ASSERT((mData[mOffset] & 0xF0) == 0x10);
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -1384,28 +1384,22 @@ Http2Session::ResponseHeadersComplete()
nsresult rv;
int32_t httpResponseCode; // out param to ConvertResponseHeaders
mFlatHTTPResponseHeadersOut = 0;
rv = mInputFrameDataStream->ConvertResponseHeaders(&mDecompressor,
mDecompressBuffer,
mFlatHTTPResponseHeaders,
httpResponseCode);
- if (rv == NS_ERROR_ABORT) {
- LOG(("Http2Session::ResponseHeadersComplete ConvertResponseHeaders aborted\n"));
- if (mInputFrameDataStream->IsTunnel()) {
- rv = gHttpHandler->ConnMgr()->CancelTransactions(
- mInputFrameDataStream->Transaction()->ConnectionInfo(),
- NS_ERROR_CONNECTION_REFUSED);
- if (NS_FAILED(rv)) {
- LOG(("Http2Session::ResponseHeadersComplete "
- "CancelTransactions failed: %08x\n", static_cast<uint32_t>(rv)));
- }
- }
- CleanupStream(mInputFrameDataStream, rv, CANCEL_ERROR);
+ if (rv == NS_ERROR_NET_RESET) {
+ LOG(("Http2Session::ResponseHeadersComplete %p ConvertResponseHeaders reset\n", this));
+ // This means the stream found connection-oriented auth. Treat this like we
+ // got a reset with HTTP_1_1_REQUIRED.
+ mInputFrameDataStream->Transaction()->DisableSpdy();
+ CleanupStream(mInputFrameDataStream, NS_ERROR_NET_RESET, CANCEL_ERROR);
ResetDownstreamState();
return NS_OK;
} else if (NS_FAILED(rv)) {
return rv;
}
// allow more headers in the case of 1xx
if (((httpResponseCode / 100) == 1) && didFirstSetAllRecvd) {