Skip to content
This repository was archived by the owner on Jul 6, 2022. It is now read-only.

Comments

Client validation bugfixes#25

Merged
emlun merged 4 commits intoYubico:masterfrom
NWilson:ncw-bugfixes
Oct 23, 2017
Merged

Client validation bugfixes#25
emlun merged 4 commits intoYubico:masterfrom
NWilson:ncw-bugfixes

Conversation

@NWilson
Copy link
Contributor

@NWilson NWilson commented Jul 1, 2016

Bugfixes for several nasty errors handling client input data.

Worst of all, clients can even cause AssertionError to be thrown simply by sending data that's too short.

The tests do not even cover basic cases of malformed client data. The lack of error handling throughout the library is worrying; I've fixed all the errors I could find but it's quite possible there are more.

Also made a persistence constructor public (like all the other objects' constructors) so that it can be used directly if not using JSON for persistence.

NWilson added 4 commits July 1, 2016 14:57
I don't know why it wouldn't be, the constructors for other objects are
public.  If you aren't using JSON to persist the DeviceRegistration,
then you're really stuck, since you need to construct one in order to
call the other APIs.
@emlun
Copy link
Member

emlun commented Oct 20, 2017

Hi, thank you for your contribution! Sorry we haven't responded until now. I'll get back to this as soon as I can.

@emlun emlun self-assigned this Oct 20, 2017
@emlun emlun merged commit 5fe74cd into Yubico:master Oct 23, 2017
@emlun
Copy link
Member

emlun commented Oct 23, 2017

Looks good, thank you for contributing!

@emlun emlun added this to the 0.19.0 milestone Dec 21, 2017
emlun added a commit that referenced this pull request Jan 15, 2018
Breaking changes:

- Overhauled exception hierarchy
  - New exception class: `U2fCeremonyException`
  - New exception class:
    `U2fRegistrationException extends U2fCeremonyException`
  - New exception class:
    `U2fAuthenticationException extends U2fCeremonyException`
  - The following exception classes now extend
    `U2fAuthenticationException`:
    - `DeviceCompromisedException`
    - `InvalidDeviceCounterException`
    - `NoEligableDevicesException`
    - `NoEligibleDevicesException`
  - `U2fBadConfigurationException` is now a checked exception
  - `U2fBadInputException` is now a checked exception, and is no longer
    thrown directly by the methods of `U2F`.
    - Methods of `U2F` now catch this exception and wrap it in a
      `U2fRegistrationException` or ``U2fAuthenticationException`.
- `DeviceRegistration.getAttestationCertificate()` now returns `null`
  instead of throwing `NoSuchFieldException`
- `static ClientData.getString(JsonNode, String)` now throws
  `U2fBadInputException` instead of `NullPointerException`, or if the
  returned field is not a `String` value
- Some `AssertionError`s and `IllegalArgumentException`s are now
  `U2fBadInputException`s instead

Improvements:

- `BouncyCastleCrypto` now throws more descriptive exceptions

Bug fixes:

- Improved error handling in client data input validation
  - Thanks to Nicholas Wilson for the contribution, see
    #25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants