Skip to content

Conversation

JavierEspina
Copy link
Collaborator

…eferences to future SDPi versions for maintainability

📑 Description

☑ Mandatory Tasks

The following aspects have been respected by the pull request assignee and at least one reviewer:

  • Changelog update (necessity checked and entry added or not added respectively)
    • [x ] Pull Request Assignee
    • Reviewer

…eferences to future SDPi versions for maintainability
@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Apr 11, 2025

@ToddCooper and @d-gregorczyk @PeterKranich , I think this PR implements all the changes described in the ticket. HOWEVER please check carefully that we are following a good approach to deprecate content (i.e. what to display as reserved)

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Apr 11, 2025

@d-gregorczyk @PeterKranich , I am not sure if we ever discussed what should happen to the "Bye" parts of DEV-46 and DEV-47. Should they also be withdrawn?

@JavierEspina
Copy link
Collaborator Author

SDPi call 11 April 2025:
Another question is if the "removal" notes in 2:A.2.8 MDPWS: Announce Network Departure [DEV-34] and 2:3.34 Announce Network Departure [DEV-34] should ALSO SAY that it is not recommended to use bye messages in Ad-hoc mode,... and that a CONSUMER should ignore them if he gets them.

@PeterKranich
Copy link
Collaborator

@d-gregorczyk @PeterKranich , I am not sure if we ever discussed what should happen to the "Bye" parts of DEV-46 and DEV-47. Should they also be withdrawn?

IMHO, a Bye message has some advantages but none of them are extremely important:

  • Consumer would be able to detect a graceful disconnect of the provider
  • Consumer and Discovery proxy may detect an intended disconnect fast rather than waiting for the timeouts

Altough, the bye message has a security issue with Ad-hoc discovery it is less critical in combination with a discovery proxy since the communiccation is encrypted.

I wouldn't mind not to support the bye message right now.

@ToddCooper
Copy link
Collaborator

ToddCooper commented Apr 15, 2025

@PeterKranich - So the question was a few parts:

  1. Include it as a supported (included) transaction in SDPi, perhaps changing it to OPTIONAL (currently it is marked as Required for SOMDS_PROVIDER actors) + Add a note (again to that same table) indicating that there are security issues and to see the MANAGED DISCOVERY profile option etc. + Add a note to the MDPWS section indicating the deprecation
  2. OR remove the transaction (NOT SUPPORTED) and add a note to MDPWS that it is NOT supported in the SDPi-P profile

My sense is that we wanted to do the first approach and not the second BUT add the documentation as indicated. YES?

@PaulMartinsen
Copy link
Collaborator

@PeterKranich , as I understood it the discussion was about about dropping the bye message for ad-hoc discovery only. Providers connecting to a discovery proxy would still send bye. I think the concern was any device on the network could send the bye message on behalf of another device if they choose (denial of service? ). Then the question became was dropping bye or making it optional sufficient? Do we need something like

  • Consumer's shall ignore any multicast bye message if they are configured to use managed discovery
  • Discovery proxies shall ignore any multicast by message.

I wonder if multicast hello is problematic too? Any device could send a hello claiming to be some end-point reference but providing a different IP address. Perhaps TLS for the rest of communications takes care of this apparent man-in-the-middle attack. But do we need consumers to ignore any multicast hello message if they are configured to used managed discovery? Potentially ad-hoc hello could be used just to bootstrap managed discovery along the lines shown in the ws-discovery spec:
image

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Apr 16, 2025

Great discussion (that keeps widening up!)
Regarding Paul's comment immediately above, note that we have a related issue (#411) queued up for SDPi 2.X (where probably X = 2)

@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Jun 6, 2025

2025-06-06 SDPi call: decision made to complete this PR as proposed (full removal of DEv-34, leaving a note behind). PR details need approvers' checking.

Copy link
Collaborator

@ToddCooper ToddCooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes reviewed and approved.

@JavierEspina JavierEspina requested a review from ToddCooper June 11, 2025 12:41
@JavierEspina
Copy link
Collaborator Author

JavierEspina commented Jun 11, 2025

@PeterKranich , @ToddCooper @d-gregorczyk , could you please review again for approval?
The previous approval by Todd just lost validity because I have made a few simple additional changes (changelog conflict resolution + removing mentioning of DEV-34 in a couple of places previously overlooked)

Copy link
Collaborator

@ToddCooper ToddCooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes reviewed and approved.

@ToddCooper ToddCooper merged commit ea3aa73 into master Jun 13, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Gemini SDPi Releases Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Action items from the SDPi 2.0 Developers & Tester (Nov 2024)
4 participants