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

RFC: Configuring gateway 'root' properties from blueprints #1297

Open
1 of 5 tasks
Julusian opened this issue Oct 23, 2024 · 6 comments
Open
1 of 5 tasks

RFC: Configuring gateway 'root' properties from blueprints #1297

Julusian opened this issue Oct 23, 2024 · 6 comments
Labels
Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) Contribution RFC

Comments

@Julusian
Copy link
Member

About Me

This RFC is posted on behalf of the BBC

Use Case

We want the blueprints to own the configuration of the system.

For a MOS gateway, the ID of the gateway is defined at the root level of the gateway, which blueprints are not able to configure.
For playout gateway, there are various boolean options at the root level which control some behaviour of the gateway, these have suboptimal defaults that cannot be changed by the blueprints.

Proposal

I do not have a proposed solution, I am looking for ideas on how this should be solved.

Process

The Sofie Team will evaluate this RFC and open up a discussion about it, usually within a week.

  • RFC created
  • Sofie Team has evaluated the RFC
  • A workshop has been planned
  • RFC has been discussed in a workshop
  • A conclusion has been reached, see comments in thread
@jstarpl
Copy link
Member

jstarpl commented Oct 23, 2024

Could this potentially be a task for system blueprints? Maybe they could provide callbacks to return defaults for a first connection of a PeripheralDevice? Or even every connection, and "onPeripheralDeviceConnected" callback - if it were async, one could imagine a sort-of DHCP setup, where the System Blueprint could query a central command server for what these settings should be, and then that system could make the decision. Perhaps it would then make sense to expose some environment variables to the System Blueprint to make identifying instances easier.

@nytamin nytamin added the Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) label Oct 28, 2024
@nytamin
Copy link
Contributor

nytamin commented Oct 29, 2024

I've got a potential proposal that might solve this:

I believe the root problem lies with that the current PeripheralDevice type (see definition) contains some properties that are owned and set by the device itself (such as category, type, status, versions, token), some properties owned and set by Sofie Core (studioId, settings, connected) as well as one whose ownership is a ambiguous (name).

I propose that we rework the PeripheralDevice to be a pure "status carrier" instead, so that it'll contain only statuses and identifier info (such as token, status, connected, category, versions, nrcsName etc).

So we would:

  1. move the settings to be a setting on the Studio, similar to how we've done it with the sub-devices settings.
  2. move the studioId to a new collection (PeripheralDeviceStudioMap). This is to make the lookup for Device -> Studio simple and not require a deep lookup of the Studio.
  3. Add a blueprint method (to the system blueprints) to be able to add a PeripheralDeviceStudioMap entry.
  4. Add a blueprint method (to the studio blueprints ) to be able to set the settings for a PeripheralDevice (similar to the sub-devices).

@scriptorian
Copy link
Contributor

Please also consider the configuration API impact.
I can imagine that there may be some blueprint defaults for PeripheralDevice settings that we can define in a config-preset and provide a more limited set of settings to the API to offer simpler client implementation and allow safer parameter validation.

@Julusian
Copy link
Member Author

Julusian commented Nov 4, 2024

@nytamin I'm not entirely opposed to that plan, but have some thoughts

1 & 4) I think this would work. I think it would fit into the existing applyConfig method without needing a new one. I think the question then becomes, is the current strategy of having the subdevices separated from the parent device ok?

2 & 3) I am not sure I like this. Adding a collection which contains just a pair of ids feels very SQL and not very noSQL.
And will we be able to give that blueprint method enough info to make an informed decision about which studio a device should be part of?

What if the device provided the studioId as part of its init?
From a quick skim, I don't see not persisting this as making anything harder, in various places we are finding devices by studioId, but we can simply use the object on the studio document.
That moves the determination of the id to be defined in the environment, which I think makes sense. Your deployment kind of needs to know this relationship anyway so that you dont restart/delete/update the wrong one when doing maintenance. So I think it better to rely on that instead of relying on the blueprints to figure out and match this.
Of course, for installations which are a single studio this will be trivial as everything can default to studio0 unless something else is specified


I know I've pitched before that I would like to see the parent and child devices combined into one object at some point, might be worth considering doing that at the same time as the rest of this if it feels like not much more work.

@nytamin
Copy link
Contributor

nytamin commented Nov 4, 2024

What if the device provided the studioId as part of its init?

I don't think that's a good idea, because the action of "adding a device to a studio" is essentally giving that device access to that studio, so that is something that should be done by an authorized user / internally/ blueprints. Otherwise that would be a potential attack vector by a malicious actor.

Instead of having the StudioId-DeviceId-pair as a separate collection, it could be a property on the Studio? I guess if we add a db-index for that property as well, doing a lookup on Studios, to figure out which studio a Device has should be fine.

@Julusian
Copy link
Member Author

Julusian commented Nov 4, 2024

I don't think that's a good idea, because the action of "adding a device to a studio" is essentally giving that device access to that studio, so that is something that should be done by an authorized user / internally/ blueprints. Otherwise that would be a potential attack vector by a malicious actor.

I think I've said this before, but I would also really like to change the registration flow to be that the deviceId and token has to be added in Sofie by the user before the device is allowed to connect.
What I have proposed here would be leaning into that direction, but not going all the way there yet.

This got me curious, and I can now say that we don't have enough functional security for the current system to do anything. Any ddp client (including a malicious client) can after calling init for the first time, issue a core.ddp.ddpClient.call('/peripheralDevices/update', [{ _id: 'ExampleDevice' }, { $set: { studioId: 'studio0' } }, {}, ]) to add themselves to a studio. That is simply replicating what the ui does when attaching a device to the studio.

After #1287, if running with the new security mode enabled then that won't be possible. But it is also expected that nginx/whatever should be performing the authentication and only allowing connection to sofie if the peripheraldevice has valid credentials. Meaning the token wouldn't really do anything. And if the deviceId/studioId were specified via header instead of part of the init payload, then the proxy could enforce certain values to tighten it up even more. The result of this being that the auto-add and attach behaviour would actually be fine and desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) Contribution RFC
Projects
None yet
Development

No branches or pull requests

4 participants