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

Reconsider the default mode for LTC_ARGCHK #458

Open
karel-m opened this issue Oct 26, 2018 · 4 comments
Open

Reconsider the default mode for LTC_ARGCHK #458

karel-m opened this issue Oct 26, 2018 · 4 comments
Milestone

Comments

@karel-m
Copy link
Member

karel-m commented Oct 26, 2018

Currently by default the LTC_ARGCK calls abort() when the check in question fails.

Isn't it too strict to literally die (core-dump) when you for example "only" pass an IV of invalid length?

You can probably imagine what does it mean for example when LTC_ARGCHK fails somewhere deep inside my perl bindings (it will cause the whole perl interpreter to crash).

I would prefer to only return CRYPT_INVALID_ARG by default (which means using current ARGTYPE == 4 as a default).

@sjaeckel
Copy link
Member

IIUC you're not using a system-provided ltc, right? why don't you then simply define -DARGTYPE=4 while compiling ltc?
I wouldn't want to change this per default TBH

@karel-m
Copy link
Member Author

karel-m commented Oct 26, 2018

Yes, the trick with -DARGTYPE=4 is exactly what I did. But for example Fedora/RedHat guys for some reason insist on linking with system's libtomcrypt.

The thing is that the ARGTYPE==0 decision is basically hardcoded in library binaries. We use LTC_ARGCHK pretty everywhere not only in really severe situations where there is no other way than to core dump.

It might happen that you have for example a nonce of invalid length (in a really mean scenario it may be a user input) which you pass to some libtomcrypt function ... and ... it will abort the whole program due to LTC_ARGCHK fail.

@sjaeckel
Copy link
Member

Then these cases shouldn't use the macro anymore!?

@sjaeckel sjaeckel added this to the next milestone Oct 26, 2020
@levitte
Copy link
Collaborator

levitte commented Aug 20, 2024

I've heard arguments of a similar vein over in OpenSSL space, and there are basically two camps opposing each other:

  • one camp that argue that:
    1. any incorrect input is a security problem
    2. calling applications should do everything they can to check that the input is correct 1
      (anything else is a bug in the application, see security problem)
    3. an abort() enables finding the exact spot where things went wrong when in a debugger
    4. programmers suck at checking returned status codes anyway...
  • the other camp that argues that:
    1. applications should be relied on checking returned statues codes
    2. OpenSSL libraries should give them a chance to recuperate should anything go wrong

I'm personally ambivalent... there are cases where an abort makes it much much easier to find a bug 2, but on the other hand, aborting is quite harsh.

The method chosen with OpenSSL is that a debug build will do harsh aborts, while a release build just returns with an error indication. This is essentially ARGTYPE=1 and ARGTYPE=4 combined.

Footnotes

  1. I found that it's not that difficult to get things right, as the diverse descriptors have quite a bit of data to go on.

  2. True story! I've found numerous bugs this way, which would have been very hard to trace otherwise.

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

3 participants