[jira] [Commented] (SOLR-16955) Tracing v2 apis breaks SecurityConfHandler

["Alex Deparvu (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-16955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17761352#comment-17761352 ] 

Alex Deparvu commented on SOLR-16955:
-------------------------------------

Adding more details: the span name for v2 apis has the following format: 'path:verb'.
For the `verb` part, the method is consuming the stream to read commands as an attempt pick a better value than the request method.

For the failing test (BasicAuthIntegrationTest.testBasicAuth)
* the command is `{"set-user":{"harry":"HarryIsUberCool"}}`
* the result being a span named `set-user:/cluster/security/authentication`.
* the non glamorous alternative would be a simple `post:/cluster/security/authentication`

I find the approach to read the stream too risky and invasive. there is no guarantee the entire current and future code base will respect the pattern and call `solrReq.getCommands(validateInput)` instead of just simply reading the stream directly. this is why I am hesitant to fix the SecurityConfHandler and I would opt for removing the stream reading on the tracing side. spans can have a simpler name, and if we want to augment the metadata we can do so form inside the SecurityConfHandler (for example we can set an attribute 'op' to 'set-user' when it is safe to do so).
My proposal is to remove the code inside V2HttpCall, and if strictly needed I can add an extra attribute for the operation.
[~janhoy][~dsmiley] please let me know your thoughts.


> Tracing v2 apis breaks SecurityConfHandler
> ------------------------------------------
>
>                 Key: SOLR-16955
>                 URL: https://issues.apache.org/jira/browse/SOLR-16955
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: tracing
>            Reporter: Alex Deparvu
>            Priority: Blocker
>             Fix For: 9.4
>
>
> Tracing for v2 apis will eagerly consume the contentstream to find a good name for the span. the assumption here is that the operations are cached so the stream ca be consumed early, but that is not the case for the SecurityConfHandler and possibly other parts of the code.
> With tracing enabled you cannot add a user, a role, etc. Admin UI Security is broken too.
> The clash is between
> * V2HttpCall [solrReq.getCommands|https://github.com/apache/solr/blob/9a453854a56125fef6740ce8e4a1f69fb444eabe/solr/core/src/java/org/apache/solr/api/V2HttpCall.java#L513]
> * SecurityConfHandler [CommandOperation.readCommands(req.getContentStreams()..)|https://github.com/apache/solr/blob/9a453854a56125fef6740ce8e4a1f69fb444eabe/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java#L115]
> relevant stacktrace
> {noformat}
> 2023-08-29 19:13:57.591 INFO  (qtp605101809-21) [t:750564258779a1605867f784ee33f78a] o.a.s.s.HttpSolrCall [admin] webapp=null path=/cluster/security/authorization params={wt=json&_=1693361587976} status=0 QTime=0
> 2023-08-29 19:13:57.613 ERROR (qtp605101809-27) [t:b3ab14d057a292fb71ac727c5d871156] o.a.s.h.RequestHandlerBase Server exception => java.lang.RuntimeException: The JSON must be an Object of the form {"command": {...},...
> 	at org.apache.solr.common.util.CommandOperation.parse(CommandOperation.java:275)
> java.lang.RuntimeException: The JSON must be an Object of the form {"command": {...},...
> 	at org.apache.solr.common.util.CommandOperation.parse(CommandOperation.java:275) ~[?:?]
> 	at org.apache.solr.common.util.CommandOperation.readCommands(CommandOperation.java:354) ~[?:?]
> 	at org.apache.solr.common.util.CommandOperation.readCommands(CommandOperation.java:327) ~[?:?]
> 	at org.apache.solr.handler.admin.SecurityConfHandler.doEdit(SecurityConfHandler.java:116) ~[?:?]
> 	at org.apache.solr.handler.admin.SecurityConfHandler.handleRequestBody(SecurityConfHandler.java:89) ~[?:?]
> 	at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:224) ~[?:?]
> 	at org.apache.solr.api.ApiBag$ReqHandlerToApi.call(ApiBag.java:362) ~[?:?]
> 	at org.apache.solr.api.V2HttpCall.handleAdmin(V2HttpCall.java:438) ~[?:?]
> 	at org.apache.solr.servlet.HttpSolrCall.handleAdminRequest(HttpSolrCall.java:870) ~[?:?]
> 	at org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:539) ~[?:?]
> 	at org.apache.solr.servlet.SolrDispatchFilter.dispatch(SolrDispatchFilter.java:248) ~[?:?]
> 	at org.apache.solr.servlet.SolrDispatchFilter.lambda$doFilter$0(SolrDispatchFilter.java:215) ~[?:?]
> 	at org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2(ServletUtils.java:241) ~[?:?]
> 	at org.apache.solr.servlet.ServletUtils.rateLimitRequest(ServletUtils.java:211) ~[?:?]
> 	at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:209) ~[?:?]
> 	at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:192) ~[?:?]
> {noformat}
> I ran into this a few times and could not pinpoint it until now. the reason this does not trigger on the alway-on tracer is that this is hidden behind a 'TraceUtils.ifNotNoop' check.
> Also, I think this might be a problem on 9.x too, not completely sure yet.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org