feat(gateway): revoke unlisted authorizations upon init#9896
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| .resources | ||
| .extract_if(|resource, _| !authorization.contains(resource)) | ||
| { | ||
| tracing::info!(%resource, "Revoking resource authorization"); |
There was a problem hiding this comment.
| tracing::info!(%resource, "Revoking resource authorization"); | |
| tracing::debug!(%resource, "Revoking resource authorization"); |
This could be quite noisy (thousands of messages) if the Gateway hasn't connected in a while, or connects after a major access change such as an IdP deletion.
There was a problem hiding this comment.
I think it is still a major state change of the system so INFO seems to be justified.
| /// In this case, we won't receive a `relays_presence` but instead we will receive relays with the same ID yet different credentials. | ||
| RebootRelaysWhilePartitioned(BTreeMap<RelayId, Host<u64>>), | ||
|
|
||
| /// De-authorize access to a resource whilst the Gateway is network-partitioned from the portal. |
There was a problem hiding this comment.
Out of curiosity, what does this test? That we respect the expiration timestamp of an authorization even while partitioned?
There was a problem hiding this comment.
Did you read the PR description? I tried to explain there what do test and what we don't test:
We (partly) test this by introducing a new transition in our proptests that de-authorizes a certain resource whilst the Gateway is simulated to be partitioned. It is difficult to test that we cannot make a connection once that has happened because we would have to simulate a malicious client that knows about resources / connections or ignores the "remove resource" message.
Testing this is deferred to a dedicated task. We do test that we hit the code path of revoking the resource authorization and because the other resources keep working, we also test that we are at least not revoking the wrong ones.
There was a problem hiding this comment.
This transition essentially simulates a re-connect to the portal and receiving a list of authorizations that contains all but the above resource.
This will be easier to understand once I get around to do #9895.
| </ChangeItem> | ||
| <ChangeItem pull="9896"> | ||
| Fixes a security issue where resource authorizations would not get | ||
| revoked if the Gateway was disconnected from the portal. |
There was a problem hiding this comment.
| revoked if the Gateway was disconnected from the portal. | |
| revoked if the Gateway was disconnected from the portal while access was removed. |
| establish. | ||
| </ChangeItem> | ||
| <ChangeItem pull="9896"> | ||
| Fixes a security issue where resource authorizations would not get |
There was a problem hiding this comment.
| Fixes a security issue where resource authorizations would not get | |
| Fixes a potential security issue where prior resource authorizations would not get |
There was a problem hiding this comment.
Just want to be clear / explicit here
2 participants
When receiving an
initmessage from the portal, we will now revoke all authorizations not listed in theauthorizationslist of theinitmessage.We (partly) test this by introducing a new transition in our proptests that de-authorizes a certain resource whilst the Gateway is simulated to be partitioned. It is difficult to test that we cannot make a connection once that has happened because we would have to simulate a malicious client that knows about resources / connections or ignores the "remove resource" message.
Testing this is deferred to a dedicated task. We do test that we hit the code path of revoking the resource authorization and because the other resources keep working, we also test that we are at least not revoking the wrong ones.
Resolves: #9892