silverstripe / silverstripe-webauthn-authenticator

A Web Authentication (WebAuthn) authenticator for silverstripe/silverstripe-mfa
BSD 3-Clause "New" or "Revised" License
4 stars 3 forks source link

Keys are re-useable across different users #61

Closed ScopeyNZ closed 1 year ago

ScopeyNZ commented 5 years ago

@Firesphere commented on Sat Sep 14 2019

As an admin

I would like to be able to prevent the same key to be used for multiple accounts

So that

I can be sure any secrets are not shared across users

Because

My CMS is not the size of Google/GitHub/etc. where it is common for users to have multiple accounts, compared to my website CMS.


@brynwhyman commented on Sun Sep 15 2019

Thanks for the suggestion! What 'key' are you referring to here?


@Firesphere commented on Sun Sep 15 2019

Specifically, physical keys (e.g. Yubikeys). A shared TOTP is practically impossible


@ScopeyNZ commented on Sun Sep 15 2019

I can be sure any secrets are not shared across users

Just FYI, most U2F keys generate private/key pairs for each registration, meaning that there's almost always a different public key per registration. Public keys should be different per registration (depends on device). Here's a good description here:

https://developers.yubico.com/U2F/Protocol_details/Key_generation.html

Also, I don't think that we should refer to a public key as a "secret". Disclosure of the registered key data does not give anybody elevated access as they still need the individual key. In fact, you can only use the original key to even check if registered key data applies to that key, so there's not really any information disclosure with registered key data.

I don't mind the idea of this as an enhancement - but I personally believe that it's really not any security improvement at all (right now) and should be treated as a very "impact/low" enhancement unless there's a good explanation for how this affects a users security at all.

ScopeyNZ commented 5 years ago

I've moved this issue to the relevant authenticator repo (I should've used GitHub "transfer" rather than whatever I used)

In terms of implementation, it's effort/hard because the key storage architecture right now stores as a blob. This would have to change individual security keys to be a "has many" relation on the registered method.

Firesphere commented 5 years ago

@ScopeyNZ I'm aware that there are different public/private keys generated, I'm talking about the physical key.

As an admin I want

ScopeyNZ commented 5 years ago

Sure - that's why it's still open - I was just clearing it up for any future developer why it's tagged as impact/low 😃

GuySartorelli commented 1 year ago

Great idea but it's an old issue that we're not going to do anything with any time soon. Closing. If someone is keen for this and wants to put in the work to implement it, @ me and we can revisit.