[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

jeffret-b · 2020-01-29T23:02:05Z

See JENKINS-60913 for an explanation of the motivation. As discussed in Jenkins Security Advisory 2020-01-29 these services are vulnerable to SECURITY-1641 / CVE-2020-2100. The security release disables these protocols but they should be completely removed to finish the clean up.

I can post in the User and Dev lists to see if anyone objects to this removal.

Proposed changelog entries

Remove network discovery services (UDP and DNS).

Proposed upgrade guidelines

Network discovery features, DNS multicast and UDP broadcast, were previously disabled and discouraged because of various problems including Jenkins Security Advisory 2020-01-29. They have now been removed. There is no replacement.

Swarm Plugin needs to be updated to version 3.18 or above, otherwise the error described in JENKINS-61029 occurs. This also removes the network discovery capability of the plugin.

Submitter checklist

JIRA issue is well described
Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
- Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
Appropriate autotests or explanation to why this change has no tests
No tests needed for removed capability.
[n/a] For dependency updates: links to external changelogs and, if possible, full diffs

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least 2 approvals for the pull request and no outstanding requests for change
Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
Changelog entries in the PR title and/or Proposed changelog entries are correct
Proper changelog labels are set so that the changelog can be generated automatically
If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
If it would make sense to backport the change to LTS, a JIRA issue should exist and be labeled as lts-candidate

Doesn't need to be referenced in the other pom any more. We need to keep the class and the property around for a while. Jenkins Test Harness sets the property. Until we can get all versions aligned so it isn't needed, it's best to keep the property doing nothing.

res0nance

If no one uses it that's fine, I think maybe a post in both lists will help in case there is some other usage of it that we are not aware of (Ideally not).

Perhaps an upgrade guide would be needed so users were aware.

alecharp

I'm totally for removing something we shouldn't be using. As we already have a release out there with this deactivated, removing it is fine.

For sure, we will need a message on both mailing and maybe a blog post.

jvz

LGTM. Are you able to remove any dependencies, too?

core/src/main/java/hudson/DNSMultiCast.java

jeffret-b · 2020-01-30T16:10:37Z

For sure, we will need a message on both mailing and maybe a blog post.

I'm preparing a message to the email lists. I'm not so sure about a blog post, though. It doesn't make much sense to blog about a proposal. I'm not sure there's enough here for an announcement post. Not sure we have a really good place for announcing things like this other than the changelog and email lists.

jeffret-b · 2020-01-30T16:23:51Z

Are you able to remove any dependencies, too?

Good catch, @jvz. I was able to remove one.

jeffret-b · 2020-01-30T17:25:13Z

Removal proposal email sent to users and dev email lists.

core/src/main/java/hudson/DNSMultiCast.java

jglick · 2020-01-30T21:52:07Z

core/src/main/java/hudson/UDPBroadcastFragment.java

- * @since 1.304
- * @see UDPBroadcastThread
- */
-public abstract class UDPBroadcastFragment implements ExtensionPoint {


Still in use apparently:

https://github.com/jenkinsci/swarm-plugin/blob/fbf7b55a665a1439f8bb9471336300381c097948/plugin/src/main/java/hudson/plugins/swarm/UDPFragmentImpl.java#L14

If swarm needs it, then this sounds like an argument for detaching to a plugin…or moving to the swarm plugin.

@basil?

I had forgotten that I had seen a usage in the swarm plugin ages ago. That's one area that makes some sense. On the other hand, keep in mind that we announced a security vulnerability in these services yesterday. See SECURITY-1641 / CVE-2020-2100.

If swarm needs this service then it inherently provides this vulnerability. With the proper configuration and environment this may not be an issue, but this would need to be well documented.

I'm open to helping transfer it to swarm or to a plugin but not to maintaining that. Anywhere it ends up needs to take responsibility for the security concern.

So, yes, @basil, what do you think? Or anyone else who has a need for this?

core/src/main/java/hudson/DNSMultiCast.java

Co-Authored-By: Jesse Glick <[email protected]>

oleg-nenashev

+1 for moving it out of the core. There is an older ticket for it IIRC, will find it

oleg-nenashev · 2020-02-01T07:12:35Z

core/src/main/java/hudson/DNSMultiCast.java

 */
+@Deprecated


Add Restricted(DoNotUse) to fail builds?

I couldn't get it to work without failing the Jenkins build. We could use NoExternalUse, though.

jenkinsci/lib-access-modifier#17

timja · 2020-02-04T07:33:44Z

FYI: small conflict after merging disabled of the test on master

daniel-beck

I think it would make sense, and be straightforward enough to do, to add an @Initializer that checks the value of the deprecated fields or system properties, and if they indicate an explicitly "enabled" kind of value (true, or any actual port number, respectively), to print a warning.

(Explicitly disabled is fine, there's no difference in expected and actual behavior, and no urgency to remove the option.)

Doesn't need to be referenced in the other pom any more. We need to keep the class and the property around for a while. Jenkins Test Harness sets the property. Until we can get all versions aligned so it isn't needed, it's best to keep the property doing nothing.

daniel-beck

LGTM 👍

🚢🇮🇹

Added suggestions for links but I'm not insisting they need to be added. Just not seeing a downside, even if the linked docs isn't super relevant (at first). I'd be happy to provide the corresponding jenkins.io change to ensure they're not a dead end if the changes are accepted.

core/src/main/java/hudson/DNSMultiCast.java

core/src/main/java/hudson/UDPBroadcastThread.java

oleg-nenashev · 2020-02-06T12:36:03Z

@jeffret-b please prepare the upgrade guidelines so that I can merge it

daniel-beck · 2020-02-06T12:43:42Z

It's a straightforward "These features, which have been disabled before, are now removed, and there is no replacement". Would not let that block merging IMO.

oleg-nenashev · 2020-02-06T12:57:01Z

Ok
I plan to merge it tomorrow if no negative feedback

Co-Authored-By: Daniel Beck <[email protected]>

jeffret-b · 2020-02-06T15:43:00Z

I added upgrade guidelines to the original description.

oleg-nenashev · 2020-02-07T00:06:12Z

Thanks @jeffret-b !

The class was removed with f10d9f9 ("[JENKINS-60913] Remove network discovery services (jenkinsci#4460)") but is required by the swarm client. As result, swarm clients are unable use Jenkins: java.lang.NoClassDefFoundError: hudson/UDPBroadcastFragment at hudson.plugins.swarm.PluginImpl.getSwarmSecret(PluginImpl.java:262) This commit re-adds hudson.UDPBroadcastFragment to allow swarm client to the use the master again and hence fixes JENKINS-61029.

The class was removed with f10d9f9 ("[JENKINS-60913] Remove network discovery services (jenkinsci#4460)") but is required by the swarm client. As result, swarm clients are unable use Jenkins: java.lang.NoClassDefFoundError: hudson/UDPBroadcastFragment at hudson.plugins.swarm.PluginImpl.getSwarmSecret(PluginImpl.java:262) This commit re-adds hudson.UDPBroadcastFragment to jenkin's core, to allow swarm client to the use the master again and hence fixes JENKINS-61029.

oleg-nenashev · 2020-02-10T12:55:12Z

The change caused a regression in Swarm Plugin clients: https://issues.jenkins-ci.org/browse/JENKINS-61029 . We should ensure that somebody does a binary compatibility double-check when reviewing such pull requests. I will try to add it to the maintainer guide

timja · 2020-02-10T13:07:06Z

It was called out here @oleg-nenashev #4460 (comment)

oleg-nenashev · 2020-02-10T13:12:36Z

Yeah, and I totally missed this comment 🤦‍♂

jeffret-b · 2020-02-10T17:32:02Z

Yeah, there was a discussion about it here and on one of the email lists. The discussion here wasn't marked as resolved so it is still readily visible.

This PR is interesting, because it had a lot of approvals and no disapprovals. We had some discussion about people who might want to be concerned about it, but didn't get any response from anyone who actually was.

Perhaps we should have placed this on-hold for a time-limited duration to give people more opportunity to express concerns or prepare, but I really don't see that additional time would have likely yielded different responses. We don't want to leave things open forever, delaying something that so many support hoping that someone will eventually respond. Maybe we need to improve that process but we want to make sure not to make it too difficult to move Jenkins forward.

oleg-nenashev · 2020-02-12T12:01:50Z

I added Swarm Plugin reference to the upgrade guide draft. We still need a release of jenkinsci/swarm-plugin#179 to know the exact required version

basil · 2020-02-13T01:55:57Z

We still need a release of jenkinsci/swarm-plugin#179 to know the exact required version

I am currently working on a release. As I explained in jenkinsci/swarm-plugin#179 (comment), I still need to finish jenkinsci/swarm-plugin#182 and do some additional testing to understand (and document!) the impact for users with regard to backwards compatibility. As I am a bit short on time these days, any help would be appreciated, but otherwise I should be able to get to this by the end of the week.

basil · 2020-02-13T17:26:05Z

I am currently working on a release.

I just released Swarm 3.18.

jeffret-b · 2020-02-13T17:39:04Z

Yay! I'm glad you were able to get that done.

I reviewed jenkinsci/jenkins#4460 and found no links that were not already included in the network discovery text. I thought a link to the 2.204.2 upgrade guide might be helpful to some readers, but it is misleading because it indicates that there is an 'escape hatch', while this change intentionally removes the 'escape hatch'.

jeffret-b added 2 commits January 29, 2020 13:26

Remove multicast discovery services.

9afc63f

jeffret-b requested review from jvz, daniel-beck and Wadeck January 29, 2020 23:02

res0nance approved these changes Jan 30, 2020

View reviewed changes

alecharp approved these changes Jan 30, 2020

View reviewed changes

jvz approved these changes Jan 30, 2020

View reviewed changes

core/src/main/java/hudson/DNSMultiCast.java Show resolved Hide resolved

Deprecate remaining class fragment.

43096a9

Remove unneeded dependency.

684fbbd

halkeye approved these changes Jan 30, 2020

View reviewed changes

timja approved these changes Jan 30, 2020

View reviewed changes

jglick reviewed Jan 30, 2020

View reviewed changes

core/src/main/java/hudson/DNSMultiCast.java Outdated Show resolved Hide resolved

core/src/main/java/hudson/DNSMultiCast.java Show resolved Hide resolved

Improve code comments for external usages.

c7afc8a

Co-Authored-By: Jesse Glick <[email protected]>

res0nance added the upgrade-guide-needed This changes might be breaking in rare circumstances, an entry in the LTS upgrade guide is needed label Feb 1, 2020

oleg-nenashev added the major-rfe For changelog: Major enhancement. Will be highlighted on the top label Feb 1, 2020

oleg-nenashev approved these changes Feb 1, 2020

View reviewed changes

Wadeck mentioned this pull request Feb 2, 2020

Disable failing test in UDP broadcast #4467

Merged

markjacksonfishing approved these changes Feb 2, 2020

View reviewed changes

daniel-beck reviewed Feb 4, 2020

View reviewed changes

jeffret-b added 5 commits February 5, 2020 10:48

Remove multicast discovery services.

4274291

Deprecate remaining class fragment.

d18c7b5

Remove unneeded dependency.

5afcead

Add log warning messages.

8fec924

varyvol approved these changes Feb 6, 2020

View reviewed changes

timja requested a review from daniel-beck February 6, 2020 10:11

daniel-beck approved these changes Feb 6, 2020

View reviewed changes

core/src/main/java/hudson/DNSMultiCast.java Outdated Show resolved Hide resolved

core/src/main/java/hudson/UDPBroadcastThread.java Outdated Show resolved Hide resolved

oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Feb 6, 2020

Add URLs to log messages.

4b471a1

Co-Authored-By: Daniel Beck <[email protected]>

oleg-nenashev merged commit f10d9f9 into jenkinsci:master Feb 7, 2020

daniel-beck mentioned this pull request Feb 9, 2020

Add redirect URLs for removed network discovery features jenkins-infra/jenkins.io#2842

Merged

oleg-nenashev changed the title ~~[JENKINS-60913] Remove network discovery services~~ [JENKINS-33596, JENKINS-60913] Remove network discovery services Feb 10, 2020

Flowdalic mentioned this pull request Feb 10, 2020

[JENKINS-61029] - Re-add hudson.UDPBroadcastFragment #4486

Closed

oleg-nenashev mentioned this pull request Feb 17, 2020

Jenkins 2.220 changelog: Add a banner warning for Swarm Plugin users jenkins-infra/jenkins.io#2866

Merged

jglick mentioned this pull request Sep 10, 2021

[JENKINS-33596] Delete more of UDPBroadcastThread & DNSMultiCast #5720

Merged

6 tasks

This was referenced Mar 13, 2023

Remove unused hudson.udp system property jenkinsci/plugin-pom#719

Merged

Remove unused hudson.udp property jenkinsci/gerrit-trigger-plugin#486

Merged

Remove unused hudson.udp property jenkinsci/blueocean-plugin#2413

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

jeffret-b commented Jan 29, 2020 •

edited by daniel-beck

Loading

res0nance left a comment •

edited

Loading

alecharp left a comment

jvz left a comment

jeffret-b commented Jan 30, 2020

jeffret-b commented Jan 30, 2020

jeffret-b commented Jan 30, 2020

jglick Jan 30, 2020

jeffret-b Jan 30, 2020

oleg-nenashev left a comment

oleg-nenashev Feb 1, 2020

jeffret-b Feb 3, 2020

jglick Feb 3, 2020

timja commented Feb 4, 2020

daniel-beck left a comment

daniel-beck left a comment

oleg-nenashev commented Feb 6, 2020

daniel-beck commented Feb 6, 2020

oleg-nenashev commented Feb 6, 2020

jeffret-b commented Feb 6, 2020

oleg-nenashev commented Feb 7, 2020

oleg-nenashev commented Feb 10, 2020

timja commented Feb 10, 2020

oleg-nenashev commented Feb 10, 2020

jeffret-b commented Feb 10, 2020

oleg-nenashev commented Feb 12, 2020

basil commented Feb 13, 2020

basil commented Feb 13, 2020

jeffret-b commented Feb 13, 2020

		*/
		@Deprecated

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

Conversation

jeffret-b commented Jan 29, 2020 • edited by daniel-beck Loading

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Maintainer checklist

res0nance left a comment • edited Loading

Choose a reason for hiding this comment

alecharp left a comment

Choose a reason for hiding this comment

jvz left a comment

Choose a reason for hiding this comment

jeffret-b commented Jan 30, 2020

jeffret-b commented Jan 30, 2020

jeffret-b commented Jan 30, 2020

jglick Jan 30, 2020

Choose a reason for hiding this comment

jeffret-b Jan 30, 2020

Choose a reason for hiding this comment

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev Feb 1, 2020

Choose a reason for hiding this comment

jeffret-b Feb 3, 2020

Choose a reason for hiding this comment

jglick Feb 3, 2020

Choose a reason for hiding this comment

timja commented Feb 4, 2020

daniel-beck left a comment

Choose a reason for hiding this comment

daniel-beck left a comment

Choose a reason for hiding this comment

oleg-nenashev commented Feb 6, 2020

daniel-beck commented Feb 6, 2020

oleg-nenashev commented Feb 6, 2020

jeffret-b commented Feb 6, 2020

oleg-nenashev commented Feb 7, 2020

oleg-nenashev commented Feb 10, 2020

timja commented Feb 10, 2020

oleg-nenashev commented Feb 10, 2020

jeffret-b commented Feb 10, 2020

oleg-nenashev commented Feb 12, 2020

basil commented Feb 13, 2020

basil commented Feb 13, 2020

jeffret-b commented Feb 13, 2020

jeffret-b commented Jan 29, 2020 •

edited by daniel-beck

Loading

res0nance left a comment •

edited

Loading