Guidance on how far SECURITY fixes should go

100 views
Skip to first unread message

Stephen Connolly

unread,
Sep 15, 2015, 6:39:41 AM9/15/15
to jenkin...@googlegroups.com
Those of us who are on the jenkins-cert list have probably seen my
comments about how there needs to be some guidance from the community
on how far SECURITY issue fixes can go.

I am starting this thread to try and start compiling that guidance.

The general problem (as I cannot share the specifics outside of the
jenkins-cert list) is as follows:

* User raises a SECURITY issue from noticing some implicitly defined
API of Jenkins has a hole of some sort
* We have to fix the issue, but fixing it will involve changing the
implicitly defined API and thus risking breaking existing "users" of
the implicitly defined API
* Much heated debate around should we leave the old API present and
try and redirect to the new one (thereby leaving the hole open) or
break existing plugins and close the hole.

In some cases it is possible to use a system property to re-enable the
hole for those users who are accepting of the risk... but other cases
require changing class structures (this can range from encapsulating
fields to completely reworking the java class that gets returned)

My view is that a hole is a hole until it is fully plugged, and if
plugging the hole means that some plugins break, well sorry but
SECURITY... now by all means we should seek to minimize that breakage
*while closing the hole*... and where possible we should provide a
System property based switch to re-open the hole for those users who
upgrade and are ok with the risk.

Let's take a semi-concrete example:

In order to fix SECURITY-144 we had to change the interface of
h.r.Callable to include the role checking... being pre-java 8 we
cannot use default methods, so this was a backwards incompatible
change... any plugin that does not implement the role checking method
would potentially be broken... as a result - and after much debate -
it was decided that a majority of people implicitly trust their slaves
as much as the master and so the role checking is likely not required
for maybe 60-70% of installations. Thus the role checking ships off by
default and we implemented a way to whitelist plugins that are
compiled without role-checking support. Thus if you have slaves that
are less trusted than the master you can opt-in to the higher security
requirements... if there are plugins that you need that get broken,
you can white-list those specific plugins if you absolutely need to.

The point here is that we closed the hole. We felt the risk of plugins
being broken was sufficiently high that the hole would not be closed
by default (The plan is to measure the plugins with support for roles
and once above a threshold then turn on secure by default) but when
the security is turned on it is on.

So what kind of things do we need guidance on:

* Some methods of some objects allow you to discover information about
other objects that you do not have permission to discover.
- Do we just protect the ways that we know about and leave the
original method present with a @Restricted(DoNotUse.class) so that
when plugins upgrade their core version they are forced to switch to
the new method; or
- Do we change the contract of the current method (so it now
returns maybe a null or throws an exception or omits entries from a
collection) so that the method does what it was originally supposed to
do but didn't... this may break existing plugins in fun and exciting
ways

* Some objects have public fields that are either mutable or of a type
that needs to be changed in order to remove the hole.
- Do we leave the field as is and just introduce the new accessor
methods - leaves the hole as is... so we have to rely on adding a
stapler action method (since they are higher priority than public
fields) to mask that route to discovery via Web and hope that there
are no other routes to that field...
- Do we say kill that field, you shouldn't have been using it (oh
and let's hire a hitman to find the lazy developer who made a public
field rather than use their IDE to generate getters and setters) and
risk breaking any plugins that actually use the field.

* Some HTTP requests can be used for more than their original intended
purpose. Jenkins Admins may well have discovered this and - rather
than thinking oh look a security issue - used such back doors to do
important things for their instance
- Do we try and make such back-door usage as safe as possible -
which in a sense makes this side-usage officially part of the API of
that HTTP request
- Do we lock the HTTP request down to its original intended
purpose and break any side-effect usage that Jenkins Admins were
hijacking?

There are probably other classes of guidance required, but my brain is
melted trying to derive the classes from the issues I am aware of so I
cannot think of any others at the moment.

It all boils down to the question:

Which is more important, maintaining backwards compatibility or fixing
the security hole.

My view is that fixing the security hole trumps backwards
compatibility, if we have to break a few plugins or a Jenkins admin
has to find a different way to solve their problem... well sorry but
SECURITY... that doesn't mean we should start by breaking backwards
compatibility, rather we should do everything we can to close the hole
while maintaining backwards compatibility... but when that isn't
enough to close the hole (and usually it is not enough BTW)... well we
need to close the hole.

Over to you!

oliver gondža

unread,
Sep 15, 2015, 6:59:19 AM9/15/15
to jenkin...@googlegroups.com
On Tue, 15 Sep 2015 12:39:36 +0200, Stephen Connolly
<stephen.al...@gmail.com> wrote:

> My view is that fixing the security hole trumps backwards
> compatibility,

Personally I disagree, especially as SECURITY issues are merged into LTS
as they are now. LTS users choose stability and their vulnerabilities as
disclosed as rapidly as those of weekly release users. I do not think it
provides enough time to evaluate possible impact of such invasive fixes.

I lean towards preserving backward compatibility (in SECURITY fix) or at
least making the fix optional if breakage is inevitable.

--
oliver

Stephen Connolly

unread,
Sep 15, 2015, 8:04:54 AM9/15/15
to jenkin...@googlegroups.com
oliver IIUC you are on the cert list, there are two specific examples
that are guiding my thinking at present. I will send the references
out of band.

The security fixes are typically prepared in advance for this reason
and pushed to every branch at the same time to minimize exposure. If
we do not do the concurrent releases then LTS becomes vulnerable.
> --
> You received this message because you are subscribed to the Google Groups
> "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jenkinsci-de...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/jenkinsci-dev/op.x4z3g0gjsbfict%40arch.
> For more options, visit https://groups.google.com/d/optout.

ogondza

unread,
Sep 15, 2015, 8:44:59 AM9/15/15
to Jenkins Developers
On Tuesday, September 15, 2015 at 2:04:54 PM UTC+2, Stephen Connolly wrote:

The security fixes are typically prepared in advance for this reason
and pushed to every branch at the same time to minimize exposure. If
we do not do the concurrent releases then LTS becomes vulnerable.

I understand that, though it is enough we dictate LTS users when to update (even if there is no other way, they are hardly happy about that). If those users can not rely there will be no breaking changes (even within one LTS line) it will hurt stability (because of the breakage we may caused) and security (as admins can postpone the upgrade because of the potential of breakage) IMO.

What is the other side of the coin here? The fixes will be smaller and simpler for sure if we are allowed not to preserve BC in 100% of cases. Anything else?

--
oliver

Jesse Glick

unread,
Sep 15, 2015, 8:53:11 AM9/15/15
to Jenkins Dev
On Tue, Sep 15, 2015 at 6:39 AM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> In some cases it is possible to use a system property to re-enable the hole

Generally I would really push back on introducing alternate behaviors
using a system property, especially something important like this.

I think it is worth considering having a “risk slider” setting in
Jenkins configuration:

1. My instance has no security, whatever, stop asking me questions.
2. I have some basic login authentication, but am not worried about
concerted attacks.
3. OK, defend against the obvious stuff like CSRF attacks, but do not
get too exotic.
4. Please do not keep any secrets on local disk. My build slaves are
not trusted. Users must be compartmentalized and never see information
about other teams.
5. I work for the government. I cannot discuss details.

Just a simple sliding scale, and then we can infer various other
security settings based on this. Obviously when we can defend against
something with no downside, we just do it, but this master setting
would help us decide when to make certain compromises.

Recent versions of the JRE do this, FWIW.

> My view is that a hole is a hole until it is fully plugged, and if
> plugging the hole means that some plugins break, well sorry but
> SECURITY

Generally agree.

> The point here is that we closed [SECURITY-144]. We felt the risk of plugins
> being broken was sufficiently high that the hole would not be closed
> by default (The plan is to measure the plugins with support for roles
> and once above a threshold then turn on secure by default) but when
> the security is turned on it is on.

FWIW I am not aware of any actual regressions from this delivered fix
and I still think we should have just enabled it by default. And is
anyone actually tracking follow-up tasks from this? AFAIK we shipped
and then it just dropped off the radar.

> Do we just protect the ways that we know about and leave the
> original method present with a @Restricted(DoNotUse.class) so that
> when plugins upgrade their core version they are forced to switch to
> the new method

+1 on this approach when feasible.

> Do we leave the field as is and just introduce the new accessor methods

I think so, combined with `@Restricted`.

> Do we lock the HTTP request down to its original intended
> purpose and break any side-effect usage that Jenkins Admins were
> hijacking?

IMO yes.

Jesse Glick

unread,
Sep 15, 2015, 8:56:01 AM9/15/15
to Jenkins Dev
On Tue, Sep 15, 2015 at 8:44 AM, ogondza <ogo...@gmail.com> wrote:
> If those users can not rely there will be no breaking changes (even within one LTS line)
> it will hurt stability (because of the breakage we may caused)
> and security (as admins can postpone the upgrade because of the potential of breakage)

Would it help to do separate security vs. general bug fix LTS
releases, for example using odd vs. even micro numbers? Oracle’s Java
team started doing this and I think it makes some sense.

Stephen Connolly

unread,
Sep 15, 2015, 9:00:54 AM9/15/15
to jenkin...@googlegroups.com
Well in my view, if we do not have the break BC escape hatch we may
not be able to fix some of the issues at all.

If you have a method that returns a collection of things and then you
change the contract so that it now only returns the subset of things
that the current authentication is allowed to know about... that is
breaking BC in the strict sense, but it is the only way to fix a
security hole of the `XYZ.getWidgets() returns all the Widgets even
the ones that the user does not have permission to see` variety.
Similarly a `XYZ. getWidget(name)` may have to return `null` for a
Widget `name` that exists but the user does not have permission to
access. There are plugins that may break because of this behaviour...
for example a plugin might assume that `XYZ. getWidget(name)==null`
implies that the widget does not exist and thus
`XYZ.addWidget(name,widget)` will succeed...

So the fix I would prefer to see in the above example is that
`XYZ.getWidgets()` returns the subset that the user has permission to
see, `XYZ.getWidget(name)` returns null both for widgets that do not
exist and widgets that the user does not have permission to see and
`XYZ.addWidget(name,widget)` either does a no-op or throws an
exception when trying to replace a widget that you do not have
permission to access... all those changes risk breaking plugins...

If we don't do that fix, then what we have is `XYZ.getWidgets()`
returning all widgets, we mark that as `@Restricted(DoNotUse.class)`
and add `XYZ.getUserVisibleWidgets()` hoping that the downstream
plugins update and get the fix... until then we have just pushed the
SECURITY-XYZ issue into 5 SECURITY-... issues in those five plugins
that used `XYZ.getWidgets()`... that is not what core should be doing
IMHO... it doesn't make sense to trade one SECURITY issue in core for
5 SECURITY issues in plugins... better is to trade one SECURITY issue
in core for 5 JENKINS issues in plugins... that lets a wider group of
people work on fixing those bugs
> --
> You received this message because you are subscribed to the Google Groups
> "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jenkinsci-de...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/jenkinsci-dev/a2b2e18e-41d2-4c29-a1ce-03392a5b8509%40googlegroups.com.

Stephen Connolly

unread,
Sep 15, 2015, 9:12:28 AM9/15/15
to jenkin...@googlegroups.com
On 15 September 2015 at 13:53, Jesse Glick <jgl...@cloudbees.com> wrote:
> On Tue, Sep 15, 2015 at 6:39 AM, Stephen Connolly
> <stephen.al...@gmail.com> wrote:
>> In some cases it is possible to use a system property to re-enable the hole
>
> Generally I would really push back on introducing alternate behaviors
> using a system property, especially something important like this.
>
> I think it is worth considering having a “risk slider” setting in
> Jenkins configuration:
>
> 1. My instance has no security, whatever, stop asking me questions.
> 2. I have some basic login authentication, but am not worried about
> concerted attacks.
> 3. OK, defend against the obvious stuff like CSRF attacks, but do not
> get too exotic.
> 4. Please do not keep any secrets on local disk. My build slaves are
> not trusted. Users must be compartmentalized and never see information
> about other teams.
> 5. I work for the government. I cannot discuss details.
>
> Just a simple sliding scale, and then we can infer various other
> security settings based on this. Obviously when we can defend against
> something with no downside, we just do it, but this master setting
> would help us decide when to make certain compromises.
>
> Recent versions of the JRE do this, FWIW.

I like this model... we would need to build guidance for assessing
security issues against that categorization

>
>> My view is that a hole is a hole until it is fully plugged, and if
>> plugging the hole means that some plugins break, well sorry but
>> SECURITY
>
> Generally agree.
>
>> The point here is that we closed [SECURITY-144]. We felt the risk of plugins
>> being broken was sufficiently high that the hole would not be closed
>> by default (The plan is to measure the plugins with support for roles
>> and once above a threshold then turn on secure by default) but when
>> the security is turned on it is on.
>
> FWIW I am not aware of any actual regressions from this delivered fix
> and I still think we should have just enabled it by default. And is
> anyone actually tracking follow-up tasks from this? AFAIK we shipped
> and then it just dropped off the radar.

Yeah I know (I am a sad panda)

>
>> Do we just protect the ways that we know about and leave the
>> original method present with a @Restricted(DoNotUse.class) so that
>> when plugins upgrade their core version they are forced to switch to
>> the new method
>
> +1 on this approach when feasible.

The problem I see is that we have just transformed one SECURITY-...
issue against core into N x SECURITY-... issues against plugins that
use the old method... some of them may not be exposing the issue in
their use of the method, but each and every one needs to be
assessed... given the extra work involved in handling SECURITY issues
(needing CVEs etc, reduced number of collaborators, etc) I think it is
actually better if the downstream side-effects are just regular bugs.
So fixing the existing method - even if that risks breaking plugins -
is IMHO better than the `@Restricted(DoNotUse.class)` model... *BUT*
we'd still need to go and create all those downstream JENKINS issues
as part of the SECURITY fix... If we (as in the people on the
jenkins-cert list) do not create the downstream tracking bugs for the
side-effects of breaking BC then we are in just the same place as if
we didn't create the downstream SECURITY issues to investigate each
plugin's use of the old method and replace it with the new method as
appropriate

>
>> Do we leave the field as is and just introduce the new accessor methods
>
> I think so, combined with `@Restricted`.

Same 1 -> N issue multiplication concern

>
>> Do we lock the HTTP request down to its original intended
>> purpose and break any side-effect usage that Jenkins Admins were
>> hijacking?
>
> IMO yes.
>
> --
> You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr1c%3DN8kkdZexpnt8VNLMVQ9tr7P6%2Bo3JVn03n0JqSxGWw%40mail.gmail.com.

Oleg Nenashev

unread,
Sep 15, 2015, 11:32:52 AM9/15/15
to Jenkins Developers
I think it is worth considering having a “risk slider” setting in 
Jenkins configuration: 

+1, but probably it's hard to manage so many security modes. I would propose to add a single "SECURITY.PARANOID" mode, which would be available through system properties or the global security page. 

FWIW I am not aware of any actual regressions from this delivered fix 
 and I still think we should have just enabled it by default

+1 if we have an opportunity to disable the security fixes on the per-issue basis. In such case the admins would have an option to temporary fix the regressions on their own risk.

If we (as in the people on the 
jenkins-cert list) do not create the downstream tracking bugs for the 
side-effects of breaking BC then we are in just the same place as if 
we didn't create the downstream SECURITY issues to investigate each 
plugin's use of the old method and replace it with the new method as 
appropriate  ... Same 1 -> N issue multiplication concern 

In several pending PRs N will be too big (see the parallel jenkinsci-cert thread if you have an access). I doubt we can cover all usage cases even in OSS and to create follow-up issues in plugins.

вторник, 15 сентября 2015 г., 16:12:28 UTC+3 пользователь Stephen Connolly написал:

Suckow, Thomas J

unread,
Sep 15, 2015, 12:14:16 PM9/15/15
to jenkin...@googlegroups.com
What about a screen in Jenkins Admin that lists all the "breaking"
security fixes since the release of the LTS. By default after an update
any new security fixes are in "Unacknowledged mode". Having any
unacknowledged items adds a banner to the entire application.

You can then either "Accept the risk" and leave it unfixed or plug the
hole understanding that it might break christmas. If you are really fancy
you can track the callstacks of users of the api.

Pros:
* User consciously acts on breaking changes.
* User can probably determine outdated plugins and bug them to update.

Cons:
* A lot of work for the Core Jenkins team
* Something will inevitably happen where this pattern won't work


Just a thought. Though I don't use LTS and am used to Jenkins breaking on
a weekly basis because the world hates me.

-Thomas

On 9/15/15, 3:39 AM, "jenkin...@googlegroups.com on behalf of Stephen
Connolly" <jenkin...@googlegroups.com on behalf of
>--
>You received this message because you are subscribed to the Google Groups
>"Jenkins Developers" group.
>To unsubscribe from this group and stop receiving emails from it, send an
>email to jenkinsci-de...@googlegroups.com.
>To view this discussion on the web visit
>https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMw6rhFBDP-jjqaaNN8
>YZoxT1kqGUXR1F9hvDT6s%2B01_Bw%40mail.gmail.com.

Jesse Glick

unread,
Sep 15, 2015, 2:31:39 PM9/15/15
to Jenkins Dev
On Tue, Sep 15, 2015 at 12:06 PM, Suckow, Thomas J
<Thomas...@pnnl.gov> wrote:
> Cons:
> * A lot of work for the Core Jenkins team

Yeah, it is a nice idea, I just wonder if the developers available to
work on security fixes have the time to do that much polish.

Daniel Beck

unread,
Sep 15, 2015, 3:53:20 PM9/15/15
to jenkin...@googlegroups.com

On 15.09.2015, at 14:53, Jesse Glick <jgl...@cloudbees.com> wrote:

>> In some cases it is possible to use a system property to re-enable the hole
>
> Generally I would really push back on introducing alternate behaviors
> using a system property, especially something important like this.

We currently claim no compatibility guarantee for features controlled by system properties.

So we could introduce these with a predetermined expiration date to allow users to upgrade and retain their existing functionality, for a time (e.g. twelve weeks/one LTS cycle).

Jesse Glick

unread,
Sep 15, 2015, 5:14:06 PM9/15/15
to Jenkins Dev
On Tue, Sep 15, 2015 at 3:53 PM, Daniel Beck <m...@beckweb.net> wrote:
> we could introduce [system properties] with a predetermined expiration date to allow users to upgrade and retain their existing functionality, for a time

This could be a good solution for cases where we have prepared patches
to fix client plugins to behave properly in the face of occasionally
incompatible core behavior changes, but need some extra time to get
those patches accepted by maintainers and released.

Kohsuke Kawaguchi

unread,
Sep 15, 2015, 6:41:19 PM9/15/15
to jenkin...@googlegroups.com
I don't think everyone is basically agreeing and the contention points appear to be small, but we can't make blanket statements like "the security hole trumps backwards compatibility." It's obviously more nuanced than that. Generally we tried to keep both goals.

Fortunately, so far as I recall for most issues, we didn't have to break backward compatibility and in doing so we didn't feel like we had to make the fix "ugly." SECURITY-144 was a complicated example --- it added a new API (which we generally try to avoid in security fixes) but it was not a binary incompatible change.

The situation we need to avoid is to require a non-trivial number of plugins to be updated as a part of the core update. We cannot coordinate fixes in those plugins with core release due to the way security fixes are prepared, and users have no effective means to update them all in one.

There are also many users who don't care about the vulnerability the fix is addressing, and we want to make sure fixes are not affecting them.

And above all, if we end up creating a perception that security fixes are risky, that's very dangerous. And it's hard to win back that kind of trust once we lose them.

Those are some of the arguments why backward compatibility is important, and why we have to have damn good reasons if we are going to break them.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMw6rhFBDP-jjqaaNN8YZoxT1kqGUXR1F9hvDT6s%2B01_Bw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.



--
Kohsuke Kawaguchi

Oleg Nenashev

unread,
Sep 16, 2015, 4:06:27 AM9/16/15
to Jenkins Developers
What about a screen in Jenkins Admin that lists all the "breaking" 
security fixes since the release of the LTS. By default after an update 
any new security fixes are in "Unacknowledged mode". Having any 
unacknowledged items adds a banner to the entire application. 

Yes, it's reasonable. Since nobody else likes System properties, a global config seems to be the  the only way.

Yeah, it is a nice idea, I just wonder if the developers available to 
work on security fixes have the time to do that much polish.

The pending SECURITY pull requests have been created by CloudBees engineers. If the community decides that breaking changes cannot be accepted without Advanced SECURITY vulnerability management engine or whatever rocket-science thing, we can go back to our Product Management and discuss it (such implementation was out the initial scope).

My concern is that any UI-based implementation requires something very flexible in order to deliver security fixes into 1.625.x. I suppose CERT team agrees that the risks are important enough to be considered for the backporting.
  • I would vote for a new Restricted extension point in the core + a new bundled plugin, which provides the configurable proposed by Thomas.
  • I bet somebody will shut down the bundled plugin idea immediately

среда, 16 сентября 2015 г., 1:41:19 UTC+3 пользователь Kohsuke Kawaguchi написал:

Nigel Magnay

unread,
Sep 16, 2015, 6:58:20 AM9/16/15
to jenkin...@googlegroups.com
From the perspective of a plugin developer and Jenkins Instance owner, who often goes months at a time between re-visiting plugin code, I'd like the following (ymmv, of course!):

1) I'd like the interfaces I build to, to not have at all interface methods and fields that are no longer secure / broken. I.E: If I upgrade my POM to point to a 'new' version of Jenkins 1.629, I'd like the compile to fail if I'm using something that is broken, security or otherwise.

Having @Restricted and @Deprecated annotations all over the place just means I'm likely never to even notice, let alone actually get around to doing the work to update it. Looking at the API becomes a total mess of ancient cruft. Every time I see "@deprecated" I just think "oh yeah.. when?"

2) I don't want to have to touch plugins when updating an LTS version, and I don't want LTS versions to break 'just because of security'. I'm firewalled, and frankly couldn't care less about security - it's just a gigantic ache for me that I'd like a big red button marked 'OFF'.

Thus:

Could we have a mechanism where Jenkins maintains all the old APIs in the binary (I.E: allows plugins build against older versions of Jenkins to continue to function without change) - but you build against a 'stripped' version of the current, "correct" API?

In the example where the signature doesn't change, but the contract potentially does - if it's not reasonable to be able to turn off the security change (big-red-off), then I think the signature should change. The interface has been versioned.

At least this way you could probably have tight security on installs that need it (If jenkins is Version X, all plugins must be built to that version or later, and @Restricted methods can safely fail closing the back door). You could also use the jenkins version the plugin built against to control whether your API - by default - applies new semantics or old ones.



Stephen Connolly

unread,
Sep 16, 2015, 7:27:14 AM9/16/15
to jenkin...@googlegroups.com
Well @Restricted(DoNotUse.class) should give you a build time failure
if you are using the Maven toolchain... if you are using the groovy
toolchain I presume you can add the same checks...

What I think you are thus asking for is something like

@SecureAlternative(name="getFooSecure")
public Foo getFoo(...) {
...
}

public Foo getFooSecure(...) {
...
}

So that when the annotation is processed the annotated method gets the
equivalent injected:

if (BigFatSecuritySwitch.isActive()) {
return getFooSecure(...);
}

at the start

If we design the BigFatSecuritySwitch.isActive() method correctly, the
JVM should be able to inline all the way and remove the cost of the
branch point.

The downside that I see is that we end up in the nightmare of the
getInstance() -> getActiveInstance() transition (which should really
have been adding @NonNull to getInstance() and adding a
getInstanceUnchecked() method)

So perhaps an even better way would be

@UnsecureAlternative(name="getFooUnsafe", until="1.650")
public Foo getFoo(...) {
...
}

private Foo getFooUnsafe(...) {
...
}

With the above you inject (as long as Jenkins' pom.xml's version <
1.650) the following into getFoo()

if (!BigFatSecuritySwitch.isActive()) {
return getFooUnsafe(...);
}

We could then add an Admin monitor that looks at the until fields on
Jenkins and tells you if any of your plugins were compiled against a
version of Jenkins older than the until... similarly the plugin
manager could alert you for plugins that are compiled against an older
jenkins than your oldest until... (we could expose the same logic for
plugin dependencies so that plugins could use the @UnsecureAlternative
annotation also)

The advantage of @UnsecureAlternative is that we get to move the API
contracts while allowing people to flick the switch if they don't care

WDYT?
> https://groups.google.com/d/msgid/jenkinsci-dev/CAPYP83R8yd%3DD%3D%2BeQdoGVCgsOk4_YyuXn7K4ePqZe4DLdQhL%2Bxg%40mail.gmail.com.

Nigel Magnay

unread,
Sep 16, 2015, 8:58:44 AM9/16/15
to jenkin...@googlegroups.com
The annotation processing to deliver BigFatSecuritySwitch (or deferring to another method) is basically what I'd do.

Provided the build fails, I guess it doesn't really matter whether it's a method-not-found, or an enforcer-style rule (e.g: No calling @deprecated). Basically I *know* I'm lazy, and unless the toolchain basically forces me to fix something, I know I never will. My indolence then means it's hard to remove old, crufty things.

As to whether using annotations to both provide the machinery for API manipulation and the documentation / 'end-user API' for it I guess is a judgement call. Personally I rather prefer in my IDE if I type JenkinsThing<dot> it doesn't provide me with the history of the universe of methods with annotations my IDE won't understand. But I also appreciate that there are platform constraints (e.g: you can't tell which java interface a method was called through, meaning interface-versioning is quite hard) that mean it's hard to satisfy everyone.

I'll prepare the barricades before someone suggests OSGi or other similar self-abuse toolkits :-)


Jesse Glick

unread,
Sep 16, 2015, 10:02:54 AM9/16/15
to Jenkins Dev
On Wed, Sep 16, 2015 at 7:27 AM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> @UnsecureAlternative(name="getFooUnsafe", until="1.650")
> public Foo getFoo(...) {
> ...
> }

Too complicated, too reflective. Let us keep it to basic
statically-checked idioms as much as possible.

Anyway I am not convinced the actual problems at hand warrant this
much fuss, or any incompatibility at all, but it cannot be discussed
here.

Oleg Nenashev

unread,
Sep 18, 2015, 12:32:12 AM9/18/15
to Jenkins Developers
The annotation processing to deliver BigFatSecuritySwitch (or deferring to another method) is basically what I'd do.

It would be a simple way, but it does not allow a fine-grain control. The impact of the pending and potential fixes is too high, so many Jenkins installations may blow up.

Well @Restricted(DoNotUse.class) should give you a build time failure 
if you are using the Maven toolchain... if you are using the groovy 
toolchain I presume you can add the same checks... 
 
We could then add an Admin monitor that looks at the until fields on 

Jenkins and tells you if any of your plugins were compiled against a 
version of Jenkins older than the until... similarly the plugin 
manager could alert you for plugins that are compiled against an older 
jenkins than your oldest until...

I like the idea of a second unsafe method, but we will also need to notify users that a plugin uses unsafe API. Otherwise instance maintainers won't be aware that they are installing a plugin having a security risk.

There are following TODOs:
  • Just warn users about it in UC
  • Process the limitations in Script Security Plugin
  • [Optional] - Allow filtering out plugins with unsafe API by a global switch in UC
  • [Optional] - Process plugins during the load/installation procedure and add all kinds admin monitors
    • It is especially required to cover the use-case with manual plugin installation
I suspect the efforts would be quite big in any case.


среда, 16 сентября 2015 г., 17:02:54 UTC+3 пользователь Jesse Glick написал:
Reply all
Reply to author
Forward
0 new messages