Repository navigation
Conversation
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>
|
|
||
| keyStore.load(null, keyStorePassword); | ||
| logger.debug("keystore file not found, created new one"); | ||
| FilePermissionUtil.createOwnerOnlyFile(keyStoreFile); |
There was a problem hiding this comment.
A failed first boot leaves an empty keystore that blocks every later boot.
Tthe new createOwnerOnlyFile call creates keystore.pfx before the encryption key and default certificate are generated and written. Before this PR, the file was only created just before writing.
If key or certificate setup throws (for example, a bad encryption setting), the error is only logged and a 0-byte keystore.pfx stays on disk. On the next start the file exists, loading it fails, and security setup fails on every boot until someone deletes the file by hand.
Fix: create the owner-only file right before writing, or delete it when setup fails.
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.