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>
* fix(worker): Verify contents of loaded session (#2629)
* fix(worker): Verify contents of loaded session
Previously, it was possible for an empty private key to be used
when the session had no user or project associated with it.
The new checks guarantee that we will fail gracefully in these
cases.
* Check against len instead of nil
* Add CHANGELOG
* fix(session): Always invoke all parts of session cancel trigger
The session cancelation trigger would not set the key_id to null
appropriately if another one of the fields on the session was
already null, since that case statement would be matched first.
The new structure matches all statements, in case any of them have
special logic (such as in the project case).
This fixes a theoretical issue with database interaction. Because the
fields in the database are signed 64-bit integers, it's theoretically
possible to overflow them if we're using unsigned 64-bit integers in the
application code.
With this commit, the Worker now sends BytesUp and BytesDown for each
of the connections it handles to the Controller in the Status request.
The Controller then persists these fields to the database. There is also
a new trigger to prevent bytes_up and bytes_down to change after a
connection has been closed, as the last update to a session_connection
row should be the one that is performed as part of the connection
closure process.
Introduces data about bytes read and written to the connection close
payload. No changes are required in the Controller because the RPC
messages already contained the required fields. As of this commit,
bytes_up and bytes_down columns in the session_connection table are now
populated when a connection closes.
Refs: #2509
This commit introduces a new net.Conn implementation (countingConn) that
will keep track of bytes read and written for a particular client
connection. This is then implemented into the worker code using
interface composition.
Refs: #2501
* Randomize order of worker output. This uses the standard
rand.Shuffle() call; cribbed from
https://pkg.go.dev/math/rand#example-Shuffle
* Have some error/eventing in the worker status path use the base
context instead of the status-call-specific context to allow eventing
after a timeout event
Jeff MitchellandIrena Rindos
Todd
Eng Zer Jun
Johan Brandhorst-Satzkorn
Haotian
Hugo Vieira
Haotian