SLFO-pool/grpc
SHA256
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Sync from SUSE:SLFO:Main grpc revision a1ef6217985585227ea544ad7e8442b9

Browse Source
This commit is contained in:
Adrian Schröter 2025-02-25 19:22:32 +01:00
parent d7d6bdf277
commit c7d8fb54b0
4 changed files with 448 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
grpc-CVE-2024-11407.patch Normal file
View File

@ -0,0 +1,25 @@
From e9046b2bbebc0cb7f5dc42008f807f6c7e98e791 Mon Sep 17 00:00:00 2001
From: Vignesh Babu <vigneshbabu@google.com>
Date: Thu, 12 Sep 2024 11:13:45 -0700
Subject: [PATCH] [EventEngine] Fix bug in Tx0cp code path in posix endpoint.
This fix ensures that the iov_base pointers point to the right address.
PiperOrigin-RevId: 673923651
---
src/core/lib/event_engine/posix_engine/posix_endpoint.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: grpc-1.60.0/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
===================================================================
--- grpc-1.60.0.orig/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
+++ grpc-1.60.0/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
@@ -239,7 +239,7 @@ msg_iovlen_type TcpZerocopySendRecord::P
iov_size++) {
MutableSlice& slice = internal::SliceCast<MutableSlice>(
buf_.MutableSliceAt(out_offset_.slice_idx));
- iov[iov_size].iov_base = slice.begin();
+ iov[iov_size].iov_base = slice.begin() + out_offset_.byte_idx;
iov[iov_size].iov_len = slice.length() - out_offset_.byte_idx;
*sending_length += iov[iov_size].iov_len;
++(out_offset_.slice_idx);

409
grpc-CVE-2024-7246.patch Normal file
View File

@ -0,0 +1,409 @@
From 4a3092a29cf0cf53595c137c7b20909255f78923 Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Thu, 1 Aug 2024 13:02:21 -0700
Subject: [PATCH] [v1.59] [chttp2] Fix a bug in hpack error handling (#37367)
PiperOrigin-RevId: 657234128
PiperOrigin-RevId: 658458047
---
.../chttp2/transport/hpack_parser.cc | 63 +++++++------
.../transport/chttp2/transport/hpack_parser.h | 2 +
.../transport/chttp2/hpack_parser_test.cc | 89 ++++++++++++++++---
.../transport/chttp2/hpack_sync_fuzzer.cc | 62 +++++++++++++
.../transport/chttp2/hpack_sync_fuzzer.proto | 3 +
5 files changed, 179 insertions(+), 40 deletions(-)
diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
index 31bf46456f1d0..f2fe80c504e58 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc
+++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
@@ -91,12 +91,14 @@ constexpr Base64InverseTable kBase64InverseTable;
class HPackParser::Input {
public:
Input(grpc_slice_refcount* current_slice_refcount, const uint8_t* begin,
- const uint8_t* end, absl::BitGenRef bitsrc, HpackParseResult& error)
+ const uint8_t* end, absl::BitGenRef bitsrc,
+ HpackParseResult& frame_error, HpackParseResult& field_error)
: current_slice_refcount_(current_slice_refcount),
begin_(begin),
end_(end),
frontier_(begin),
- error_(error),
+ frame_error_(frame_error),
+ field_error_(field_error),
bitsrc_(bitsrc) {}
// If input is backed by a slice, retrieve its refcount. If not, return
@@ -215,14 +217,18 @@ class HPackParser::Input {
// Check if we saw an EOF
bool eof_error() const {
- return min_progress_size_ != 0 || error_.connection_error();
+ return min_progress_size_ != 0 || frame_error_.connection_error();
+ }
+
+ // Reset the field error to be ok
+ void ClearFieldError() {
+ if (field_error_.ok()) return;
+ field_error_ = HpackParseResult();
}
// Minimum number of bytes to unstuck the current parse
size_t min_progress_size() const { return min_progress_size_; }
- bool has_error() const { return !error_.ok(); }
-
// Set the current error - tweaks the error to include a stream id so that
// chttp2 does not close the connection.
// Intended for errors that are specific to a stream and recoverable.
@@ -246,10 +252,7 @@ class HPackParser::Input {
// read prior to being able to get further in this parse.
void UnexpectedEOF(size_t min_progress_size) {
GPR_ASSERT(min_progress_size > 0);
- if (min_progress_size_ != 0 || error_.connection_error()) {
- GPR_DEBUG_ASSERT(eof_error());
- return;
- }
+ if (eof_error()) return;
// Set min progress size, taking into account bytes parsed already but not
// consumed.
min_progress_size_ = min_progress_size + (begin_ - frontier_);
@@ -302,13 +305,18 @@ class HPackParser::Input {
// Do not use this directly, instead use SetErrorAndContinueParsing or
// SetErrorAndStopParsing.
void SetError(HpackParseResult error) {
- if (!error_.ok() || min_progress_size_ > 0) {
- if (error.connection_error() && !error_.connection_error()) {
- error_ = std::move(error); // connection errors dominate
+ SetErrorFor(frame_error_, error);
+ SetErrorFor(field_error_, std::move(error));
+ }
+
+ void SetErrorFor(HpackParseResult& error, HpackParseResult new_error) {
+ if (!error.ok() || min_progress_size_ > 0) {
+ if (new_error.connection_error() && !error.connection_error()) {
+ error = std::move(new_error); // connection errors dominate
}
return;
}
- error_ = std::move(error);
+ error = std::move(new_error);
}
// Refcount if we are backed by a slice
@@ -320,7 +328,8 @@ class HPackParser::Input {
// Frontier denotes the first byte past successfully processed input
const uint8_t* frontier_;
// Current error
- HpackParseResult& error_;
+ HpackParseResult& frame_error_;
+ HpackParseResult& field_error_;
// If the error was EOF, we flag it here by noting how many more bytes would
// be needed to make progress
size_t min_progress_size_ = 0;
@@ -597,6 +606,7 @@ class HPackParser::Parser {
bool ParseTop() {
GPR_DEBUG_ASSERT(state_.parse_state == ParseState::kTop);
auto cur = *input_->Next();
+ input_->ClearFieldError();
switch (cur >> 4) {
// Literal header not indexed - First byte format: 0000xxxx
// Literal header never indexed - First byte format: 0001xxxx
@@ -702,7 +712,7 @@ class HPackParser::Parser {
break;
}
gpr_log(
- GPR_DEBUG, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type,
+ GPR_INFO, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type,
log_info_.is_client ? "CLI" : "SVR", memento.md.DebugString().c_str(),
memento.parse_status == nullptr
? ""
@@ -951,11 +961,10 @@ class HPackParser::Parser {
state_.string_length)
: String::Parse(input_, state_.is_string_huff_compressed,
state_.string_length);
- HpackParseResult& status = state_.frame_error;
absl::string_view key_string;
if (auto* s = absl::get_if<Slice>(&state_.key)) {
key_string = s->as_string_view();
- if (status.ok()) {
+ if (state_.field_error.ok()) {
auto r = ValidateKey(key_string);
if (r != ValidateMetadataResult::kOk) {
input_->SetErrorAndContinueParsing(
@@ -965,7 +974,7 @@ class HPackParser::Parser {
} else {
const auto* memento = absl::get<const HPackTable::Memento*>(state_.key);
key_string = memento->md.key();
- if (status.ok() && memento->parse_status != nullptr) {
+ if (state_.field_error.ok() && memento->parse_status != nullptr) {
input_->SetErrorAndContinueParsing(*memento->parse_status);
}
}
@@ -992,16 +1001,16 @@ class HPackParser::Parser {
key_string.size() + value.wire_size + hpack_constants::kEntryOverhead;
auto md = grpc_metadata_batch::Parse(
key_string, std::move(value_slice), state_.add_to_table, transport_size,
- [key_string, &status, this](absl::string_view message, const Slice&) {
- if (!status.ok()) return;
+ [key_string, this](absl::string_view message, const Slice&) {
+ if (!state_.field_error.ok()) return;
input_->SetErrorAndContinueParsing(
HpackParseResult::MetadataParseError(key_string));
gpr_log(GPR_ERROR, "Error parsing '%s' metadata: %s",
std::string(key_string).c_str(),
std::string(message).c_str());
});
- HPackTable::Memento memento{std::move(md),
- status.PersistentStreamErrorOrNullptr()};
+ HPackTable::Memento memento{
+ std::move(md), state_.field_error.PersistentStreamErrorOrNullptr()};
input_->UpdateFrontier();
state_.parse_state = ParseState::kTop;
if (state_.add_to_table) {
@@ -1163,13 +1172,13 @@ grpc_error_handle HPackParser::Parse(
std::vector<uint8_t> buffer = std::move(unparsed_bytes_);
return ParseInput(
Input(nullptr, buffer.data(), buffer.data() + buffer.size(), bitsrc,
- state_.frame_error),
+ state_.frame_error, state_.field_error),
is_last, call_tracer);
}
- return ParseInput(
- Input(slice.refcount, GRPC_SLICE_START_PTR(slice),
- GRPC_SLICE_END_PTR(slice), bitsrc, state_.frame_error),
- is_last, call_tracer);
+ return ParseInput(Input(slice.refcount, GRPC_SLICE_START_PTR(slice),
+ GRPC_SLICE_END_PTR(slice), bitsrc, state_.frame_error,
+ state_.field_error),
+ is_last, call_tracer);
}
grpc_error_handle HPackParser::ParseInput(
diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.h b/src/core/ext/transport/chttp2/transport/hpack_parser.h
index 6b28daad61d4b..8f7132092958c 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_parser.h
+++ b/src/core/ext/transport/chttp2/transport/hpack_parser.h
@@ -236,6 +236,8 @@ class HPackParser {
HPackTable hpack_table;
// Error so far for this frame (set by class Input)
HpackParseResult frame_error;
+ // Error so far for this field (set by class Input)
+ HpackParseResult field_error;
// Length of frame so far.
uint32_t frame_length = 0;
// Length of the string being parsed
diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc
index 3772d909b9b8b..d5b9c6cb68da2 100644
--- a/test/core/transport/chttp2/hpack_parser_test.cc
+++ b/test/core/transport/chttp2/hpack_parser_test.cc
@@ -440,19 +440,82 @@ INSTANTIATE_TEST_SUITE_P(
Test{"Base64LegalEncoding",
{},
{},
- {// Binary metadata: created using:
- // tools/codegen/core/gen_header_frame.py
- // --compression inc --no_framing --output hexstr
- // < test/core/transport/chttp2/bad-base64.headers
- {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
- "27732074756573646179",
- absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
- "illegal base64 encoding"),
- 0},
- {"be",
- absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
- "illegal base64 encoding"),
- 0}}},
+ {
+ // Binary metadata: created using:
+ // tools/codegen/core/gen_header_frame.py
+ // --compression inc --no_framing --output hexstr
+ // < test/core/transport/chttp2/bad-base64.headers
+ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
+ "27732074756573646179",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ 0},
+ {"be",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ kEndOfHeaders},
+ {"82", ":method: GET\n", 0},
+ }},
+ Test{"Base64LegalEncodingWorksAfterFailure",
+ {},
+ {},
+ {
+ // Binary metadata: created using:
+ // tools/codegen/core/gen_header_frame.py
+ // --compression inc --no_framing --output hexstr
+ // < test/core/transport/chttp2/bad-base64.headers
+ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
+ "27732074756573646179",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ 0},
+ {"be",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ 0},
+ {"400e636f6e74656e742d6c656e6774680135",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ kEndOfHeaders},
+ {"be", "content-length: 5\n", 0},
+ }},
+ Test{"Base64LegalEncodingWorksAfterFailure2",
+ {},
+ {},
+ {
+ {// Generated with: tools/codegen/core/gen_header_frame.py
+ // --compression inc --output hexstr --no_framing <
+ // test/core/transport/chttp2/MiXeD-CaSe.headers
+ "400a4d695865442d436153651073686f756c64206e6f74207061727365",
+ absl::InternalError("Illegal header key: MiXeD-CaSe"), 0},
+ // Binary metadata: created using:
+ // tools/codegen/core/gen_header_frame.py
+ // --compression inc --no_framing --output hexstr
+ // < test/core/transport/chttp2/bad-base64.headers
+ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
+ "27732074756573646179",
+ absl::InternalError("Illegal header key: MiXeD-CaSe"), 0},
+ {"be", absl::InternalError("Illegal header key: MiXeD-CaSe"),
+ 0},
+ {"400e636f6e74656e742d6c656e6774680135",
+ absl::InternalError("Illegal header key: MiXeD-CaSe"),
+ kEndOfHeaders},
+ {"be", "content-length: 5\n", 0},
+ {"bf",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ 0},
+ // Only the first error in each frame is reported, so we should
+ // still see the same error here...
+ {"c0",
+ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+ "illegal base64 encoding"),
+ kEndOfHeaders},
+ // ... but if we look at the next frame we should see the
+ // stored error
+ {"c0", absl::InternalError("Illegal header key: MiXeD-CaSe"),
+ kEndOfHeaders},
+ }},
Test{"TeIsTrailers",
{},
{},
diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc
index 81809afcf7175..40de14def3d92 100644
--- a/test/core/transport/chttp2/hpack_sync_fuzzer.cc
+++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc
@@ -86,6 +86,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
// Not an interesting case to fuzz
continue;
}
+ if (msg.check_ab_preservation() &&
+ header.literal_inc_idx().key() == "a") {
+ continue;
+ }
if (absl::EndsWith(header.literal_inc_idx().value(), "-bin")) {
std::ignore = encoder.EmitLitHdrWithBinaryStringKeyIncIdx(
Slice::FromCopiedString(header.literal_inc_idx().key()),
@@ -97,6 +101,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
}
break;
case hpack_sync_fuzzer::Header::kLiteralNotIdx:
+ if (msg.check_ab_preservation() &&
+ header.literal_not_idx().key() == "a") {
+ continue;
+ }
if (absl::EndsWith(header.literal_not_idx().value(), "-bin")) {
encoder.EmitLitHdrWithBinaryStringKeyNotIdx(
Slice::FromCopiedString(header.literal_not_idx().key()),
@@ -115,6 +123,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
break;
}
}
+ if (msg.check_ab_preservation()) {
+ std::ignore = encoder.EmitLitHdrWithNonBinaryStringKeyIncIdx(
+ Slice::FromCopiedString("a"), Slice::FromCopiedString("b"));
+ }
// STAGE 2: Decode the buffer (encode_output) into a list of headers
HPackParser parser;
@@ -141,6 +153,21 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
}
}
+ if (seen_errors.empty() && msg.check_ab_preservation()) {
+ std::string backing;
+ auto a_value = read_metadata.GetStringValue("a", &backing);
+ if (!a_value.has_value()) {
+ fprintf(stderr, "Expected 'a' header to be present: %s\n",
+ read_metadata.DebugString().c_str());
+ abort();
+ }
+ if (a_value != "b") {
+ fprintf(stderr, "Expected 'a' header to be 'b', got '%s'\n",
+ std::string(*a_value).c_str());
+ abort();
+ }
+ }
+
// STAGE 3: If we reached here we either had a stream error or no error
// parsing.
// Either way, the hpack tables should be of the same size between client and
@@ -169,6 +196,41 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
}
abort();
}
+
+ if (msg.check_ab_preservation()) {
+ SliceBuffer encode_output_2;
+ hpack_encoder_detail::Encoder encoder_2(
+ &compressor, msg.use_true_binary_metadata(), encode_output_2);
+ encoder_2.EmitIndexed(62);
+ GPR_ASSERT(encode_output_2.Count() == 1);
+ grpc_metadata_batch read_metadata_2(arena.get());
+ parser.BeginFrame(
+ &read_metadata_2, 1024, 1024, HPackParser::Boundary::EndOfHeaders,
+ HPackParser::Priority::None,
+ HPackParser::LogInfo{3, HPackParser::LogInfo::kHeaders, false});
+ auto err = parser.Parse(encode_output_2.c_slice_at(0), true,
+ absl::BitGenRef(proto_bit_src),
+ /*call_tracer=*/nullptr);
+ if (!err.ok()) {
+ fprintf(stderr, "Error parsing preservation encoded data: %s\n",
+ err.ToString().c_str());
+ abort();
+ }
+ std::string backing;
+ auto a_value = read_metadata_2.GetStringValue("a", &backing);
+ if (!a_value.has_value()) {
+ fprintf(stderr,
+ "Expected 'a' header to be present: %s\nfirst metadata: %s\n",
+ read_metadata_2.DebugString().c_str(),
+ read_metadata.DebugString().c_str());
+ abort();
+ }
+ if (a_value != "b") {
+ fprintf(stderr, "Expected 'a' header to be 'b', got '%s'\n",
+ std::string(*a_value).c_str());
+ abort();
+ }
+ }
}
} // namespace
diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.proto b/test/core/transport/chttp2/hpack_sync_fuzzer.proto
index 72792b60d640a..2c075a6abb1a1 100644
--- a/test/core/transport/chttp2/hpack_sync_fuzzer.proto
+++ b/test/core/transport/chttp2/hpack_sync_fuzzer.proto
@@ -44,4 +44,7 @@ message Msg {
repeated Header headers = 2;
grpc.testing.FuzzConfigVars config_vars = 3;
repeated uint64 random_numbers = 4;
+ // Ensure that a header "a: b" appended to headers with hpack incremental
+ // indexing is correctly added to the hpack table.
+ bool check_ab_preservation = 5;
}

10
grpc.changes
View File

@ -1,3 +1,13 @@
-------------------------------------------------------------------
Tue Dec 17 13:58:55 UTC 2024 - pgajdos@suse.com
- security update
- added patches
fix CVE-2024-11407 [bsc#1233821], servers with transmit zero copy enabled through GRPC_ARG_TCP_TX_ZEROCOPY_ENABLED can experience data corruption issues
+ grpc-CVE-2024-11407.patch
fix CVE-2024-7246 [bsc#1228919], gRPC clients communicating with a HTTP/2 proxy can poison the HPACK table between the proxy and the backend
+ grpc-CVE-2024-7246.patch
-------------------------------------------------------------------
Tue Oct 31 05:21:51 UTC 2023 - Jan Engelhardt <jengelh@inai.de>

4
grpc.spec
View File

@ -28,6 +28,10 @@ Group: Development/Tools/Building
URL: https://grpc.io/
Source: https://github.com/grpc/grpc/archive/v%version.tar.gz
Source2: %name-rpmlintrc
# CVE-2024-7246 [bsc#1228919], gRPC clients communicating with a HTTP/2 proxy can poison the HPACK table between the proxy and the backend
Patch0: grpc-CVE-2024-7246.patch
# CVE-2024-11407 [bsc#1233821], servers with transmit zero copy enabled through GRPC_ARG_TCP_TX_ZEROCOPY_ENABLED can experience data corruption issues
Patch1: grpc-CVE-2024-11407.patch
BuildRequires: abseil-cpp-devel
BuildRequires: cmake
BuildRequires: fdupes