Skip to content

Commit 35af0e6

Browse files
authored
Merge branch 'release/1.21.x' into backport/sanikachavan5/upgrade-golang-to-latest-patch/poorly-pleasing-lion
2 parents e6ad556 + ff8358e commit 35af0e6

File tree

12 files changed

+2511
-3653
lines changed

12 files changed

+2511
-3653
lines changed

.changelog/22836.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:security
2+
security: adding a maximum Content-Length on the event endpoint to fix denial-of-service (DoS) attacks. This resolves [CVE-2025-11375](https://nvd.nist.gov/vuln/detail/CVE-2025-11375).
3+
```

.changelog/22916.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:security
2+
security: Improved validation of the Content-Length header in the Consul KV endpoint to prevent potential denial of service attacks[CVE-2025-11374]()
3+
```

agent/event_endpoint.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package agent
55

66
import (
77
"bytes"
8+
"fmt"
89
"io"
910
"net/http"
1011
"strconv"
@@ -44,12 +45,29 @@ func (s *HTTPHandlers) EventFire(resp http.ResponseWriter, req *http.Request) (i
4445
}
4546

4647
// Get the payload
47-
if req.ContentLength > 0 {
48+
if req.ContentLength >= 0 {
49+
// The underlying gossip sets limits on the size of a user event
50+
// message. It is hard to give an exact number, as it depends on various
51+
// parameters of the event, but the payload should be kept very small
52+
// (< 100 bytes). We've multiplied this by 3 to be safe.
53+
const maxEventPayloadSize = 300
54+
if req.ContentLength > maxEventPayloadSize {
55+
return nil, HTTPError{
56+
StatusCode: http.StatusRequestEntityTooLarge,
57+
Reason: fmt.Sprintf("Event payload too large, received %d bytes, max size: %d bytes. User events should be kept small for efficient gossip propagation.",
58+
req.ContentLength, maxEventPayloadSize),
59+
}
60+
}
61+
4862
var buf bytes.Buffer
49-
if _, err := io.Copy(&buf, req.Body); err != nil {
50-
return nil, err
63+
if req.Body != nil {
64+
if _, err := io.Copy(&buf, req.Body); err != nil {
65+
return nil, err
66+
}
67+
event.Payload = buf.Bytes()
5168
}
52-
event.Payload = buf.Bytes()
69+
} else {
70+
return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Event payload size must be greater than zero"}
5371
}
5472

5573
// Try to fire the event

agent/event_endpoint_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,125 @@ func TestEventList_EventBufOrder(t *testing.T) {
379379
})
380380
}
381381

382+
func TestEventFire_PayloadSizeLimit(t *testing.T) {
383+
if testing.Short() {
384+
t.Skip("too slow for testing.Short")
385+
}
386+
387+
t.Parallel()
388+
a := NewTestAgent(t, "")
389+
defer a.Shutdown()
390+
testrpc.WaitForTestAgent(t, a.RPC, "dc1")
391+
392+
const maxPayloadSize = 300
393+
394+
type expectedResponse struct {
395+
success bool
396+
statusCode int
397+
errorMessage string
398+
eventName string
399+
payloadSize int
400+
}
401+
402+
testCases := []struct {
403+
name string
404+
payloadSize int
405+
expectedResponse *expectedResponse
406+
description string
407+
}{
408+
{
409+
name: "empty payload",
410+
payloadSize: 0,
411+
expectedResponse: &expectedResponse{
412+
success: true,
413+
eventName: "test",
414+
payloadSize: 0,
415+
},
416+
description: "empty payload should be accepted",
417+
},
418+
{
419+
name: "payload within limit",
420+
payloadSize: 50,
421+
expectedResponse: &expectedResponse{
422+
success: true,
423+
eventName: "test",
424+
payloadSize: 50,
425+
},
426+
description: "small payload should be accepted",
427+
},
428+
{
429+
name: "payload at exact limit",
430+
payloadSize: maxPayloadSize,
431+
expectedResponse: &expectedResponse{
432+
success: true,
433+
eventName: "test",
434+
payloadSize: maxPayloadSize,
435+
},
436+
description: "payload at exactly 300 bytes should be accepted",
437+
},
438+
{
439+
name: "payload exceeds limit by 1 byte",
440+
payloadSize: maxPayloadSize + 1,
441+
expectedResponse: &expectedResponse{
442+
statusCode: http.StatusRequestEntityTooLarge,
443+
errorMessage: "Event payload too large",
444+
},
445+
description: "payload exceeding limit should be rejected",
446+
},
447+
{
448+
name: "large payload",
449+
payloadSize: 500,
450+
expectedResponse: &expectedResponse{
451+
statusCode: http.StatusRequestEntityTooLarge,
452+
errorMessage: "Event payload too large",
453+
},
454+
description: "large payload should be rejected",
455+
},
456+
}
457+
458+
for _, tc := range testCases {
459+
t.Run(tc.name, func(t *testing.T) {
460+
var payload []byte
461+
if tc.payloadSize <= 0 {
462+
payload = []byte{}
463+
} else {
464+
payload = bytes.Repeat([]byte("x"), tc.payloadSize)
465+
}
466+
467+
url := "/v1/event/fire/test"
468+
req, err := http.NewRequest("PUT", url, bytes.NewBuffer(payload))
469+
require.NoError(t, err)
470+
471+
resp := httptest.NewRecorder()
472+
obj, err := a.srv.EventFire(resp, req)
473+
474+
if tc.expectedResponse.success {
475+
require.NoError(t, err, tc.description)
476+
require.NotNil(t, obj, "Should return event object on success")
477+
478+
event, ok := obj.(*UserEvent)
479+
require.True(t, ok, "Expected *UserEvent, got %T", obj)
480+
require.Equal(t, tc.expectedResponse.eventName, event.Name)
481+
482+
if tc.expectedResponse.payloadSize == 0 {
483+
// Empty payload should result in nil
484+
require.Nil(t, event.Payload)
485+
} else {
486+
expectedPayload := bytes.Repeat([]byte("x"), tc.expectedResponse.payloadSize)
487+
require.Equal(t, expectedPayload, event.Payload)
488+
}
489+
} else {
490+
require.Error(t, err, tc.description)
491+
httpErr, ok := err.(HTTPError)
492+
require.True(t, ok, "Expected HTTPError, got %T", err)
493+
require.Equal(t, tc.expectedResponse.statusCode, httpErr.StatusCode)
494+
require.Contains(t, httpErr.Reason, tc.expectedResponse.errorMessage)
495+
require.Nil(t, obj, "Should not return event object on error")
496+
}
497+
})
498+
}
499+
}
500+
382501
func TestUUIDToUint64(t *testing.T) {
383502
t.Parallel()
384503
inp := "cb9a81ad-fff6-52ac-92a7-5f70687805ec"

agent/kvs_endpoint.go

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package agent
55

66
import (
77
"bytes"
8+
"errors"
89
"fmt"
910
"io"
1011
"net/http"
@@ -238,19 +239,45 @@ func (s *HTTPHandlers) KVSPut(resp http.ResponseWriter, req *http.Request, args
238239
}
239240

240241
// Check the content-length
241-
if req.ContentLength > int64(s.agent.config.KVMaxValueSize) {
242+
maxSize := int64(s.agent.config.KVMaxValueSize)
243+
var buf *bytes.Buffer
244+
245+
switch {
246+
case req.ContentLength <= 0 && req.Body == nil:
247+
return "Request has no content-length & no body", nil
248+
249+
case req.ContentLength <= 0 && req.Body != nil:
250+
// LimitReader to limit copy of large requests with no Content-Length
251+
//+1 ensures we can detect if the body is too large
252+
byteReader := http.MaxBytesReader(nil, req.Body, maxSize+1)
253+
buf = new(bytes.Buffer)
254+
if _, err := io.Copy(buf, byteReader); err != nil {
255+
var bodyTooLargeErr *http.MaxBytesError
256+
if errors.As(err, &bodyTooLargeErr) {
257+
return nil, HTTPError{
258+
StatusCode: http.StatusRequestEntityTooLarge,
259+
Reason: fmt.Sprintf("Request body too large. Max allowed is %d bytes.", maxSize),
260+
}
261+
}
262+
return nil, err
263+
}
264+
265+
case req.ContentLength > maxSize:
266+
// Throw error if Content-Length is greater than max size
242267
return nil, HTTPError{
243268
StatusCode: http.StatusRequestEntityTooLarge,
244269
Reason: fmt.Sprintf("Request body(%d bytes) too large, max size: %d bytes. See %s.",
245-
req.ContentLength, s.agent.config.KVMaxValueSize, "https://developer.hashicorp.com/docs/agent/config/config-files#kv_max_value_size"),
270+
req.ContentLength, maxSize, "https://developer.hashicorp.com/docs/agent/config/config-files#kv_max_value_size"),
246271
}
247-
}
248272

249-
// Copy the value
250-
buf := bytes.NewBuffer(nil)
251-
if _, err := io.Copy(buf, req.Body); err != nil {
252-
return nil, err
273+
default:
274+
// Copy the value
275+
buf = bytes.NewBuffer(nil)
276+
if _, err := io.Copy(buf, req.Body); err != nil {
277+
return nil, err
278+
}
253279
}
280+
254281
applyReq.DirEnt.Value = buf.Bytes()
255282

256283
// Make the RPC

agent/kvs_endpoint_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package agent
66
import (
77
"bytes"
88
"fmt"
9+
"io"
910
"net/http"
1011
"net/http/httptest"
1112
"path"
@@ -37,6 +38,7 @@ func TestKVSEndpoint_PUT_GET_DELETE(t *testing.T) {
3738
for _, key := range keys {
3839
buf := bytes.NewBuffer([]byte("test"))
3940
req, _ := http.NewRequest("PUT", "/v1/kv/"+key, buf)
41+
req.ContentLength = int64(buf.Len())
4042
resp := httptest.NewRecorder()
4143
obj, err := a.srv.KVSEndpoint(resp, req)
4244
if err != nil {
@@ -554,6 +556,96 @@ func TestKVSEndpoint_GET(t *testing.T) {
554556
}
555557
}
556558

559+
func TestKVSPUT_SwitchCases(t *testing.T) {
560+
if testing.Short() {
561+
t.Skip("skipping test in short mode")
562+
}
563+
564+
t.Parallel()
565+
a := NewTestAgent(t, "")
566+
defer a.Shutdown()
567+
568+
maxSize := int(a.srv.agent.config.KVMaxValueSize)
569+
570+
tests := []struct {
571+
name string
572+
body string
573+
contentLength int64
574+
expectErr bool
575+
expectMsg string
576+
expectHTTPMsg string
577+
}{
578+
{
579+
name: "Case 2: No Content-Length but Body exists (allowed size)",
580+
body: "small-value",
581+
contentLength: 0,
582+
expectErr: false,
583+
},
584+
{
585+
name: "Case 2b: No Content-Length but Body exists (too large)",
586+
body: strings.Repeat("x", maxSize+50),
587+
contentLength: 0,
588+
expectErr: true,
589+
expectHTTPMsg: fmt.Sprintf("Request body too large. Max allowed is %d bytes.", maxSize),
590+
},
591+
{
592+
name: "Case 3: Content-Length greater than max allowed limit",
593+
body: strings.Repeat("x", maxSize+10),
594+
contentLength: int64(maxSize) + 10,
595+
expectErr: true,
596+
expectHTTPMsg: fmt.Sprintf("Request body(%d bytes) too large, max size: %d bytes.", int64(maxSize)+10, maxSize),
597+
},
598+
{
599+
name: "Case 4: Normal body within allowed limit",
600+
body: "tiny",
601+
contentLength: 4,
602+
expectErr: false,
603+
},
604+
}
605+
606+
for _, tt := range tests {
607+
t.Run(tt.name, func(t *testing.T) {
608+
var bodyReader io.Reader
609+
if tt.body != "" {
610+
bodyReader = bytes.NewBufferString(tt.body)
611+
} else {
612+
bodyReader = nil
613+
}
614+
615+
req := httptest.NewRequest(http.MethodPut, "/v1/kv/switch-test", bodyReader)
616+
req.ContentLength = tt.contentLength
617+
resp := httptest.NewRecorder()
618+
619+
obj, err := a.srv.KVSEndpoint(resp, req)
620+
621+
// Expected error cases
622+
if tt.expectErr {
623+
if err == nil {
624+
t.Fatalf("expected error, got nil")
625+
}
626+
httpErr, ok := err.(HTTPError)
627+
if !ok {
628+
t.Fatalf("expected HTTPError, got %T", err)
629+
}
630+
if !strings.Contains(httpErr.Reason, tt.expectHTTPMsg[:20]) { // partial match
631+
t.Fatalf("expected HTTPError reason to contain %q, got %q", tt.expectHTTPMsg, httpErr.Reason)
632+
}
633+
return
634+
}
635+
636+
// Unexpected error
637+
if err != nil {
638+
t.Fatalf("unexpected error: %v", err)
639+
}
640+
641+
// Normal successful PUT result
642+
if res, ok := obj.(bool); !ok || !res {
643+
t.Fatalf("expected successful PUT result, got %v", obj)
644+
}
645+
})
646+
}
647+
}
648+
557649
func TestKVSEndpoint_DELETE_ConflictingFlags(t *testing.T) {
558650
if testing.Short() {
559651
t.Skip("too slow for testing.Short")

ui/packages/consul-ui/app/mixins/with-blocking-actions.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { singularize } from 'ember-inflector';
2424
*
2525
*/
2626
export default Mixin.create({
27+
router: service('router'),
2728
_feedback: service('feedback'),
2829
settings: service('settings'),
2930
init: function () {
@@ -50,7 +51,7 @@ export default Mixin.create({
5051
// e.g. index or edit
5152
parts.pop();
5253
// e.g. dc.intentions, essentially go to the listings page
53-
return this.transitionTo(parts.join('.'));
54+
return this.router.transitionTo(parts.join('.'));
5455
},
5556
afterDelete: function (item) {
5657
// e.g. dc.intentions.index
@@ -63,7 +64,7 @@ export default Mixin.create({
6364
return this.refresh();
6465
default:
6566
// e.g. dc.intentions essentially do to the listings page
66-
return this.transitionTo(parts.join('.'));
67+
return this.router.transitionTo(parts.join('.'));
6768
}
6869
},
6970
errorCreate: function (type, e) {

ui/packages/consul-ui/app/routes/dc/kv/folder.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,16 @@
44
*/
55

66
import Route from './index';
7+
import { inject as service } from '@ember/service';
78

89
export default class FolderRoute extends Route {
10+
@service router;
11+
912
beforeModel(transition) {
1013
super.beforeModel(...arguments);
1114
const params = this.paramsFor('dc.kv.folder');
1215
if (params.key === '/' || params.key == null) {
13-
return this.transitionTo('dc.kv.index');
16+
return this.router.transitionTo('dc.kv.index');
1417
}
1518
}
1619
}

0 commit comments

Comments
 (0)