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

MSC1935: Key validity enforcement #1935

Closed
wants to merge 3 commits into from
Closed

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Mar 20, 2019

@turt2live turt2live requested a review from a team March 20, 2019 23:28
@turt2live turt2live added proposal-in-review proposal A matrix spec change proposal labels Mar 20, 2019
@turt2live turt2live changed the title Proposal for key validity enforcement MSC1935: Key validity enforcement Mar 20, 2019
Includes room for the potential of slashes being dropped from event IDs.
@ara4n
Copy link
Member

ara4n commented Mar 20, 2019

lgtm mod comment

@anoadragon453
Copy link
Member

Looks good to me. I think it's fine to change the definition of past room versions as well as long as we agree upon it democratically.

It seems like this'll be a fairly minor change that people will be happy with, so:

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Mar 21, 2019

Team member @anoadragon453 has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@mscbot mscbot added disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Mar 21, 2019
@turt2live
Copy link
Member Author

I think it's fine to change the definition of past room versions as well as long as we agree upon it democratically.

ftr, the reason the room version specs are in-tree is for cases like this. Like other specifications, they are immutable once written however by moving stuff into a new room version we have to update the old versions as well. The old versions should not be receiving changes which alter their intended purpose or expectation for the time they were introduced. More plainly said, room versions get added information but cannot change.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

I have two, somewhat related, concerns here.

The basic problem is that I think there is more to enforcing validity periods than just saying "make it so". There are unanswered questions around how you apply validity periods to objects which may have been signed some time ago, how you go about establishing whether a key was valid at a given point in the past, whether you should actively attempt to refresh your copy of a key which will shortly expire, etc etc. These are the reasons that I have avoided making this into an MSC so far: I want to do some experimentation and investigation to figure out exactly what's involved.

Relatedly, to date we've tried out things which need room version changes in a test room version first. For example, we might discover that there is some critical security bug that needs to go into a new room version asap and I don't want to have to make it v5 because v4 is reserved for something that we aren't ready for yet.

@mscbot concern needs more detail
@mscbot concern v4 definition should be a separate msc

@richvdh
Copy link
Member

richvdh commented Mar 21, 2019

I have two, somewhat related, concerns here.

The basic problem is that I think there is more to enforcing validity periods than just saying "make it so". There are unanswered questions around how you apply validity periods to objects which may have been signed some time ago, how you go about establishing whether a key was valid at a given point in the past, whether you should actively attempt to refresh your copy of a key which will shortly expire, etc etc. These are the reasons that I have avoided making this into an MSC so far: I want to do some experimentation and investigation to figure out exactly what's involved.

Relatedly, to date we've tried out things which need room version changes in a test room version first. For example, we might discover that there is some critical security bug that needs to go into a new room version asap and I don't want to have to make it v5 because v4 is reserved for something that we aren't ready for yet.

@mscbot concern needs more detail
@mscbot concern v4 definition should be a separate msc

@turt2live
Copy link
Member Author

This MSC wasn't created in the right frame of mind, and as @richvdh mentions the proposal is certainly lacking in many areas. Instead of proposing a half-baked solution here, I'm closing this so that someone with a lot more knowledge on the system at play can form a more concrete proposal.

Also, I fully agree the v4 inclusions need to be done separately.

@mscbot fcp cancel

@mscbot
Copy link
Collaborator

mscbot commented Mar 21, 2019

@turt2live proposal cancelled.

@mscbot mscbot removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Mar 21, 2019
@turt2live turt2live closed this Mar 21, 2019
@turt2live turt2live added abandoned A proposal where the author/shepherd is not responsive and removed proposal-in-review labels Mar 21, 2019
@turt2live turt2live deleted the travis/msc/key-validity branch August 21, 2019 23:54
@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned A proposal where the author/shepherd is not responsive kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants