SOLR-14853 enableRemoteStreaming and enableStreamBody are now sys props

[dsmiley]

No longer configSet / per-core.

Existing configuration (e.g. in solrconfig.xml or via config edit API) is ignored with a deprecation warning.

Even though this is a backwards incompatible change, I think it's fine in a minor release because:

• is probably mostly being used in a local developer setting, not in production, thus backwards incompatibility is much less of a concern
• it's easy to adapt
• the user gets a warning if they don't
• existing configs or config-edit won't fail (but will no-op admittedly)
• improves security (defense-in-depth)

ref guide / changes.txt edits needed

https://issues.apache.org/jira/browse/SOLR-14853

[chatman]

Existing configuration (e.g. in solrconfig.xml or via config edit API) is ignored with a deprecation warning.

I think they should be removed as well.

[janhoy]

Did a quick browse, looks good. I agree that this change is acceptable in a minor release, for the reasons given.

@chatman, I think it makes sense to keep the core-level settings and APIs as no-ops in 9.x, declare them as deprecated in 9x and remove in main branch only.

For all the test-solrconfig.xml files I think we can do a global search/replace <requestParsers.*?enableRemoteStreaming=".*?" with <requestParsers and the same with stream body, as these are no-op anyway now.

[janhoy]

We should also give these two new sysProps their own line and env.var in solr.in.sh? And unfortunately we'll also need some code in bin/solr[.cmd] for parsing it. I can contribute commits for this if you wish.

[dsmiley]

Sure; will remove from the configs in Solr -- easy enough.

Why modify solr.in.sh at all? I'd rather not; to be honest, I'd rather that file not exist at all (by default).

If/once Solr has a solr.env=prod|dev|test then we'd enable this setting in dev mode, and then feel no need later to give it such first class treatment to deserve it's very own env (which I assume is what motivated you to suggest an entry in solr.in.sh?).

[chatman]

If/once Solr has a solr.env=prod|dev|test then we'd enable this setting in dev mode

@dsmiley , there's already "-Dsolr.environment=prod" in Solr.

https://github.com/apache/solr/blob/main/solr/bin/solr.in.sh#L239-L242

[janhoy]

Why modify solr.in.sh at all? I'd rather not; to be honest, I'd rather that file not exist at all (by default).

All lines in solr.in.sh are commented, so it is not needed by default. But the reason I'd prefer all sysprops to have their own ENV equivalent is for ease of configuration in e.g. Docker / k8s as a simple env.var instead of having to do SOLR_OPTS concatenation.

@dsmiley , there's already "-Dsolr.environment=prod" in Solr.

Yes, I put it in there, and all it does now is change color in Admin UI between envs :) We could certainly re-use it to set defaults for other things. But do we want remote streaming to be enabled by default in dev/test?

[dsmiley]

I propose in this issue not looking at solr.environment -- leave that for https://issues.apache.org/jira/browse/SOLR-15875

[chatman]

But do we want remote streaming to be enabled by default in dev/test?

Not in my opinion. Rather, I think we should remove remote streaming from Solr. If something shouldn't be used in production, it shouldn't exist in Solr.

[dsmiley]

BTW I want to call attention to

solr/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java

Line 129 in 6d5529a

public Name getPermissionName(AuthorizationContext request) {

which feels weird to me. I don't think I love varying the permission of a handler based on wether remote streaming is enabled or not. Nor do I know why CONFIG_READ_PERM in particular was chosen. I suppose this is here because it's super easy to have Solr fetch whatever URL for you on your behalf... but if you enabled remote streaming then, shrug, it all seems moot honestly.

[chatman]

Good catch, David. I think the remote streaming "functionality" should be scrapped. DumpRequestHandler should either be scrapped or, at least, shouldn't be allowed to work with remote streaming, and the https://solr.apache.org/guide/8_11/content-streams.html page should be re-written to remove all references to such features.

[janhoy]

Nor do I know why CONFIG_READ_PERM in particular was chosen.

You and I discussed this in the original PR to secure all request handlers: https://github.com/apache/solr/pull/372/files/ae46e8d101026c3439fb23a07d1603c884048765#r737730716

[janhoy]

Why modify solr.in.sh at all? I'd rather not; to be honest, I'd rather that file not exist at all (by default).

I'll push a change that only adds parsing of SOLR_ENABLE_REMOTE_STREAMING and SOLR_ENABLE_STREAM_BODY to the scripts, but nothing in solr.in.xx. That way we don't advertise this feature, while still allowing Solr to pick it up from the environment, and we can document both sys.prop and env.var in refGuide.

[janhoy]

I'm going to commit solrconfig.xml fixes (removing now noop params and xml comments about them)

[janhoy]

Only RefGuide & Changes left to do here I believe?

[dsmiley]

Yes. Thanks for your help, BTW. I'll be busy today but I offer this:

Proposed CHANGES.txt

• SOLR-14853: Security: Converted enableRemoteStreaming and enableStreamBody solrconfig options into system properties and env vars. Attempts to set them the old way will no-op and log a warning. (me, Jan, Ishan)

upgrade notes:
In order to improve security, use of stream.file and stream.url and stream.body is no longer enabled via configuration in solrconfig.xml, nor dynamic equivalents with the config API. Instead, set an env var: ___ or ___ or system property equivalents.

[dsmiley]

This should do it. I'll commit in a couple days if nobody says anything.

[dsmiley]

I prefer to not show any example on how to set any of these requestParsers details dynamically, thus I did not swap out the examples for the other still-valid configuration attributes. IMO none of these should be in solrconfig.xml in the first place -- they are obscure things that I don't think someone would want to set on any one config (collection); instead I think all of them should be global. Making stuff configurable has some maintenance cost and are arcane details to many people. This is just my opinion.