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

der: Document (or improve) der::Decode trait impl error handling #1492

Open
str4d opened this issue Sep 1, 2024 · 3 comments
Open

der: Document (or improve) der::Decode trait impl error handling #1492

str4d opened this issue Sep 1, 2024 · 3 comments

Comments

@str4d
Copy link

str4d commented Sep 1, 2024

As part of #1491, I needed to implement decoding for the custom extension I wrote for #1490. I encounted several problems:

  • I need to validate the data present inside the extension. In particular, for the YubiKey PIV attestation policy extension I need to verify that the PIN and touch policies are one of the expected values. However, der::Decode::decode returns der::Result, and AFAICT there is no way to return a type-specific error code in der::Error. None of the existing der::ErrorKinds appear to be usable for this (or at least, I cannot figure out whether returning one of them will cause unexpected side-effects).
  • Attempting to even construct der::Error was confusing, until I stumbled upon der::Reader::error. The der::Decode trait (as well as der::Error) should document that errors should be created using decoder.error() (or whatever new method is added for defining user errors). It is also unclear to me whether the position that this method uses will be correct, if I am doing post-read validation (in which case I think the position will be one after the actual problem position).
@tarcieri
Copy link
Member

tarcieri commented Sep 1, 2024

The Decode trait (now) has an associated Error type: https://docs.rs/der/0.8.0-rc.0/der/trait.Decode.html#associatedtype.Error

If der::Error is too limited for a given use case it would be good to switch to a dedicated error type.

@tarcieri
Copy link
Member

tarcieri commented Sep 10, 2024

@str4d was this issue primarily motivated by iqlusioninc/yubikey.rs#580 or something else?

The Decode trait now has an associated Error type, which would be my suggested way of handling any errors which aren't a direct result of DER decoding (i.e. define a custom error type and do whatever you want error handling-wise).

That's a new addition which may not be properly reflected in the documentation yet.

@tarcieri
Copy link
Member

Regarding speculative decoding and computing the exact position for that, you can use the peek functionality to look ahead without actually modifying the cursor in the buffer, if that's helpful.

Otherwise, Error::new can be used to construct a new der::Error with whatever computed position you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants