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

Add SingleUsePassword #388

Closed
wants to merge 5 commits into from
Closed

Conversation

adrian-salajan
Copy link

One might want to use a password just once to initialise something, after that the password should not stay in memory.

@codecov
Copy link

codecov bot commented Sep 24, 2020

Codecov Report

Merging #388 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #388   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        16    +1     
  Lines          388       394    +6     
  Branches        15        21    +6     
=========================================
+ Hits           388       394    +6     
Impacted Files Coverage Δ
...ules/core/src/main/scala/ciris/ConfigDecoder.scala 100.00% <100.00%> (ø)
.../core/src/main/scala/ciris/SingleUsePassword.scala 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fef96f...8e24290. Read the comment docs.

@adrian-salajan
Copy link
Author

I'm not sure which implementation is the better/correct one
SingleUsePassword[F[_]] private (private val password: PasswordProtection) - current PR
vs
SingleUsePassword[F[_]] private (private val password: Resource[F,PasswordProtection]) - first commit in this PR

@vlovgr
Copy link
Owner

vlovgr commented Sep 29, 2020

Hey, thanks for raising this.

You might want to have a look at SafePassword as reference.

@adrian-salajan
Copy link
Author

Hey, thanks for raising this.

You might want to have a look at SafePassword as reference.

Hi, I saw it, actually it was what triggered this attempt :)

  1. i guess it is better to use PasswordProtection as a core type
  2. it might be safer to not use String anywhere, for construction or returning the password, to avoid any JVM string interning stuff
  3. i suspect my initial design of the class SingleUsePassword[F[_]] private (private val password: Resource[F,PasswordProtection]) might be safer since it is a description of a computation so it avoids any storing of the password until the computation is run

@vlovgr
Copy link
Owner

vlovgr commented Apr 23, 2021

@adrian-salajan Sorry for leaving this for so long. But I'd like to get this merged (in some form) for the next release candidate. I've also opened #457 to showcase an alternative implementation to the one in this pull request.

i guess it is better to use PasswordProtection as a core type

I don't see it providing any benefit. It uses Arrays.fill(chars, ' ') which is what I've opted for in #457 as well.

it might be safer to not use String anywhere, for construction or returning the password, to avoid any JVM string interning stuff

I agree it makes sense to use Array[Char] everywhere for security, even though it's mutable. I've opted for this in #457.

@vlovgr
Copy link
Owner

vlovgr commented May 26, 2021

Closing in favour of #457.

@vlovgr vlovgr closed this May 26, 2021
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

Successfully merging this pull request may close these issues.

2 participants