elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.43k stars 24.87k forks source link

Explicitly configuring the `default` Transport profile silently overrides the typical settings #90964

Open gwbrown opened 2 years ago

gwbrown commented 2 years ago

Elasticsearch Version

current main, all supported versions of ES I'm aware of

Installed Plugins

n/a

Java Version

n/a

OS Version

n/a

Problem Description

The default Transport port uses an implicit Transport profile named default. Because Transport profiles can be configured using the transport.profiles.<profile_name>.* setting groups, the default Transport profile can be configured using the transport.profiles.default.* settings as well as the "usual" settings we would recommend for this, such as transport.port, xpack.security.transport.filter.enabled, etc.

At least in the case of Security's Transport filters, settings on the transport.profiles.default.* group silently override the transport.*/xpack.security.transport.* settings.

I haven't yet put in the time to figure out if this applies to all Transport settings, or just the Transport filters.

Steps to Reproduce

  1. Configure a node with a configuration similar to the following, with conflicting Transport filter settings between the "typical" settings and transport.profiles.default.*:
    
    # Port 9300 with `xpack.security.transport.*` filter
    transport.host: [ local ]
    transport.port: 9300
    xpack.security.transport.filter.enabled: true

xpack.security.transport.filter.allow: [ "127.0.0.0/8" ] xpack.security.transport.filter.deny: _all

Ports 9501 & 9502 with transport.profiles.default.* filter

transport.profiles.default.xpack.security.filter.allow: [ "10.0.0.0/8" ] transport.profiles.default.xpack.security.filter.deny: _all


2. Start the node.
3. Observe that the Transport filters configured using `transport.profiles.default.*` are in effect, while those configured using `xpack.security.transport.filter.*` are not.

### Logs (if relevant)

_No response_
elasticsearchmachine commented 2 years ago

Pinging @elastic/es-security (Team:Security)

elasticsearchmachine commented 2 years ago

Pinging @elastic/es-distributed (Team:Distributed)

gwbrown commented 2 years ago

The relevant code can be found here: https://github.com/elastic/elasticsearch/blob/954d288f4587e014ba5b381bce2bd090ce12285c/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java#L315-L325

We put the profile configuration from the "usual" settings into a Map<String, SecurityIpFilterRule[]> with the hardcoded default key, then iterate through the Profile setting groups doing the same thing, so if there's a profile with the name default it'll stomp on the initial default entry.

gwbrown commented 2 years ago

The transport.port also follows this pattern, so at least we're consistent: https://github.com/elastic/elasticsearch/blob/ae9fa4e3b02843d9223b63ab39cbb9d9576aa401/server/src/main/java/org/elasticsearch/transport/TcpTransport.java#L597-L604

I will also note that the pattern that e.g. transport.host use is that transport.host is used as the default for transport.bind_host and transport.publish_host, which in turn are used as defaults for transport.profiles.<profile_name>.bind_host & ".publish_host, so transport.profiles.default.publish_host will silently override transport.host - it's likely the behavior described in the original ticket was intentional to match behavior for, say, transport.port and transport.profiles.default.port. I'm not convinced this is necessarily the best setup - while it's consistent, it's not really intuitive - and perhaps an argument in favor of making transport profiles as unnecessary as possible.