mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-04 21:32:15 +00:00
Merge commit from fork
* Add DenyLists parsing * Remove unneeded returned error * Plug global denylist into Command * app creation: apply denylist to backchannel logout URI * Inject denylist to backchannel logout worker * webhook config: validate against blocked URLs * Add notificationsWebhook denylist target * command: Add SMTP endpoint validation against blocklist * command: Add SMS endpoint validation against blocklist * Validate webhook endpoint against denylist on channel notification * Remove unused tests * handle deprecated denylists * remove unintended denylist entry in deprecated list * use single http client * fix tests * update comments * fixes * cleanup * address comments * fix merge --------- Co-authored-by: Livio Spring <[email protected]>
This commit is contained in:
Marco A.
Livio Spring
@@ -0,0 +1,131 @@
|
||||
package http
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/denylist"
|
||||
)
|
||||
|
||||
type ClientConfig struct {
|
||||
MaxBodySize int64
|
||||
Timeout time.Duration
|
||||
MaxRedirects int
|
||||
AllowHTTPSDowngrade bool
|
||||
DenyList []denylist.AddressChecker
|
||||
}
|
||||
|
||||
// NewClient returns a new http.Client with the configured settings.
|
||||
// The client is protected against DNS rebinding attacks, redirects, HTTPs downgrades, and response body size limits.
|
||||
func (c *ClientConfig) NewClient() *http.Client {
|
||||
baseTransport := denylist.NewHTTPTransport(c.DenyList)
|
||||
|
||||
protectedTransport := &MaxBytesRoundTripper{
|
||||
Underlying: baseTransport,
|
||||
MaxBytes: c.MaxBodySize,
|
||||
}
|
||||
|
||||
return &http.Client{
|
||||
Transport: protectedTransport,
|
||||
Timeout: c.Timeout,
|
||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||
if len(via) >= c.MaxRedirects {
|
||||
return ErrTooManyRedirects
|
||||
}
|
||||
if !c.AllowHTTPSDowngrade && len(via) > 0 {
|
||||
prev := via[len(via)-1]
|
||||
if prev != nil && prev.URL != nil && req.URL != nil &&
|
||||
strings.EqualFold(prev.URL.Scheme, "https") &&
|
||||
!strings.EqualFold(req.URL.Scheme, "https") {
|
||||
return ErrHTTPsDowngrade
|
||||
}
|
||||
}
|
||||
return denylist.IsURLBlocked(c.DenyList, req.URL, nil)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// MergeDeprecatedDenylists merges the two deprecated (actions) denylists into the main denylist.
|
||||
func (c *ClientConfig) MergeDeprecatedDenylists(actionsV1, actionsV2 []denylist.AddressChecker) {
|
||||
c.DenyList = append(c.DenyList, actionsV1...)
|
||||
c.DenyList = append(c.DenyList, actionsV2...)
|
||||
}
|
||||
|
||||
var (
|
||||
// ErrResponseTooLarge is returned when the response body exceeds the configured limit.
|
||||
ErrResponseTooLarge = errors.New("response body exceeded maximum allowed size")
|
||||
//ErrTooManyRedirects is returned when the number of redirects exceeds the configured limit.
|
||||
ErrTooManyRedirects = errors.New("stopped after too many redirects")
|
||||
// ErrHTTPsDowngrade is returned when the client attempts to downgrade to HTTP.
|
||||
ErrHTTPsDowngrade = errors.New("redirect downgrade from https to http is not allowed")
|
||||
)
|
||||
|
||||
// MaxBytesRoundTripper wraps an existing RoundTripper to protect against OOM.
|
||||
type MaxBytesRoundTripper struct {
|
||||
Underlying http.RoundTripper
|
||||
MaxBytes int64
|
||||
}
|
||||
|
||||
func (m *MaxBytesRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
transport := m.Underlying
|
||||
if transport == nil {
|
||||
transport = http.DefaultTransport
|
||||
}
|
||||
|
||||
resp, err := transport.RoundTrip(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if resp.ContentLength > m.MaxBytes {
|
||||
resp.Body.Close()
|
||||
return nil, fmt.Errorf("%w: Content-Length is %d (limit %d)", ErrResponseTooLarge, resp.ContentLength, m.MaxBytes)
|
||||
}
|
||||
|
||||
resp.Body = &strictMaxBytesReader{
|
||||
limitReader: io.LimitReader(resp.Body, m.MaxBytes+1), // We initialize with +1 to detect overflows during Read
|
||||
closer: resp.Body,
|
||||
limit: m.MaxBytes,
|
||||
}
|
||||
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
var _ http.RoundTripper = (*MaxBytesRoundTripper)(nil)
|
||||
|
||||
// strictMaxBytesReader enforces a hard limit and returns an explicit error if exceeded.
|
||||
type strictMaxBytesReader struct {
|
||||
limitReader io.Reader
|
||||
closer io.Closer
|
||||
limit int64
|
||||
bytesRead int64
|
||||
}
|
||||
|
||||
func (s *strictMaxBytesReader) Read(p []byte) (int, error) {
|
||||
n, err := s.limitReader.Read(p)
|
||||
s.bytesRead += int64(n)
|
||||
|
||||
if s.bytesRead > s.limit {
|
||||
// Because LimitReader stops at limit+1, excess is mathematically guaranteed to be 1
|
||||
safeN := n - 1
|
||||
if safeN < 0 {
|
||||
safeN = 0
|
||||
}
|
||||
|
||||
// This instantly slices off that single offending byte from the view of the caller
|
||||
p = p[:safeN]
|
||||
return safeN, ErrResponseTooLarge
|
||||
}
|
||||
|
||||
return n, err
|
||||
}
|
||||
|
||||
func (s *strictMaxBytesReader) Close() error {
|
||||
return s.closer.Close()
|
||||
}
|
||||
|
||||
var _ io.ReadCloser = (*strictMaxBytesReader)(nil)
|
||||
@@ -0,0 +1,144 @@
|
||||
package http
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/denylist"
|
||||
)
|
||||
|
||||
func TestClientConfig_NewClient_Allowed(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Spin up a simple local test server to serve a dummy response
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte("hello world"))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
// An empty denylist permits all safe destinations
|
||||
cfg := &ClientConfig{
|
||||
MaxBodySize: 1024,
|
||||
Timeout: 2 * time.Second,
|
||||
MaxRedirects: 3,
|
||||
DenyList: []denylist.AddressChecker{},
|
||||
}
|
||||
|
||||
client := cfg.NewClient()
|
||||
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil)
|
||||
assert.NoError(t, err)
|
||||
|
||||
resp, err := client.Do(req)
|
||||
assert.NoError(t, err)
|
||||
if resp != nil {
|
||||
defer resp.Body.Close()
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "hello world", string(body))
|
||||
}
|
||||
}
|
||||
|
||||
func TestClientConfig_Redirect_HTTPSDowngradeBlocked(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cfg := &ClientConfig{
|
||||
MaxBodySize: 1024,
|
||||
Timeout: 2 * time.Second,
|
||||
MaxRedirects: 3,
|
||||
AllowHTTPSDowngrade: false, // Explicitly disallow downgrades
|
||||
DenyList: []denylist.AddressChecker{},
|
||||
}
|
||||
|
||||
client := cfg.NewClient()
|
||||
|
||||
// Mock a historical request chain moving from a secure schema to an unencrypted target
|
||||
viaReq, _ := http.NewRequest(http.MethodGet, "https://secure-identity.com/oauth", nil)
|
||||
currentReq, _ := http.NewRequest(http.MethodGet, "http://insecure-identity.com/callback", nil)
|
||||
|
||||
err := client.CheckRedirect(currentReq, []*http.Request{viaReq})
|
||||
assert.ErrorIs(t, err, ErrHTTPsDowngrade)
|
||||
}
|
||||
|
||||
func TestMaxBytesRoundTripper_EnforcesCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Spin up a local server delivering an exact 10-byte payload response string
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write([]byte("0123456789"))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
maxBytes int64
|
||||
expectError error
|
||||
}{
|
||||
{
|
||||
name: "body cleanly within threshold limit",
|
||||
maxBytes: 15,
|
||||
expectError: nil,
|
||||
},
|
||||
{
|
||||
name: "body matches boundary size exactly",
|
||||
maxBytes: 10,
|
||||
expectError: nil,
|
||||
},
|
||||
{
|
||||
name: "body exceeds allowable limit",
|
||||
maxBytes: 5,
|
||||
expectError: ErrResponseTooLarge,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
cfg := &ClientConfig{
|
||||
MaxBodySize: tt.maxBytes,
|
||||
Timeout: 2 * time.Second,
|
||||
MaxRedirects: 3,
|
||||
DenyList: []denylist.AddressChecker{},
|
||||
}
|
||||
|
||||
client := cfg.NewClient()
|
||||
req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil)
|
||||
resp, err := client.Do(req)
|
||||
|
||||
if tt.expectError != nil {
|
||||
// Depending on optimization, errors can trip directly on call or during chunk reading
|
||||
if err != nil {
|
||||
assert.ErrorIs(t, err, tt.expectError)
|
||||
return
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
_, readErr := io.ReadAll(resp.Body)
|
||||
assert.ErrorIs(t, readErr, tt.expectError)
|
||||
} else {
|
||||
assert.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
body, readErr := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, readErr)
|
||||
assert.Equal(t, 10, len(body))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClientConfig_MergeDeprecatedDenylists(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cfg := &ClientConfig{DenyList: []denylist.AddressChecker{denylist.NewHostChecker("1.1.1.1")}}
|
||||
|
||||
v1 := []denylist.AddressChecker{denylist.NewHostChecker("2.2.2.2")}
|
||||
v2 := []denylist.AddressChecker{denylist.NewHostChecker("3.3.3.3")}
|
||||
|
||||
cfg.MergeDeprecatedDenylists(v1, v2)
|
||||
|
||||
assert.Len(t, cfg.DenyList, 3)
|
||||
}
|
||||
Reference in New Issue
Block a user