Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WSS4J2 Wss4jSecurityInterceptor -> validation configuration errors [SWS-961] #1032

Closed
gregturn opened this issue Jun 18, 2016 · 11 comments
Closed
Assignees
Milestone

Comments

@gregturn
Copy link
Member

@gregturn gregturn commented Jun 18, 2016

Manuel Dominguez Sarmiento opened SWS-961 and commented

In our use case, we don't use any validation, just securement. This used to work in previous Spring WS versions that depend on WSS4J 1.x, without setting the validationActions property. However when upgrading to Spring WS 2.3.0 and WSS4J 2.x our XML configuration fails because of the following two issues:

a) NullPointerException in validateMessage() because validationActionsVector is not initialized. This field is only initialized if setValidationActions() is invoked, which should not be necessary if validation is not required. NoSecurity should be configured by default.

b) After configuring validationActions="NoSecurity" our configuration still caused failures (Wss4jSecurityValidationException("No WS-Security header found")), as validation was being attempted anyway. This is due to validationActionsVector being initialized as an empty list by WSSecurityUtil.decodeAction("NoSecurity"). Thus the validationActionsVector.contains(WSConstants.NO_SECURITY) check in validateMessage() fails. An empty vector should be interpreted as no validation required, even if validateRequest or validateResponse are true (which are the defaults)

We resolved this by setting the following properties, which should not be required for our use case:
validationActions="NoSecurity"
validateRequest="false"
validateResponse="false"


Affects: 2.3.0

Issue Links:

  • #1018 Setting up a Wss4jSecurityInterceptor as no security still requires WS-Security header
    ("is superseded by")

Backported to: 2.4.1

3 votes, 9 watchers

@gregturn
Copy link
Member Author

@gregturn gregturn commented Jun 27, 2016

jaminh commented

I addressed this as part of the changes I made for #1027. You can try running from my branch https://github.com/jaminh/spring-ws/tree/feature/SWS-955 and see if that fixes it for you.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 17, 2016

Marc Eckart commented

I have the same Problem, setting validateRequest and Response to false also helped me.

I did a little debugging and found in the 2.3.0 Release in the class AbstractWsSecurityInterceptor in the handleResponse method the following line:

if(this.skipValidationIfNoHeaderPresent && !this.isSecurityHeaderPresent((SoapMessage)messageContext.getRequest())) {
      return true;
  } 

Although the skipValidationIfNoHeaderPresent flag is set, it will always find a security header in the request, because I added one when sending the request to the server, so it will always step into the validation part. But I'm here in the handle response context so, why would I care for the request ... just the response. And despite from this line there is only referred to messageContext.getResponse in the rest of the handleResponse method. Maybe that's the reason why this flag is not working.

But the handling of the validationActions for NoSecurity seems also buggy, because the decode returns an empty list and not the expected Integer.valueOf(0) ... so the valudateMessage Method in class Wss4jSecurityInterceptor should better check against empty List instead of if(!this.validationActionsVector.contains(Integer.valueOf(0))) {

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 19, 2016

jaminh commented

I made a new branch to address the validation actions issues specifically https://github.com/jaminh/spring-ws/tree/feature/SWS-961. I have it checking the validationActionsVector for null or empty now. It seems strange to me that Wss4j is decoding "NoSecurity" as an empty list as opposed to a list with the NO_SECURITY(0) value in it. I left the original check for the NO_SECURITY value in there even though looking at the code it seems like it will never get used. Also I agree that the client handleResponse method should be checking the response for a security header so I changed that as well. I added tests for the null, empty and "NoSecurity" securement and validation actions. Let me know if that solves your issues or your thoughts on my solution.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 20, 2016

Ryan J. McDonough commented

I've had the same issue with using the NoSecurity option as well and found the same issue as @jaminh. The problem starts in the Wss4jSecurityInterceptor on line 386. As @jaminh points out, the list is empty. This method seems to assume that the WSSecurityUtil is returning a value, but it is not. It's returnning an empty collection as evidenced by line 418 in the WSSecurityUtil class in wss4j.

The problem really lies at line 646 of the Wss4jSecurityInterceptor as it assumes there's a value of WSConstants.NO_SECURITY in the collection, but there never will be.

I think the solution proposed by @jaminh might be the right direction, but I think there needs to be a bit more thought here. For example, if someone sets a validationAction of UsernameToken Timestamp NoSecurity, what should be the behavior? My thought is that NoSecurity should trump everything and disable validation. But with this proposal, the NoSecurity option is still dropped on the floor. My thoughts are that the setValidationActions() method should be enhanced to add a bit more logic to check for the NoSecurity option and populate the validationActionsVector accordingly.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 27, 2016

jaminh commented

I would think as far as the Wss4jSecurityInterceptor is concerned the validationAction String should be parsed the way the WSSecurityUtil decodeAction method parses it. Looking at that method is appears it will interpret either null, an empty String, or a String beginning with "NoSecurity" as meaning no security actions will be taken and an empty list is returned. Based on the example of "UsernameToken Timestamp NoSecurity" the WSSecurityUtil.decodeAction method would interpret that as the UsernameToken and Timestamp action. If you think that behaviour is incorrect I think the appropriate place to change it would be in the WSSecurityUtil.decodeAction method rather than in the Wss4jSecurityInterceptor.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

Vity commented

Jesus, this bug has been reported more than 1 year ago and the fix is easy. Are there any maintainers here?

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

jaminh commented

This issue was addressed as part of #1018.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

Greg Turnquist commented

Resolved by #1018 in 2.x and #1078 in 3.0

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

Vity commented

@jaminh @greg turniquist thanks to both of you.
I am using Spring Boot 1.5.8 (spring-boot-starter-web-services 1.5.8) which still includes version 2.4.0. Shall I wait for the next Spring Boot WS starter release or shall I override the version?

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

Greg Turnquist commented

I'd suggest setting:

spring-ws.version=2.4.1.RELEASE

and giving it a whirl in your existing application.

Depending on whether you are using Maven or Gradle, this is done differently.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 30, 2017

Vity commented

Thank you, upgraded successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.