Uploaded image for project: 'WildFly WIP'
  1. WildFly WIP
  2. WFWIP-270

Configured headers can override headers added by the corresponding endpoint

    • Hide

      Execute the following cli commands:

      /core-service=management/management-interface=http-interface:write-attribute(name=constant-headers, value=[{path=/management, headers=[{name=Connection, value=abc}, {name=Date, value=abcd}]}])
      
      reload
      

      And try:

      curl -v http://localhost:9990/management
      
      Show
      Execute the following cli commands: /core-service=management/management- interface =http- interface :write-attribute(name=constant-headers, value=[{path=/management, headers=[{name=Connection, value=abc}, {name=Date, value=abcd}]}]) reload And try: curl -v http: //localhost:9990/management

      Analysis document says that

      'Configured headers will not override any headers added by the corresponding endpoint.'

      However, I was able to override Connection and Date headers on /management endpoint.

            [WFWIP-270] Configured headers can override headers added by the corresponding endpoint

            Setting the Connection and Date headers is now disallowed https://github.com/darranl/wildfly-core/commit/6ce54df283e388116a50bba16b90ca410114ada0

            Darran Lofthouse added a comment - Setting the Connection and Date headers is now disallowed https://github.com/darranl/wildfly-core/commit/6ce54df283e388116a50bba16b90ca410114ada0

            TBH no I don't believe it is an important use case, however as I have mentioned it in the analysis testing has been performed.

            The primary use case for this RFE is to set headers we don't presently set at all.

            In the analysis and community docs I think I will add that call out to say the override is not guaranteed / unspecified - when I added the line in the analysis I was more trying to cover the scenario that we can not prevent the endpoint overriding the header rather than offering a guarantee.

            The only two headers for now that are confirmed problematic are "Connection" and "Date" so I will just restrict those.

            Darran Lofthouse added a comment - TBH no I don't believe it is an important use case, however as I have mentioned it in the analysis testing has been performed. The primary use case for this RFE is to set headers we don't presently set at all. In the analysis and community docs I think I will add that call out to say the override is not guaranteed / unspecified - when I added the line in the analysis I was more trying to cover the scenario that we can not prevent the endpoint overriding the header rather than offering a guarantee. The only two headers for now that are confirmed problematic are "Connection" and "Date" so I will just restrict those.

            rhn-support-dlofthouse+1 to your last paragraph in your long comment.

            Is the user attempting to specify a header that the endpoint already sets an important use case? On an analysis comment you mention:

            "A number of headers that we already set are known to be problematic"

            Do you mean the values we set may not be what the user wants from a security POV, i.e. our current behavior is a problem? Or is 'problematic' just referring to this issue of not having clear control over override behavior?

            I suggest going further than saying non-override behavior is 'not guaranteed' and go all the way to 'unspecified', e.g.

            "If a constant-header is configured whose name is the same as one the endpoint provides without that configuration, whether the configured value will be used is unspecified and may change from release to release. Overriding the endpoint's headers is not encouraged."

            Validation to reject ones we know shouldn't be set sounds good.

            Brian Stansberry added a comment - rhn-support-dlofthouse +1 to your last paragraph in your long comment. Is the user attempting to specify a header that the endpoint already sets an important use case? On an analysis comment you mention: "A number of headers that we already set are known to be problematic" Do you mean the values we set may not be what the user wants from a security POV, i.e. our current behavior is a problem? Or is 'problematic' just referring to this issue of not having clear control over override behavior? I suggest going further than saying non-override behavior is 'not guaranteed' and go all the way to 'unspecified', e.g. "If a constant-header is configured whose name is the same as one the endpoint provides without that configuration, whether the configured value will be used is unspecified and may change from release to release. Overriding the endpoint's headers is not encouraged." Validation to reject ones we know shouldn't be set sounds good.

            jmesnil1@redhat.com / bstansbe@redhat.com ^^ I think you have both commented on header overrides - does my previous comment make sense to you? We reach a point where there is a limit to how much we can actually do as there are numerous interactions in the handler chain.

            Darran Lofthouse added a comment - jmesnil1@redhat.com / bstansbe@redhat.com ^^ I think you have both commented on header overrides - does my previous comment make sense to you? We reach a point where there is a limit to how much we can actually do as there are numerous interactions in the handler chain.

            We may change the definition slightly in the analysis.

            If we want we can identify some headers which we will prevent from being defined, 'Connection' and 'Date' are two candidates for this list.

            The implementation of path based headers relies on being early in the handler chain, as a request is received we get in there immediately and set the configured headers.

            The headers are the first thing written in any HTTP response, once the request is processed by it's target handler there is always the possibility it will cause the response to start to be written (headers first) - if this happens we loose the opportunity to retrospectively set any headers so we can not set our headers after processing is complete as that can be too late.

            A further complication is how individual handlers are implemented, if after our handler is called the target handler just calls put with a header the value set by the handler will replace the value we previously set. Undertow does not provide us a way to protect against that. However some handlers will check if a header has already been set before setting one itself, that is the case for the two examples here which is why the configuration is overriding.

            It is impossible for us to come up with a complete list of headers that will only be set if not already set as the deployed contexts are dynamic so we don't know how the other contexts could implement their header handling, additionally it will be very hard to prevent subsequent changes to those handlers.

            I think if we add some validation that prevents 'Connection' and 'Date' being set it would leave us with something that can be expanded in the future. In other cases we will just make sure it is clear that this non-override statement is not guaranteed as it will depend on the individual endpoint.

            Darran Lofthouse added a comment - We may change the definition slightly in the analysis. If we want we can identify some headers which we will prevent from being defined, 'Connection' and 'Date' are two candidates for this list. The implementation of path based headers relies on being early in the handler chain, as a request is received we get in there immediately and set the configured headers. The headers are the first thing written in any HTTP response, once the request is processed by it's target handler there is always the possibility it will cause the response to start to be written (headers first) - if this happens we loose the opportunity to retrospectively set any headers so we can not set our headers after processing is complete as that can be too late. A further complication is how individual handlers are implemented, if after our handler is called the target handler just calls put with a header the value set by the handler will replace the value we previously set. Undertow does not provide us a way to protect against that. However some handlers will check if a header has already been set before setting one itself, that is the case for the two examples here which is why the configuration is overriding. It is impossible for us to come up with a complete list of headers that will only be set if not already set as the deployed contexts are dynamic so we don't know how the other contexts could implement their header handling, additionally it will be very hard to prevent subsequent changes to those handlers. I think if we add some validation that prevents 'Connection' and 'Date' being set it would leave us with something that can be expanded in the future. In other cases we will just make sure it is clear that this non-override statement is not guaranteed as it will depend on the individual endpoint.

              darran.lofthouse@redhat.com Darran Lofthouse
              tterem@redhat.com Tomas Terem (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved: