mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
Merge commit from fork
* fix(share): always assign the authenticated user as share owner A share's UserID was taken from the request body and only defaulted when empty, so any authenticated user could create a share attributed to another user. For playlist shares the contents are resolved in the owner's library-access context, turning the spoofed owner into an access-escalation vector in multi-library setups. Force the owner from the request context at both the service boundary and the persistence layer, ignoring any client-supplied UserID. * fix(plugins): block SSRF to private IPs resolved from hostnames The HTTP host client only checked the literal host string, so a symbolic hostname (or a trailing-dot "localhost.") resolving to a private/loopback address bypassed the SSRF guard when a plugin declared no requiredHosts. Enforce the check at dial time via net.Dialer.Control on the resolved IP, which also covers redirect hops and DNS rebinding. When an explicit requiredHosts allowlist is set, defer to it as the operator's trust decision. * fix(plugins): gate private IPs on explicit IP/CIDR allowlist entries Following review feedback: an allowlisted hostname authorizes the external service, not whatever private IP it may resolve or rebind to. Enforce the resolved-IP guard even when requiredHosts is set, permitting a private address only when a literal IP or CIDR entry explicitly covers it. This keeps "reach this external API" and "reach my internal network" as two separate, explicit operator decisions. * fix(plugins): treat unspecified addresses as private in the SSRF guard Dialing 0.0.0.0 or :: reaches the local host, so they bypassed the private/loopback check. * fix(plugins): let a bare "*" allowlist reach private addresses Plugins such as AudioMuse-AI declare requiredHosts ["*"] to reach a user-configured service on the LAN, whose address the manifest cannot know. Requiring a literal IP/CIDR entry broke them. Named hosts and subdomain wildcards still cannot resolve to private addresses. * refactor(plugins): simplify the SSRF-guarded HTTP client and release its pool Build the client directly around the guarded transport instead of replacing a throwaway one, fail closed on an unparseable dial address, and close the per-plugin transport's idle connections when the plugin unloads. Trim stale comments. * fix(plugins): stop enabling extism's unguarded http_request host function Passing requiredHosts as the extism manifest's AllowedHosts enabled extism's own http_request (pdk.NewHTTPRequest), which only glob-matches the hostname and follows redirects without re-checking, bypassing the resolved-IP SSRF guard. Plugins must use host.HTTPSend. * fix(plugins): move bundled Rust examples to the host HTTP service Extism's built-in http_request is now disabled, so the webhook and Discord examples switch to nd_pdk::host::http::send. Update the README to say host.HTTPSend is the only supported way to make HTTP requests. * fix(plugins): move the Python example to the host HTTP service coverartarchive-py used extism's built-in Http.request, which is now disabled. Call Navidrome's http_send host function instead. The plugin can no longer run under the standalone extism CLI, so drop the CLI test targets and instructions.
This commit is contained in:
Deluan Quintão
GitHub
parent
4168377b65
commit
1a8463f7de
14 changed files with 238 additions and 81 deletions
|
|
@ -12,6 +12,7 @@ import (
|
|||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
. "github.com/navidrome/navidrome/utils/gg"
|
||||
"github.com/navidrome/navidrome/utils/nanoid"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
|
|
@ -91,6 +92,11 @@ func (r *shareRepositoryWrapper) newId() (string, error) {
|
|||
|
||||
func (r *shareRepositoryWrapper) Save(entity any) (string, error) {
|
||||
s := entity.(*model.Share)
|
||||
// Owner is always the caller; never trust a client-supplied UserID, as it
|
||||
// determines the library-access context used to resolve the share contents.
|
||||
if user, ok := request.UserFrom(r.ctx); ok {
|
||||
s.UserID = user.ID
|
||||
}
|
||||
id, err := r.newId()
|
||||
if err != nil {
|
||||
return "", err
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
|
||||
"github.com/deluan/rest"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
|
@ -39,6 +40,15 @@ var _ = Describe("Share", func() {
|
|||
Expect(entity.ID).To(Equal(id))
|
||||
})
|
||||
|
||||
It("assigns the logged-in user as owner, ignoring a client-supplied UserID", func() {
|
||||
loggedInCtx := request.WithUser(context.Background(), model.User{ID: "logged-in-user"})
|
||||
repo := share.NewRepository(loggedInCtx).(rest.Persistable)
|
||||
entity := &model.Share{Description: "test", ResourceIDs: "123", UserID: "victim-user"}
|
||||
_, err := repo.Save(entity)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(entity.UserID).To(Equal("logged-in-user"))
|
||||
})
|
||||
|
||||
It("does not truncate ASCII labels shorter than 30 characters", func() {
|
||||
_ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "456", Title: "Example Media File"})
|
||||
entity := &model.Share{Description: "test", ResourceIDs: "456"}
|
||||
|
|
|
|||
|
|
@ -170,8 +170,10 @@ func (r *shareRepository) Update(id string, entity any, cols ...string) error {
|
|||
func (r *shareRepository) Save(entity any) (string, error) {
|
||||
s := entity.(*model.Share)
|
||||
// TODO Validate record
|
||||
// Owner is server-managed: for an authenticated request, never trust a
|
||||
// client-supplied UserID, as it drives the share's library-access context.
|
||||
u := loggedUser(r.ctx)
|
||||
if s.UserID == "" {
|
||||
if u.ID != invalidUserId || s.UserID == "" {
|
||||
s.UserID = u.ID
|
||||
}
|
||||
s.CreatedAt = time.Now()
|
||||
|
|
|
|||
|
|
@ -393,6 +393,28 @@ var _ = Describe("ShareRepository", func() {
|
|||
})
|
||||
})
|
||||
|
||||
Describe("Save", func() {
|
||||
It("assigns the logged-in user as owner, ignoring a client-supplied UserID", func() {
|
||||
ur := NewUserRepository(ctx, GetDBXBuilder())
|
||||
Expect(ur.Put(&ownerUser)).To(Succeed())
|
||||
Expect(ur.Put(&otherUser)).To(Succeed())
|
||||
|
||||
attackerCtx := request.WithUser(log.NewContext(GinkgoT().Context()), ownerUser)
|
||||
attackerRepo := NewShareRepository(attackerCtx, GetDBXBuilder()).(rest.Persistable)
|
||||
|
||||
id, err := attackerRepo.Save(&model.Share{
|
||||
ID: "spoof-save-share", UserID: otherUser.ID,
|
||||
ResourceType: "media_file", ResourceIDs: "1001",
|
||||
})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
adminRepo := NewShareRepository(request.WithUser(log.NewContext(GinkgoT().Context()), adminUser), GetDBXBuilder())
|
||||
got, err := adminRepo.Get(id)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(got.UserID).To(Equal(ownerUser.ID))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Update", func() {
|
||||
It("allows a non-admin user to update their own share", func() {
|
||||
insertShare("own-share-upd", ownerUser.ID)
|
||||
|
|
|
|||
|
|
@ -401,7 +401,7 @@ import "github.com/navidrome/navidrome/plugins/pdk/go/host"
|
|||
|
||||
### HTTP
|
||||
|
||||
Make HTTP requests to external services. This is a dedicated host service (separate from Extism's built-in HTTP support) with additional features like timeouts and redirect control.
|
||||
Make HTTP requests to external services, with timeouts, redirect control, and protection against reaching private network addresses. This is the only supported way to make HTTP requests: Extism's built-in HTTP (`pdk.NewHTTPRequest`) is disabled.
|
||||
|
||||
**Manifest permission:**
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
# Build the Cover Art Archive Python plugin
|
||||
.PHONY: build test clean
|
||||
.PHONY: build clean
|
||||
|
||||
WASM_FILE = coverartarchive-py.wasm
|
||||
|
||||
|
|
@ -8,20 +8,5 @@ build: $(WASM_FILE)
|
|||
$(WASM_FILE): plugin/__init__.py
|
||||
extism-py plugin/__init__.py -o $(WASM_FILE)
|
||||
|
||||
test: build
|
||||
@echo "Testing nd_manifest..."
|
||||
extism call $(WASM_FILE) nd_manifest --wasi
|
||||
@echo ""
|
||||
@echo "Testing nd_get_album_images with Portishead's Dummy MBID..."
|
||||
extism call $(WASM_FILE) nd_get_album_images --wasi \
|
||||
--input '{"name":"Dummy","artist":"Portishead","mbid":"76df3287-6cda-33eb-8e9a-044b5e15ffdd"}' \
|
||||
--allow-host "coverartarchive.org" --allow-host "archive.org"
|
||||
|
||||
test-error: build
|
||||
@echo "Testing error case (missing MBID)..."
|
||||
-extism call $(WASM_FILE) nd_get_album_images --wasi \
|
||||
--input '{"name":"Test Album","artist":"Test Artist"}' \
|
||||
--allow-host "coverartarchive.org"
|
||||
|
||||
clean:
|
||||
rm -f $(WASM_FILE)
|
||||
|
|
|
|||
|
|
@ -51,14 +51,7 @@ zip -j coverartarchive-py.ndp manifest.json plugin.wasm
|
|||
|
||||
## Testing
|
||||
|
||||
Extract the wasm file and test:
|
||||
|
||||
```bash
|
||||
unzip -p coverartarchive-py.ndp plugin.wasm > coverartarchive-py.wasm
|
||||
extism call coverartarchive-py.wasm nd_get_album_images --wasi \
|
||||
--input '{"name":"Dummy","artist":"Portishead","mbid":"76df3287-6cda-33eb-8e9a-044b5e15ffdd"}' \
|
||||
--allow-host "coverartarchive.org" --allow-host "archive.org"
|
||||
```
|
||||
The plugin makes HTTP requests through Navidrome's `http_send` host function, so it only runs inside Navidrome (the `extism` CLI can't provide that function). Install the `.ndp` as described above, then open an album that has a MusicBrainz ID and check the Navidrome logs.
|
||||
|
||||
## How It Works
|
||||
|
||||
|
|
|
|||
|
|
@ -5,16 +5,25 @@
|
|||
#
|
||||
# Build with:
|
||||
# extism-py plugin/__init__.py -o coverartarchive-py.wasm
|
||||
#
|
||||
# Test with:
|
||||
# extism call coverartarchive-py.wasm nd_get_album_images --wasi \
|
||||
# --input '{"name":"Dummy","artist":"Portishead","mbid":"76df3287-6cda-33eb-8e9a-044b5e15ffdd"}' \
|
||||
# --allow-host "coverartarchive.org" --allow-host "archive.org"
|
||||
|
||||
import base64
|
||||
import extism
|
||||
import json
|
||||
|
||||
|
||||
@extism.import_fn("extism:host/user", "http_send")
|
||||
def http_send(req: dict) -> dict: ...
|
||||
|
||||
|
||||
def http_get(url):
|
||||
"""GET url via Navidrome's HTTP host service. Returns (status_code, body_bytes)."""
|
||||
resp = http_send({"request": {"method": "GET", "url": url}})
|
||||
if resp.get("error"):
|
||||
raise Exception(f"HTTP request failed: {resp['error']}")
|
||||
result = resp.get("result") or {}
|
||||
return result.get("statusCode", 0), base64.b64decode(result.get("body") or "")
|
||||
|
||||
|
||||
@extism.plugin_fn
|
||||
def nd_get_album_images():
|
||||
"""Retrieve album cover images from Cover Art Archive."""
|
||||
|
|
@ -26,13 +35,13 @@ def nd_get_album_images():
|
|||
|
||||
# Query Cover Art Archive API
|
||||
url = f"https://coverartarchive.org/release/{mbid}"
|
||||
response = extism.Http.request(url, meth="GET")
|
||||
|
||||
if response.status_code != 200:
|
||||
raise Exception(f"not found: CAA returned status {response.status_code}")
|
||||
|
||||
status, body = http_get(url)
|
||||
|
||||
if status != 200:
|
||||
raise Exception(f"not found: CAA returned status {status}")
|
||||
|
||||
try:
|
||||
data = json.loads(response.data_str())
|
||||
data = json.loads(body)
|
||||
except json.JSONDecodeError:
|
||||
raise Exception("not found: invalid JSON response")
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@
|
|||
//! presence updates, and heartbeat management.
|
||||
|
||||
use extism_pdk::*;
|
||||
use nd_pdk::host::{cache, scheduler, websocket};
|
||||
use nd_pdk::host::{cache, http, scheduler, websocket};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
// ============================================================================
|
||||
|
|
@ -359,19 +359,33 @@ fn find_username_for_connection(connection_id: &str) -> Result<Option<String>, E
|
|||
Ok(cache::get_string(&reverse_key)?.filter(|s| !s.is_empty()))
|
||||
}
|
||||
|
||||
fn get_discord_gateway() -> Result<String, Error> {
|
||||
let req = HttpRequest::new("https://discord.com/api/gateway")
|
||||
.with_method("GET");
|
||||
fn send_http(
|
||||
method: &str,
|
||||
url: &str,
|
||||
headers: std::collections::HashMap<String, String>,
|
||||
body: Vec<u8>,
|
||||
) -> Result<http::HTTPResponse, Error> {
|
||||
http::send(http::HTTPRequest {
|
||||
method: method.into(),
|
||||
url: url.into(),
|
||||
headers,
|
||||
no_follow_redirects: false,
|
||||
body,
|
||||
timeout_ms: 0,
|
||||
})?
|
||||
.ok_or_else(|| Error::msg("empty HTTP response"))
|
||||
}
|
||||
|
||||
let resp = http::request::<String>(&req, None::<String>)?;
|
||||
if resp.status_code() >= 400 {
|
||||
fn get_discord_gateway() -> Result<String, Error> {
|
||||
let resp = send_http("GET", "https://discord.com/api/gateway", Default::default(), Vec::new())?;
|
||||
if resp.status_code >= 400 {
|
||||
return Err(Error::msg(format!(
|
||||
"Failed to get Discord gateway: HTTP {}",
|
||||
resp.status_code()
|
||||
resp.status_code
|
||||
)));
|
||||
}
|
||||
|
||||
let body = resp.body();
|
||||
let body = resp.body;
|
||||
let data: std::collections::HashMap<String, String> = serde_json::from_slice(&body)
|
||||
.map_err(|e| Error::msg(format!("Failed to parse gateway response: {}", e)))?;
|
||||
|
||||
|
|
@ -487,23 +501,22 @@ fn process_image_inner(
|
|||
client_id
|
||||
);
|
||||
|
||||
let req = HttpRequest::new(&api_url)
|
||||
.with_method("POST")
|
||||
.with_header("Authorization", token)
|
||||
.with_header("Content-Type", "application/json");
|
||||
|
||||
let resp = http::request::<String>(&req, Some(body))?;
|
||||
if resp.status_code() >= 400 {
|
||||
let headers = std::collections::HashMap::from([
|
||||
("Authorization".to_string(), token.to_string()),
|
||||
("Content-Type".to_string(), "application/json".to_string()),
|
||||
]);
|
||||
let resp = send_http("POST", &api_url, headers, body.into_bytes())?;
|
||||
if resp.status_code >= 400 {
|
||||
if is_default {
|
||||
return Err(Error::msg(format!(
|
||||
"failed to process default image: HTTP {}",
|
||||
resp.status_code()
|
||||
resp.status_code
|
||||
)));
|
||||
}
|
||||
return process_image_inner(DEFAULT_IMAGE, client_id, token, true);
|
||||
}
|
||||
|
||||
let body = resp.body();
|
||||
let body = resp.body;
|
||||
let data: Vec<std::collections::HashMap<String, String>> = serde_json::from_slice(&body)
|
||||
.map_err(|e| Error::msg(format!("Failed to parse image response: {}", e)))?;
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@
|
|||
//! urls = "https://example.com/webhook1,https://example.com/webhook2"
|
||||
//! ```
|
||||
|
||||
use extism_pdk::{config, error, http, info, warn, HttpRequest};
|
||||
use extism_pdk::{config, error, info, warn};
|
||||
use nd_pdk::host::http::{self, HTTPRequest};
|
||||
use nd_pdk::scrobbler::{
|
||||
Error, IsAuthorizedRequest, NowPlayingRequest, PlaybackReportRequest, ScrobbleRequest,
|
||||
Scrobbler,
|
||||
|
|
@ -90,11 +91,18 @@ impl Scrobbler for WebhookPlugin {
|
|||
let full_url = format!("{}{}", url, query);
|
||||
info!("Sending webhook to: {}", full_url);
|
||||
|
||||
let http_req = HttpRequest::new(&full_url);
|
||||
match http::request::<()>(&http_req, None) {
|
||||
let http_req = HTTPRequest {
|
||||
method: "GET".into(),
|
||||
url: full_url,
|
||||
headers: Default::default(),
|
||||
no_follow_redirects: false,
|
||||
body: Vec::new(),
|
||||
timeout_ms: 0,
|
||||
};
|
||||
match http::send(http_req) {
|
||||
Ok(res) => {
|
||||
let status = res.status_code();
|
||||
if status >= 200 && status < 300 {
|
||||
let status = res.map_or(0, |r| r.status_code);
|
||||
if (200..300).contains(&status) {
|
||||
info!("Webhook succeeded: {} (status {})", url, status);
|
||||
} else {
|
||||
warn!("Webhook returned non-2xx status: {} (status {})", url, status);
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ import (
|
|||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/log"
|
||||
|
|
@ -34,6 +36,7 @@ type httpServiceImpl struct {
|
|||
pluginName string
|
||||
requiredHosts []string
|
||||
client *http.Client
|
||||
transport *http.Transport
|
||||
}
|
||||
|
||||
// newHTTPService creates a new HTTPService for a plugin.
|
||||
|
|
@ -46,8 +49,14 @@ func newHTTPService(pluginName string, permission *HTTPPermission) *httpServiceI
|
|||
pluginName: pluginName,
|
||||
requiredHosts: requiredHosts,
|
||||
}
|
||||
svc.transport = http.DefaultTransport.(*http.Transport).Clone()
|
||||
svc.transport.DialContext = (&net.Dialer{
|
||||
Timeout: 30 * time.Second,
|
||||
KeepAlive: 30 * time.Second,
|
||||
Control: svc.dialControl,
|
||||
}).DialContext
|
||||
// No client timeout: it is set per-request via context deadline.
|
||||
svc.client = httpclient.New(0)
|
||||
svc.client = &http.Client{Transport: httpclient.NewTransport(svc.transport)}
|
||||
svc.client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
|
||||
if req.Context().Value(noFollowRedirectsKey) != nil {
|
||||
return http.ErrUseLastResponse
|
||||
|
|
@ -65,6 +74,12 @@ func newHTTPService(pluginName string, permission *HTTPPermission) *httpServiceI
|
|||
return svc
|
||||
}
|
||||
|
||||
// Close releases the plugin's pooled connections when the plugin is unloaded.
|
||||
func (s *httpServiceImpl) Close() error {
|
||||
s.transport.CloseIdleConnections()
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *httpServiceImpl) Send(ctx context.Context, request host.HTTPRequest) (*host.HTTPResponse, error) {
|
||||
// Parse and validate URL
|
||||
parsedURL, err := url.Parse(request.URL)
|
||||
|
|
@ -159,11 +174,49 @@ func (s *httpServiceImpl) validateHost(ctx context.Context, hostStr string) erro
|
|||
return nil
|
||||
}
|
||||
|
||||
// dialControl checks the resolved IP, so hostnames can't reach private addresses unless a literal
|
||||
// IP/CIDR entry or a bare "*" (plugins targeting user-configured LAN services) allows it.
|
||||
func (s *httpServiceImpl) dialControl(_, address string, _ syscall.RawConn) error {
|
||||
if slices.Contains(s.requiredHosts, "*") {
|
||||
return nil
|
||||
}
|
||||
host, _, err := net.SplitHostPort(address)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
ip := net.ParseIP(host)
|
||||
if ip == nil || !isPrivateIP(ip) {
|
||||
return nil
|
||||
}
|
||||
for _, entry := range s.requiredHosts {
|
||||
if ipMatchesEntry(entry, ip) {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return fmt.Errorf("dial to private/loopback address %q blocked: requires an explicit IP or CIDR in requiredHosts", address)
|
||||
}
|
||||
|
||||
// ipMatchesEntry reports whether a requiredHosts entry is a literal IP or CIDR
|
||||
// that covers ip. Hostname and wildcard entries never match.
|
||||
func ipMatchesEntry(entry string, ip net.IP) bool {
|
||||
if _, cidr, err := net.ParseCIDR(entry); err == nil {
|
||||
return cidr.Contains(ip)
|
||||
}
|
||||
if entryIP := net.ParseIP(entry); entryIP != nil {
|
||||
return entryIP.Equal(ip)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (s *httpServiceImpl) isHostAllowed(hostname string) bool {
|
||||
ip := net.ParseIP(hostname)
|
||||
for _, pattern := range s.requiredHosts {
|
||||
if matchHostPattern(pattern, hostname) {
|
||||
return true
|
||||
}
|
||||
if ip != nil && ipMatchesEntry(pattern, ip) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
@ -182,11 +235,8 @@ func extractHostname(hostStr string) string {
|
|||
return hostStr
|
||||
}
|
||||
|
||||
// isPrivateOrLoopback returns true if the given hostname resolves to or is
|
||||
// a private, loopback, or link-local IP address. This includes:
|
||||
// IPv4: 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16
|
||||
// IPv6: ::1, fc00::/7, fe80::/10
|
||||
// It also blocks "localhost" by name.
|
||||
// isPrivateOrLoopback is a pre-flight check on the literal host (IP or "localhost"); it does not
|
||||
// resolve names, so dialControl remains the real guard.
|
||||
func isPrivateOrLoopback(hostname string) bool {
|
||||
if strings.EqualFold(hostname, "localhost") {
|
||||
return true
|
||||
|
|
@ -195,7 +245,11 @@ func isPrivateOrLoopback(hostname string) bool {
|
|||
if ip == nil {
|
||||
return false
|
||||
}
|
||||
return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()
|
||||
return isPrivateIP(ip)
|
||||
}
|
||||
|
||||
func isPrivateIP(ip net.IP) bool {
|
||||
return ip.IsLoopback() || ip.IsUnspecified() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()
|
||||
}
|
||||
|
||||
// Verify interface implementation
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package plugins
|
|||
import (
|
||||
"context"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
|
|
@ -43,6 +44,21 @@ var _ = Describe("httpServiceImpl", func() {
|
|||
Expect(err.Error()).To(ContainSubstring("private/loopback"))
|
||||
})
|
||||
|
||||
It("should block a symbolic hostname that resolves to loopback (SSRF)", func() {
|
||||
ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(200)
|
||||
}))
|
||||
// The trailing dot passes the pre-flight string check; only the dial-time guard catches it.
|
||||
_, port, _ := net.SplitHostPort(strings.TrimPrefix(ts.URL, "http://"))
|
||||
_, err := svc.Send(context.Background(), host.HTTPRequest{
|
||||
Method: "GET",
|
||||
URL: "http://localhost.:" + port + "/test",
|
||||
TimeoutMs: 1000,
|
||||
})
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("private/loopback"))
|
||||
})
|
||||
|
||||
It("should block requests to localhost by name", func() {
|
||||
_, err := svc.Send(context.Background(), host.HTTPRequest{
|
||||
Method: "GET",
|
||||
|
|
@ -419,13 +435,53 @@ var _ = Describe("httpServiceImpl", func() {
|
|||
Expect(resp).To(BeNil())
|
||||
})
|
||||
|
||||
It("blocks a private IP reached via a hostname allowlist entry (rebinding protection)", func() {
|
||||
// Allowlisting a name authorizes the external service, not whatever private
|
||||
// IP it may resolve or rebind to. Only literal IP/CIDR entries do that.
|
||||
svc.requiredHosts = []string{"api.example.com"}
|
||||
err := svc.dialControl("tcp", "10.0.0.1:80", nil)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("private/loopback"))
|
||||
})
|
||||
|
||||
It("allows a private IP that an explicit CIDR allowlist entry authorizes", func() {
|
||||
svc.requiredHosts = []string{"10.0.0.0/8"}
|
||||
Expect(svc.dialControl("tcp", "10.0.0.1:80", nil)).To(Succeed())
|
||||
})
|
||||
|
||||
It("allows private IPs when the allowlist is the bare '*' wildcard", func() {
|
||||
svc.requiredHosts = []string{"*"}
|
||||
Expect(svc.dialControl("tcp", "192.168.1.10:8000", nil)).To(Succeed())
|
||||
Expect(svc.dialControl("tcp", "127.0.0.1:8000", nil)).To(Succeed())
|
||||
})
|
||||
|
||||
It("still blocks private IPs for a subdomain wildcard entry", func() {
|
||||
svc.requiredHosts = []string{"*.example.com"}
|
||||
Expect(svc.dialControl("tcp", "10.0.0.1:80", nil)).To(MatchError(ContainSubstring("private/loopback")))
|
||||
})
|
||||
|
||||
It("closes idle pooled connections on Close", func() {
|
||||
closed := make(chan struct{})
|
||||
ts = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
|
||||
ts.Config.ConnState = func(_ net.Conn, state http.ConnState) {
|
||||
if state == http.StateClosed {
|
||||
close(closed)
|
||||
}
|
||||
}
|
||||
ts.Start()
|
||||
svc.requiredHosts = []string{"127.0.0.1"}
|
||||
_, err := svc.Send(context.Background(), host.HTTPRequest{Method: "GET", URL: ts.URL, TimeoutMs: 1000})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(svc.Close()).To(Succeed())
|
||||
Eventually(closed).Should(BeClosed())
|
||||
})
|
||||
|
||||
It("should allow wildcard host patterns", func() {
|
||||
ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write([]byte("wildcard"))
|
||||
}))
|
||||
// *.allowed.org is in the requiredHosts from BeforeEach, but test server is 127.0.0.1
|
||||
// Override with a wildcard that matches the test server
|
||||
svc.requiredHosts = []string{"*.0.0.1"}
|
||||
// The literal IP is what authorizes the loopback dial under the private-IP guard.
|
||||
svc.requiredHosts = []string{"*.0.0.1", "127.0.0.1"}
|
||||
resp, err := svc.Send(context.Background(), host.HTTPRequest{
|
||||
Method: "GET",
|
||||
URL: ts.URL,
|
||||
|
|
@ -566,6 +622,11 @@ var _ = Describe("isPrivateOrLoopback", func() {
|
|||
Expect(isPrivateOrLoopback("fe80::1")).To(BeTrue())
|
||||
})
|
||||
|
||||
It("should detect unspecified addresses, which dial the local host", func() {
|
||||
Expect(isPrivateOrLoopback("0.0.0.0")).To(BeTrue())
|
||||
Expect(isPrivateOrLoopback("::")).To(BeTrue())
|
||||
})
|
||||
|
||||
It("should allow public IPs", func() {
|
||||
Expect(isPrivateOrLoopback("8.8.8.8")).To(BeFalse())
|
||||
Expect(isPrivateOrLoopback("203.0.113.1")).To(BeFalse())
|
||||
|
|
|
|||
|
|
@ -148,7 +148,7 @@ var hostServices = []hostServiceEntry{
|
|||
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
|
||||
perm := ctx.permissions.Http
|
||||
service := newHTTPService(ctx.pluginName, perm)
|
||||
return host.RegisterHTTPHostFunctions(service), nil, nil
|
||||
return host.RegisterHTTPHostFunctions(service), service, nil
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -498,18 +498,12 @@ func parsePluginConfig(configJSON string) (map[string]string, error) {
|
|||
return pluginConfig, nil
|
||||
}
|
||||
|
||||
// buildExtismManifest describes the plugin to extism. It must never set
|
||||
// AllowedPaths: extism would replace our jailed FSConfig with plain dir mounts.
|
||||
// buildExtismManifest describes the plugin to extism. It must never set AllowedPaths (extism would replace our
|
||||
// jailed FSConfig) nor AllowedHosts (extism's http_request has no SSRF guard; plugins must use host.HTTPSend).
|
||||
func buildExtismManifest(pkg *ndpPackage, pluginConfig map[string]string) extism.Manifest {
|
||||
manifest := extism.Manifest{
|
||||
return extism.Manifest{
|
||||
Wasm: []extism.Wasm{extism.WasmData{Data: pkg.WasmBytes, Name: "main"}},
|
||||
Config: pluginConfig,
|
||||
Timeout: uint64(defaultTimeout.Milliseconds()),
|
||||
}
|
||||
if pkg.Manifest.Permissions != nil && pkg.Manifest.Permissions.Http != nil {
|
||||
if hosts := pkg.Manifest.Permissions.Http.RequiredHosts; len(hosts) > 0 {
|
||||
manifest.AllowedHosts = hosts
|
||||
}
|
||||
}
|
||||
return manifest
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,8 +23,8 @@ var _ = Describe("buildExtismManifest", func() {
|
|||
Expect(buildExtismManifest(pkg, nil).AllowedPaths).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("carries the hosts the plugin is allowed to reach", func() {
|
||||
Expect(buildExtismManifest(pkg, nil).AllowedHosts).To(Equal([]string{"example.com"}))
|
||||
It("never sets AllowedHosts, so plugin HTTP can't bypass the host service's SSRF guard", func() {
|
||||
Expect(buildExtismManifest(pkg, nil).AllowedHosts).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue