Not update Authenticator.SignCount when cloned authenticator is detected

[m9a]

@nicksteele @emlun I noticed that the code updates the SignCount even when a cloned authenticator is detected. I believe we should be returning immediately after the clone warning is true. wdyt?

https://github.com/duo-labs/webauthn/blob/master/webauthn/authenticator.go#L47

func (a *Authenticator) UpdateCounter(authDataCount uint32) {
	if authDataCount <= a.SignCount && (authDataCount != 0 || a.SignCount != 0) {
		a.CloneWarning = true
	}
	a.SignCount = authDataCount
}

[emlun]

That seems reasonable to me. I haven't thought hard about it, but indeed I guess the current logic would likely decrease the stored signature count, which you probably don't want. So yes, returning early (or explicitly separating a.CloneWarning = true and a.SignCount = authDataCount into the mutually exclusive if-else branches) should prevent that.

I'm not a developer of this library, though, so the decision is not mine to make.

[m9a]

@emlun thanks! I have created the MR: https://github.com/duo-labs/webauthn/pull/93. It's a fairly quick one :) I have added you and @nicksteele as the reviewers. I am new to the library and not quite sure if there are any active developers. Do you have any suggestions for reviewers?

[james-d-elliott]

@MasterKale this one can be closed as it w as resolved by #93