Most of the changes here are changes or enhancements that are useful for
tests. The crux of the issue was that I had pulled up one var to the
package level so it could be read from tests, but that var was not being
reset between tests. It's not even used in tests anymore, so I put it
back to function-local.
* Add more logging to worker auth rotation and test flags
This makes it much easier to figure out when and why rotation is
happening and control it for tests/debugging.
* Update auth rotation logic
Because we were looking at validity period of client certs, which may
not have matched the actual rotation interval, it was possible (mostly
in test scenarios) for the rotation to happen multiple times on the
roots before we actually hit the worker rotation interval.
This changes how the interval is calculated by looking at the difference
between not-after values on the certs to understand the rotation
interval being used by the controller. We then use that as the starting
point for calculating a next-rotation time.
This makes it much easier to figure out when and why rotation is
happening and control it for tests/debugging.
Remove cert skew
Cherry-picked from release/0.12.x
Do not allow PKI-KMS workers to have names updated via API
This would cause their public IDs to change, which we do not want.
Descriptions are also disallowed, since those come from the config file.
This also adds logic to allow upgrading from old KMS method to new, with
a test.
Co-authored-by: Irena Rindos <irenarindos@users.noreply.github.com>
This adds a new mechanism for worker authentication that uses the
third-party arbitration of the original KMS flow by leveraging
recently-added capabilities in nodeenrollment to support a registration
wrapper. This allows for workers to be registered to the system with
just a name and a matching wrapper, while at the same time allowing
these workers to be used for private Vault access and multi-hop as they
contain per-worker encryption keys and support rotation.
Nodes using this mode will generate and rotate credentials in-memory
(using another recently added nodeenrollment capability) so will have a
new set of keys on every startup. This has the side effect of ensuring
that there is never a conflict or re-use of these credentials.
Note that the normal test worker flow was using the now-deprecated KMS
method; there is still a test that uses this (explicitly) but all other
tests using test workers that did not specify a specific PKI flow now
actually use this mechanism. This means there is fairly decent test
coverage in a general sense, plus the specific tests updated/added for
this feature.
This also adds a new KMS type that can be used to have
distinct KMSes for upstream authentication vs. accepting from
downstream.
When possible downstream workers will include their worker id information in
the connections they establish with the upstream using the nodeenrollment
library. The tracking listener now tracks connections that are connected in
this same way instead of just connections of type *tls.Conn.
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests [complete.
Prior to this commit, temporary directory created using `os.MkdirTemp`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* Fix target port handling
This fixes two issues that compounded on each other (see the Changelog
update for more information):
* The verification logic for hosts was not correct for update operations
(in multiple ways) which meant that a host could be updated after
creation to have a port. Targets had a previously fixed bug where they
did not require a default port, which meant that ports could be used
from hosts
* A recent (unreleased) change had prioritized any port coming from the
host over the default port, which would mean there was no way if a
host had a port specified to use it in multiple targets.
This fixes the update verification logic, and strikes a middle ground
between breaking things and not by allowing existing addresses with
ports to be used with targets but ignoring that port, instead requiring
targets to have default port set at authorize time (currently there is
backwards compat that does not require this due to the original optional
port bug).
* Update CHANGELOG.md
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Jeff Mitchell
Todd
Louis Ruch
Jim
Eng Zer Jun
Johan Brandhorst-Satzkorn
Haotian