Breaking Struts changes in Confluence 8.8

Confluence Development Confluence Data Center Announcements
, ,
1

Hi all,

We have once again upgraded Struts in Confluence 8.8 and introduced 2 breaking changes as they were deemed necessary to enhance the security of our customers.

–

StrutsParameter Annotation

All Struts Action setters, getters or public fields which are intended for parameter injection must now be annotated with the new @StrutsParameter annotation.

This is in contrast to previous versions of Confluence where only “complex” parameters, that is those represented by getters that return DTOs, required annotating with @ParameterSafe.

Note that the deprecated @ParameterSafe annotation will continue to be recognised for backwards compatibility but we recommend migrating to the new annotation which offers fine-grained control with respect to parameter injection depth.

Whilst the new parameter annotation will be present in 8.8 EAP2, the requirement to annotate ALL parameters will be feature flagged off until 8.8 BETA1.

Please refer to the ‘Defining Request Parameters’ section in the Struts Module documentation for further information.

OGNL Class Allowlist

OGNL expressions are now subject to a strict class allowlist. Most plugins will function with no additional configuration as Struts will intelligently allowlist classes based on the Action classes declared in your Struts module as well as parameter annotations on your Action classes.

However, if you encounter any issues or log warnings, we have implemented a new Struts module element to allow plugins to manually allowlist any necessary classes or packages.

The OGNL Class Allowlist will be permanently enabled from 8.8 EAP2.

Please refer to the ‘OGNL Class Allowlist’ section in the Struts Module documentation for further information.

–

8.5 LTS Backports

The OGNL Class Allowlist WILL be backported to Confluence 8.5.6. We recommend ensuring compatibility of the Struts components of your plugin with Confluence 8.8 as soon as possible.

Whilst the @StrutsParameter annotation will be available beginning in Confluence 8.5.6, we do not currently intend to extend the annotation requirement to all Struts parameters as in Confluence 8.8. Instead, the present requirement of annotating only “complex” parameters will remain.

–

Regards,
Kusal Kithul-Godage
Software Engineer, Confluence Data Center

2 Likes
2

Hi @Kusal, thanks for the heads-up!

  • Is the @StrutsParameter annotation also required on setters for parameters set via static parameters defined in atlassian-plugin.xml?
  • Where is the @StrutsParameter annotation located? Do we have to use a specific class instance that is OSGi-imported from Confluence or can we just declare the annotation in our own codebase as long as the fully qualified classname matches? Background: We need to support older Confluence versions as well that might not provide this annotation and we compile against these versions.

Thanks,
Jens

3

Hi @jens

Great questions.

  • The Static Parameters Interceptor is unimpacted by these changes. You are also welcome to consult this pull request for exact implementation details if it would help.
  • It does need to be OSGi imported.
    • The easiest way to achieve compatibility when compiling against Confluence versions where this annotation is not available is to use the old @ParameterSafe annotation which will be recognised in its place (even for setters where it was not previously required). This however comes with the limitation that the depth field will be fixed at 2. It cannot be decreased for stronger security nor increased for more complex DTO structures.
    • Whilst I haven’t tested this, it’s potentially possible to have your own copy of the @StrutsParameter class in provided scope for compilation, and then define an optional OSGi import to ensure the correct variant of the class is recognised during runtime.
4

Thanks @Kusal!

Good to know the static params are unaffected. This helps a lot with one of our shared libs providing reusable action classes for iframe-based UIs.

Concerning the annotations:
I assume you refer to com.atlassian.xwork.ParameterSafe from the atlassian-xwork-core / struts-support library. That annotation should work in our case, as we’d be using it only on simple String fields.
But will it remain available in Confluence 9.0? We need to be able to support Confluence 8+9 at the same time.

Btw. it’s great to see Atlassian contributing to Struts upstream :slight_smile:

1 Like
5

By compiling against Confluence 8.5.6 you would be able to provide support for Confluence 8.5 - 9.x. Admittedly, it would not allow you to support Confluence 8.0 - 8.4. If this is a common vendor requirement we can consider extending backwards compatibility with the old annotation into Confluence 9.x.

6

Are you saying you’ll also backport the struts update with the new annotation to Confluence 8.6 and 8.7? Because unless you do this we won’t really be able to support 8.5.6+.

That’s because in AMKT we can only specify a single version range and maintaining separate app versions just for the compatibility range is very tedious.
Given that 8.5 is an LTS release we need to be able to support it.

Therefore I’d vote for providing the annotation in Confluence 9.x :slight_smile:

7

Since 8.5.6 will contain both annotations, you should be able to support 8.6 and 8.7 too by using both the old and new annotation on “complex” parameters.

8

Hi Kusal,

Thanks for the additional updates.

Your post mentions certain items that will be present from “8.8 EAP2”. To which release number (8.8.0-mXX) does “EAP2” correspond?

You also mention that “the requirement to annotate ALL parameters will be feature flagged off until 8.8 BETA1”. Is this a feature flag or dark feature that vendors can enable right now (or starting from some -mXX release) to test in advance of beta1? There is often a very short time window between the beta and the general release, so we would like to be prepared in advance.

I would also like to second Jens’s vote for extending compatibility of the ParameterSafe annotation in 9.0. Otherwise, it becomes difficult to support both Conf 8 and 9 with the same JAR.

Scott

2 Likes
9

Hi @Kusal,

One more thing. As far as I can tell, the @ParameterSafe annotation mentioned as a backup is available only starting in Confluence 8.0. There are, however, a number of Confluence 7.x releases that are still within the support window (and particularly the LTS 7.19 release that many customers are still using).

Can you provide something in confluence-compat-lib that allows us to annotate the relevant methods and still work with these older Confluence versions?

Scott

UPDATE: it looks like this class may still be provided in Conf 7.x, albeit in a different JAR (atlassian-xwork-core-*.jar), but with the same package. So this should still work, right?

1 Like
10

Hi @Kusal,

We don’t typically maintain separate JARs for our Confluence plugins. To change our dev practices to provide 2 different JARs for different Confluence versions would be a significant change for us.

In the past, Atlassian has made a single plugin JAR version possible because of the use of confluence-compat-lib and other things like backwards-compat amongst various parts of Confluence. Can we please continue that practice so that partners/vendors don’t have to start maintaining multiple JARs?

Thanks!

3 Likes
11

Hi @kusal,

I have one more “one more thing”. Do you know with whom can we escalate the issue of the continued lack of Confluence source access? Is this something you would feel comfortable helping vendors advocate for?

The inability of vendors to access Confluence sources for months on end is a significant impediment to our workflow. Although we can theoretically ask the support desk for a copy, this takes far longer than before…and the support desk is sometimes actively withholding source releases until such-and-such version comes out, with stymies vendors’ ability to study the needed code in the meantime.

The fact that breaking changes are getting tossed into point versions of the 8.5 catch-all release makes this even more necessary, since we need to see exactly which changes you are making to Confluence internals with regards to these new Struts changes, to ensure that our applications do not break.

I understand that the sources were originally being withheld due to an earlier critical security vulnerability, but this has since been released and patched. I would love to be able to get back to normal here.

Scott

4 Likes
12

That would require us to compile against 8.5.6 which would make it harder to support 8.0+ releases.
We try to support as many versions as possible and this would limit the range quite a bit. I suspect this affects other vendors in a similar way.

I understand that it makes sense to clean up the code base in major releases as much as possible, but I think providing this annotation in 9.x will make many vendor’s lives easier :slight_smile:

Another option could be to detect the annotation in such a way that it only compares the fully qualified classname and not annotation class equality. I believe something similar has been done for other annotations as well (e.g. the @HtmlSafe annotation for velocity comes to mind). This would allow us to simply keep the annotation in our own code base (or pull it in from the compat lib).

13

Hey @scott.dudley et all,

@Kusal raised this, so I wanted to take a moment to reply.

You can certainly raise it with the Marketplace Partners team, but the frank answer to this is that it is unlikely to have a significant impact (though certainly can’t hurt). The removal of source code isn’t just to do with the previous security incidents. You’ll also have seen we just released another fix for another critical security issue yesterday, so that’s an ongoing concern still anyway.

We are aware how badly thing hamstrings you, and we’ve been pushing on getting it resolved for a while. I’ll escalate it internally again and see where we end up with it.

In the meantime, when contacting Support, please feel free to point back to this response. You guys are not the concern we’re managing for, and short of any “do not release source code” directions, this can hopefully grease the wheels towards getting you a faster response. I’ve also raised this with Support directly to see what we can do.

Thanks for your understanding on this one. I know it’s a real challenge.

Thanks,
James Ponting
Engineering Manager - Confluence Data Center

1 Like
14

Hi @jponting,

Thank you so much for the candid response. From the absence of any “sources will be put back in place by such-and-such date” message, I infer that there is not yet any expected timeline to restore the service.

You mentioned that you were also pushing to resolve and escalate the issue internally on your side. Is it possible to share with us which team this is getting escalated to (or who is the ultimate decision-maker)? I assume Security? So long as vendors are asking the TPMs to work on this, I would love to be able to direct them to the right place.

Thanks!
Scott

15

Hi all,

Thank you for the feedback on compatibility - we will reevaluate our approach in Confluence 9.0.

@scott.dudley

8.8 EAP2 is yet to be designated - the OGNL allowlist capability is enabled from 8.8.0-m56.

With respect to the annotation requirement, there are still a number of core Confluence actions which are yet to support it and so you will likely run into fatal errors if you were to enable it. I expect Friday’s milestone to have this feature enabled.

Yes com.atlassian.xwork.ParameterSafe is available from before 7.x and will be compatible with 8.8.

16

Unfortunately we don’t have a timeline we share at this point, though I can say the conversation is actively ongoing. I’ve mentioned this conversation to the appropriate teams.

There are multiple stakeholders on this one and I know the TPM team are aware of the conversation, so I don’t think they’ll need guidance on where to direct your concerns.

Support are looking at their approvals process to make it a little easier on everyone, so hopefully that’ll make things a tad less painful.

Again, thanks for bearing with us.

Thanks,
James Ponting
Engineering Manager - Confluence Data Center

2 Likes
17

Note that the backport of the OGNL Class Allowlist capability has been pushed back until Confluence 8.5.7.

18

For the dropdown values, after saving it we see the values as blank , any idea…rest of the fields are fine after keeping @ParamerSafe annotation, note that the dropdown fields are changes recently according to 8.7.2 compatibility of #s

19

Hi Kusal,
With respect to your note *
Whilst I haven’t tested this, it’s potentially possible to have your own copy of the
@StrutsParameter class in provided scope for compilation, and then define an optional OSGi
import to ensure the correct variant of the class is recognised during runtime.

I have a requriement to compile my application to have a single jar deployment in 8.8 and prior versions of confluence.My application has nested objects with depth 3 and hence I have tried your above said suggestion of having copy of strutsparameter.The below is sample of how I am placing the getter

@StrutsParameter(depth=3)
public void getMyDto(MyDto myDtol) {
    this.myDto = myDtol;
}

These are my observations

  1. Own copy of StrutsParameter is working fine 7.x but not in 8.8 ( In 8.8 it is not recognising @StrutsParameter annotation though it is mentioned , it still emits in log that the method should be annotated with strutsParameter with depth mentioned)

I also tried by including atlassian StrutsParameter as dependency

  1. Having Provided scope of StrutsParameter dependency is working fine in 8.8 but not 7.x as no class will be found in 7.x version

  2. Having compiled scope is working fine in 7.x but not in 8.8 .As in 8.b it is not recognising StrutsParameter annotation (same as mentioned point 1)

In all the above 3 cases, I have included these in import-pacakges
org.apache.struts2.*;resolution:=“optional”,
org.apache.struts2.interceptor.parameter.StrutsParameter;resolution:=“optional”

Can you please shed some light on how to solve this ?

20

Hi @Kusal,

Can you confirm if this was indeed shipped in 8.5.7? I’m looking for confirmation because I do not see it mentioned in the release notes.

Thanks!