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

ERL-915: wrong spec for pubkey_pbe:pbdkdf2/7 #3709

Closed
OTP-Maintainer opened this issue Apr 17, 2019 · 5 comments
Closed

ERL-915: wrong spec for pubkey_pbe:pbdkdf2/7 #3709

OTP-Maintainer opened this issue Apr 17, 2019 · 5 comments
Labels
bug Issue is reported as a bug priority:low team:PS Assigned to OTP team PS
Milestone

Comments

@OTP-Maintainer
Copy link

Original reporter: sdl.web@gmail.com
Affected version: OTP-21.3
Fixed in version: OTP-22.1
Component: public_key
Migrated from: https://bugs.erlang.org/browse/ERL-915


The spec says first argument is string() but it crashes with badarg when given any thing larger than 255. BTW, is is possible to provide a public API for pbdkdf2?

{code:erlang}
-spec pbdkdf2(string(), iodata(), integer(), integer(), fun(), atom(), integer())
	     -> binary().
%%
%% Description: Implements password based decryption key derive function 2.
%% Exported mainly for testing purposes.
%%--------------------------------------------------------------------
pbdkdf2(Password, Salt, Count, DerivedKeyLen, Prf, PrfHash, PrfOutputLen)->
    NumBlocks = ceiling(DerivedKeyLen / PrfOutputLen),
    NumLastBlockOctets = DerivedKeyLen - (NumBlocks - 1) * PrfOutputLen ,
    blocks(NumBlocks, NumLastBlockOctets, 1, Password, Salt, 
	   Count, Prf, PrfHash, PrfOutputLen, <<>>).
{code}
@OTP-Maintainer
Copy link
Author

ingela said:

Without checking, I would say that  it is only a sub type of string() that will actually work for this function. We will look into that. It would be possible to have a public API for password based encryption functions, we have thought of doing it, but it has never been prioritized high enough.
Once you make a function public, it is hard to change, so we like it to be thought through. You are welcome to make a PR if you have a suggestion. 

@OTP-Maintainer
Copy link
Author

sdl.web@gmail.com said:

Thanks for looking into this. I can live with pubkey_pbe:pbdkdf2/7 for now until a proper public API is provided.

@OTP-Maintainer
Copy link
Author

ingela said:

I think this spec ought to be iodata(), and when making a public
API the API function should perhaps have an ascci_str()  type.

@OTP-Maintainer
Copy link
Author

ingela said:

I fixed the spec. However I do not see the API being prioritized real soon, even if it would be nice.

@OTP-Maintainer
Copy link
Author

sdl.web@gmail.com said:

Thank you for the fix.

@OTP-Maintainer OTP-Maintainer added bug Issue is reported as a bug team:PS Assigned to OTP team PS priority:low labels Feb 10, 2021
@OTP-Maintainer OTP-Maintainer added this to the OTP-22.1 milestone Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug priority:low team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

1 participant