search

devonfw / devon4j

devonfw Java stack - create enterprise-grade business apps in Java safe and fast
Apache License 2.0
83 stars 87 forks source link

improve documentation on password encryption for strong encryption #71

Closed maybeec closed 3 years ago

maybeec commented 5 years ago

I made use of jasypt password encryption in my property files. One of my first step was to choose a strong cipher algorithm, so I was taking PBEWITHHMACSHA512ANDAES_256 as this was the best provided by jasypt at this moment of time. Interestingly, running the application, you get login issues as of non-provided password. During debugging I found out, that the password is decrypted to empty string for some reason. I haven't found the cause yet.

Anyhow, googling further, it seems, that jasypt is very instable in regards to AES encryption: https://sourceforge.net/p/jasypt/bugs/32/

Are there alternatives right now? Do we wait until jasypt is released in a proper version?

maybeec commented 5 years ago

also see ulisesbocchio/jasypt-spring-boot#124

hohwille commented 5 years ago

This is only related to the documentation. Fixes need to be applied here: https://github.com/devonfw/devon4j/blob/develop/documentation/guide-configuration.asciidoc#password-encryption

hohwille commented 3 years ago

It is a pitty: We had this story almost done with PR #298 but as the author of the PR left without reworking the last review feedback and nobody took over, I have to move this issue to the next release :(

hohwille commented 3 years ago

@maybeec I have merged #332. Can you check if that now matches your expectation and the new recommended algorithm is addressing your concerns of this issue so we can close it?

hohwille commented 3 years ago

What still confuses me is why PR #332 documented the algorithm PBEWITHHMACSHA512ANDAES_256 as @maybeec suggested but also added this config suggestion (where algorithm does not match what seems wrong to me):

jasypt.encryptor.algorithm=PBEWITHMD5ANDTRIPLEDES

BTW: Should that algorithm better go to application.yml or to application.properties? If it goes to application.properties and this is our best practice - why dont we already ship this in the main application.properties of our archetype?

maybeec commented 3 years ago

I am quite confused that we have both. Having both next to each other ´application.ymlandapplication.properties` does not make sense to me.

Has encryption and running the application been tested with PBEWITHHMACSHA512ANDAES_256? just asking as I had some issues in the past to get it running.

hohwille commented 3 years ago

I am quite confused that we have both. Having both next to each other ´application.ymlandapplication.properties` does not make sense to me.

@maybeec You are IMHO missing the point: The master-password has to be configured somewhere per environment. To keep this separate from other configs (e.g. managed in git repos) containing encrypted passwords helps to keep the approach meaningful. If the masterpassword is contained in the same config file that also contains the enctypted passwords, we can also drop the encryption and leave the passwords in plain text. The benefit is that if a config file with encrypted passwords for whatever reason goes into the wrong hands (e.g. due to a human mistake it is send in an email or whatever) an attacker still can not get the unencrypted password. Therefore we introduced application.yml that is supposed to ONLY contain the master-password and nothing else (except maybe as asked by myself the algorithm).

Has encryption and running the application been tested with PBEWITHHMACSHA512ANDAES_256? just asking as I had some issues in the past to get it running.

That is exactly what I instructed @sujith-mn to do and what he has tested and documented.

maybeec commented 3 years ago

OK, fine for me. Anyhow I would even leave it out and not enable encryption by default. Even here we could have also thought about simply providing a tutorial and a minimal sample to show how to do it.

In a could environment you would most probably provide the file as a config map or even as an environment variable. So I am fine then.

hohwille commented 3 years ago

Anyhow I would even leave it out and not enable encryption by default.

We never had jasypt encryption enforced and I do not even see how this could be archieved. It was always optional and will remain such. After all it is just a documentation feature. The only thing I was suggesting, is to pre-configure the algorithm to a secure one by default in our app-template to avoid that projects start with the jasypt default algorithm which seems to be insecure as you claimed when opening this issue.

hohwille commented 3 years ago

To make it cristal clear: There is one remaining question to clarify before we can close this story and we are already overdue with the release:

PBEWITHHMACSHA512ANDAES_256 != PBEWITHMD5ANDTRIPLEDES

So our documentation is IMHO inconsistent. @sujith-mn can you fix this or can you explaing why this difference of algorithms should be correct?

hohwille commented 3 years ago

OK, with this algorithm there was more or less a missunderstanding on my end:

Sorry, that I did not get it initially. However, I have discussed with @sujith-mn and we took this missunderstanding as an indicator to further improve the doucmentation. So @sujith-mn will create another PR updating the doc to make this even more obvious and avoid such potential confusion.