Secure keystores with filesystem acl, use pkcs12 - #422
Conversation
jonbartels
left a comment
jonbartels
left a comment
There was a problem hiding this comment.
This is making my brain itch. I feel like there could be some cyclic problem, maybe with containers? where the keystore is generated and a PW set, then the keystore is retained as a file (like if appdata is a persistent volume) but the generated keystore pass isn't retained because conf isnt in a persistent volume and the env var isn't set.
Instinctively - I'd document this, but theres a massive curse of knowledge problem because I am intimate with my keystores but a more casual, first-time user won't know how to process the big error
| # no longer a password to pin here. Keystores created by earlier versions are JCEKS and were pinned | ||
| # to 'changeme', so keep those settings when one is already present in appdata. | ||
| KEYSTORE_FILE="$APP_DIR/appdata/keystore.jks" | ||
| if [ -f "$KEYSTORE_FILE" ] && [ "$(head -c 4 "$KEYSTORE_FILE" | od -An -tx1 | tr -d '[:space:]')" = "cececece" ]; then |
There was a problem hiding this comment.
Explan what this is checking and how it knows its a jceks?
There was a problem hiding this comment.
This is a dirty hack suggested to me by AI. It's checking if the first 4 bytes of the file are cececece - which is the magic signature for a JCEKS keystore (the legacy format that would require the "changeme" default password).
TL;DR: backcompat hack.
There was a problem hiding this comment.
I removed it in favor of a filename check.
| import java.util.EnumSet; | ||
| import java.util.Set; | ||
|
|
||
| public class FilePermissionUtil { |
There was a problem hiding this comment.
Non blocking comment - I checked if file utils does this for us. It does not. I dont live introducing a util class but its appropriate here
There was a problem hiding this comment.
Maybe touch to create the file? But then you still gotta mess with perms. https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FileUtils.html#touch(java.io.File)
There was a problem hiding this comment.
I don't see an alternative here for any major portion.
| keystore.storepass = 81uWxplDtB | ||
| keystore.keypass = 81uWxplDtB | ||
| keystore.type = JCEKS | ||
| keystore.storepass = |
There was a problem hiding this comment.
Per Tonys comments from chat - add a comment in the props file explaining how this is generated if its blank.
There was a problem hiding this comment.
I did on line 28. With pkcs12, there is no need to have a password, so the blank is not a sentinel value - there just is no password.
There was a problem hiding this comment.
sorry. I was off: this is a test resource, not a published asset.
| .setPermissions(EnumSet.allOf(AclEntryPermission.class)) | ||
| .build(); | ||
| // @formatter:on | ||
| aclView.setAcl(Collections.singletonList(entry)); |
There was a problem hiding this comment.
I think the local admin group should also be added with full control?
root always has access in linux, but windows does not automatically give permission to local admins.
There was a problem hiding this comment.
I agree in principle, but it is a pain. Added as a separate commit in case we change our minds.
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
36c06bd to
22cfb0a
Compare
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
3 participants
Related to #307
Sets default access controls for key material created on first boot to only be accessible to the owner (600-equivalent):
Posix:
Windows:
Removes the 81uWxplDtB password in favor of a password-less PKCS12 (pfx) file instead of the deprecated jks format for new installs. Upgrades retain their prior password.