cloudfoundry-community / vault-boshrelease

Apache License 2.0
28 stars 35 forks source link

post-start failed #58

Closed ywei2017 closed 6 years ago

ywei2017 commented 6 years ago

This issue started with 1.0.0 bosh release. All was working on 0.8.0 release. The post-start job seems to be the same, did the vault binary itself change to cause the behavioral difference? The change is not necessarily a bad thing, but should be documented so the CSR can be generated properly.

vault-server/76005e04-f753-4c40-9bc7-d61808e8ea8c:/var/vcap/packages/vault/bin# ./vault unseal <key redacted>
WARNING! The "vault unseal" command is deprecated. Please use "vault operator
unseal" instead. This command will be removed in Vault 0.11 (or later).

Error unsealing: Put https://127.0.0.1:8200/v1/sys/unseal: x509: cannot validate certificate for 127.0.0.1 because it doesn't contain any IP SANs

Thanks Yansheng

jhunt commented 6 years ago

The deprecation notice is harmless. The problem seems to be that the certificate doesn't contain an IP Subject Alternate Name for 127.0.0.1. How did you generate the cert and what is it valid for?

ywei2017 commented 6 years ago

I am using the same key/cert pair I used with 0.8, which was working fine. So must be some change from 0.8 to 1.0.0 that caused the validation error. I got a new cert with the SAN for 127.0.0.1, so the SSL validation error goes away.

I can confirm that the warning is harmless.

Then I got another error, that the post-start return "1", even though the unseal was successful:

If I change the line to [[ -n "$key" ]] && vault unseal ${key} || echo "skip empty key" the issue will go away. I am also thinking of investigating the template why there is an empty line. Of course I am also curious whether any this changed to 1.0.0.

If changing to [[ -n "$key" ]] && vault unseal ${key} || echo "skip empty key" is the right solution, I can put in a quick P/R to solve it.

Please let me know.

Thanks

ywei2017 commented 6 years ago

I was able to reproduce, and confirm (of the unseal keys) this is a bug introduced in the 1.0.0 bosh release. I compared the behavior of the vault binary (v0.9.6).

vault-server/76005e04-f753-4c40-9bc7-d61808e8ea8c:/var/vcap/packages/vault/bin# ./vault -v
Vault v0.9.6 ('7099615e31a6d13222ed1b2fbc7cf44456ddfb8e')
vault-server/76005e04-f753-4c40-9bc7-d61808e8ea8c:/var/vcap/packages/vault/bin# echo '' | ./vault unseal; echo $?
WARNING! The "vault unseal" command is deprecated. Please use "vault operator
unseal" instead. This command will be removed in Vault 0.11 (or later).

Unseal Key (will be hidden):
An error occurred attempting to ask for an unseal key. The raw error message
is shown below, but usually this is because you attempted to pipe a value
into the unseal command or you are executing outside of a terminal (tty). You
should run the unseal command from a terminal for maximum security. If this is
not an option, the unseal can be provided as the first argument to the unseal
command. The raw error was:  File descriptor 0 is not a terminal
1
vault-server/76005e04-f753-4c40-9bc7-d61808e8ea8c:/var/vcap/packages/vault/bin# echo '' | ./vault8 unseal; echo $
?
Vault is already unsealed.
0

In vault binary 0.8.3, it will return '0', and say already unsealed. In 0.9.6, it return 1, causing the post-start to fail.

Again, let me know where the following is a valid fix: [[ -n "$key" ]] && vault unseal ${key} || echo "skip empty key"

Thanks

ywei2017 commented 6 years ago

@jhunt James, will you be open for a P/R with a fix I described above? I consider that fix to be both harmless and sanitary. Granted the /var/vcap/jobs/vault/data/unseal_keys should NOT contain empty lines, I would think it is good practice the post-start script to guard against those situations.

Thanks Yansheng

jhunt commented 6 years ago

I'm cool with whatever; I don't personally use the unseal_keys method since it involves deliberately weakening the Vault's security posture.

ywei2017 commented 6 years ago

@jhunt I will put in a P/R in the next few days, to fix both this issue, and #59 .

Cheers.

ywei2017 commented 6 years ago

@jhunt See P/R #60 . I tested it locally to make sure it works as expected.

jhunt commented 6 years ago

@ywei2017 can we close this since #60 got merged?

MattSurabian commented 6 years ago

I think so, we also confirmed that the newest 0.10.2 release is still compatible with unseal meaning this post-start script should be functional for at least another release cycle

jhunt commented 6 years ago

This should be fixed / available in the 1.0.1 release: https://github.com/cloudfoundry-community/vault-boshrelease/releases/tag/v1.0.1