We can identify domain controllers and their read-only variant (RODCs)
via the primaryGroupID attribute. If we discover a domain controller,
label it with `teleport.dev/is_domain_controller: true`. This will help
identify DCs, which should be treated with extra care (and potentially
filtered out from discovery with an LDAP filter).
* Fix Reverse Tunnels Not Properly reconnecting
Nodes were not generating new agents which prevented reverse tunnels
from being re-established after a collapse. Ensuring we always
release the lease if the agent pool is unable to add a new agent.
* add tunnel collapse reconnect test
Increase tolerance on expected reads.
This should prevent failures where due to our approximation, we estimate
a fractional number of reads that exceed our tolerance of 1.
Sample error: Max difference between 10.461059975 and 9 allowed is 1, but difference was 1.4610599749999995
Updates #9492
Generate sample YAML file with a proper duration for dynamic label
period.
The new `teleport db configure` cli command generated dynamic labels
with the period field as an integer rather than `time.Duration`. Our
current yaml.v2 import treats integers as nanoseconds, but has corrected
this behavior in v3 which breaks this implementation.
This is the API call that is used to populate the session recordings
page. In order for it to return desktop sessions and SSH sessions,
it needs to search for both "session.end" and
"windows.desktop.session.end" events.
Explicity set Kubernetes feature for the test.
The test requires the Kubernetes feature to be enabled while not
explicitly enabling it on its own. Sometimes it is enabled and other
times it is not, resulting in an error.
Global session data is used to store passwordless challenges. Per-user session
data cannot be used, as the user is now known by the time the challenge is
issued.
I elected to keep both global and per-user session data storage for now, as
per-user is less subject to DDoS-type attacks than global.
#9160
* Don't open clipboard static channel when clipboard is disabled
Prior to this change, we relied only on the frontend to check whether
or not RBAC allows access to the shared clipboard feature.
Add a new config parameter, which is passed all the way down do Rust.
If the clipboard is not enabled, we will never open the CLIPRDR channel
and no clipboard data will be exchanged with the RDP server. This is
both more secure and more efficient.
We also take care to ensure that clipboard actions that are attempted
when the clipboard is disabled is treated as a no-op rather than an
error. This ensures that a misbehaving client can't cause us to kill
the connection by sending clipboard messages when the shared clipboard
is disabled.
* Sorry, Clippy.
* Generate a random smart card PIN per-session
Instead of using a shared static PIN for all sessions, use a
cryptographically secure random number generator to generate
a random 8-digit PIN that is unique per session.
Additionally, implement the verify command by checking that the
PIN that Windows sends back matches the one we randomly generated.
Since the PIN is only-known to Teleport, this prevents users from
using the virtual smartcard for anything besides the initial
Windows login.
* Fix hang if PIN doesn't match
Now that we're verifying the smart card PIN, the RDP connection between
Windows Desktop Service will be closed if the PIN doesn't match.
Unfortunately, the goroutine that's reading user input from the browser
is still blocked waiting on user input, and doesn't know that we're no
longer connected to the desktop. This situation resolves itself as soon
as you try to move the mouse or press a key, but it would be better to
shut the TDP connection down immediately rather than leave it open with
a static screen.
* Don't error when PIN verify fails
By returning an error, we kill the connection.
Instead, just return the error that indicates verification failed.
Windows won't allow access to any resources, but the connection will
remain open.
* Record desktop sessions
Here we introduce a new protobuf type (DesktopRecording) that contains
an encoded TDP message, and update AuditWriter to treat these similarly
to SessionPrint events (which are used for SSH session recordings).
We also add desktop session playback endpoint, temporarily located at
/webapi/sites/:site/desktopplaybacktest/:session
which streams TDP messages from a recorded session over
a websocket interface.
* update session end (#9795)
* Updates SessionEnd event with fields needed for frontend
* removes the clock which didn't need to be passed
* Add `Recorded` field to `WindowsDesktopSessionEnd` (#9839)
* Adds the SessionRecording field to WindowsDesktopSessionEnd event to mimic SessionEnd events (useful for easy integration with frontend).
* 14 should have been 12
* removing test logic
* switches SessionRecording to simple boolean Recorded
* session recording websocket (#9908)
* Adds the SessionRecording field to WindowsDesktopSessionEnd event to mimic SessionEnd events (useful for easy integration with frontend).
* 14 should have been 12
* removing test logic
* switches SessionRecording to simple boolean Recorded
* Updates the websocket address
* updates desktopPlaybackHandle to restart playback once it reaches the end
* adds playback state and synchronization logic for ensuring that goroutines aren't leaked
* adds toggle functionality for play/pause
* fix for the fact that urls are case insensitive
* moves desktop_playback to its own file, fixes mistaken comment about how websocket.JSON.Receive works, fixes error messaging, wraps playbackState.Close in a sync.Once
* Adds a cond variable for the two goroutines, but doesn't solve the spinning loop problem for the hanging logic
* Adds a cancel-able context which is cancelled in ps.Close() in order to avoid a spinning loop in the websocket.Handler
* Moves the majority of playback logic into the playbackState, which is now renamed to the more accurate playbackPlayer.
* changes pp.hangWhilePaused to pp.waitWhilePaused
* Moves the context out of NewPlaybackPlayer and the playbackPlayer
struct, wraps playback goroutines in playbackPlayer.Play(ctx) in
order to comply with context semantics.
* removing unnecessary warnings
* touchups
* record screen size (#9992)
* Adds the SessionRecording field to WindowsDesktopSessionEnd event to mimic SessionEnd events (useful for easy integration with frontend).
* 14 should have been 12
* removing test logic
* switches SessionRecording to simple boolean Recorded
* Updates the websocket address
* updates desktopPlaybackHandle to restart playback once it reaches the end
* adds playback state and synchronization logic for ensuring that goroutines aren't leaked
* adds toggle functionality for play/pause
* fix for the fact that urls are case insensitive
* moves desktop_playback to its own file, fixes mistaken comment about how websocket.JSON.Receive works, fixes error messaging, wraps playbackState.Close in a sync.Once
* Adds a cond variable for the two goroutines, but doesn't solve the spinning loop problem for the hanging logic
* Adds a cancel-able context which is cancelled in ps.Close() in order to avoid a spinning loop in the websocket.Handler
* Moves the majority of playback logic into the playbackState, which is now renamed to the more accurate playbackPlayer.
* changes pp.hangWhilePaused to pp.waitWhilePaused
* Moves the context out of NewPlaybackPlayer and the playbackPlayer
struct, wraps playback goroutines in playbackPlayer.Play(ctx) in
order to comply with context semantics.
* removing unnecessary warnings
* Adds an OnRecv that's similar to OnSend, for emitting audit events for particular incoming tdp messages
* Send full `DesktopRecording` event as json over playback websocket. (#10052)
* playback websocket now sends a json representation of the DesktopRecording event rather than just the raw tdp message, in order for us to have timing data on the frontend
* updating json.Marshal to utils.FastMarshal
* Removing unnecessary comment
* playback end event (#10088)
* Adds an end event so that the playback player knows to set the progress bar to its end state
* making the end message a json
* if the marshal fails we don't want to send a message over websocket
* Use a static string
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
* Add participants to session end event
Desktop sessions are not joinable, so the participants list always
has a single member - the user who started the session.
This will ensure that our example role for RBAC for sessions
(which depends on the participants field) will work for desktop
sessions.
* Minor cleanup
* Only record sessions when enabled
In order for desktop sessions to be recorded, session recording
must be enabled in the cluster's session recording config and
at least one of the user's roles must enable it.
* Cleanup
* Start to address review comments
* Move TDP event handlers out of connectRDP
* Address more review comments and add some tests
* Add playback streaming test
* Consistent comments
* Fix tests
* Don't log PNG frames that exceed the size of a protobuf
Since the PNG frame message in our desktop protocol is unbounded,
it is theoretically possible for a message to exceed the size limit
of a single protobuf.
In practice, this is unlikely to occur with any legitimate RDP traffic,
as the bitmaps are at most 64x64 pixels and compressed in PNG form.
Rather than complicating the protocol to allow for PNGs to be split
across events, we simply refuse to log anything this big.
* Mark RFD 48 implemented
Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
Notably: line endings must be CRLF, and the strings must be
null-terminated.
We convert from LF **to** CRLF endings when pasting on Windows, but we don't
do any conversion in the other direction (CRLF copied on a remote
desktop will be sent directly to the user's workstation unmodified).
* Fix logic problem in (*AuditWriter).tryResumeStream
If all the attempts are discarded due to timeouts with no errors
we might return nil, nil in tryResumeStream, this adds a message
to the potentially nil error that makes it an actual error.
* Use `LimitExceeded` instead of wrapping the error
This adds an rpc UpsertKubeServiceV2 to replace UpsertKubeService. Currently, kubernetes service does not have a keepalive heartbeat unlike app, db, and windows service. This brings functionality in line with the others. This would allow for future use of the keepalive to track connected agents via prometheus metrics.
Prior to this change, Teleport would always emit errors to the logs
when a desktop session is closed (for example, when the user closes
their browser tab).
This change ensures that:
- the input and output writing goroutines in the proxy don't race
to close connections (we use a sync.Once here)
- EOF and other "connection closed" errors are not logged as errors
Fixes#8661
The use of a re-badged buildbox image (rebadging quay.io/gravitational/teleport-buildbox to us-docker.pkg.dev/ci-account/teleport/buildbox-root) is holdover from when CI builds required separate "root user" and "non-root user" images to execute tests under different contexts.
This requirement has since been lifted by using a Go-based build script in CI, and the having to manually invoke a process to re-badge the buildboxes is, tedious, confusing and causing stoppages.
This patch restores the use of buildboxes from quay.io in CI.
* Add the `cert.create` event
For now, this is only emitted for user certificate issuance.
* Make `cert_type` a string rather than an enum
* Match field names and json tags in events.Identity
* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits
* Event code shouldn't be under T10xx anymore
* Reconnect broken LDAP connections
Ensure that our LDAP client returns trace.ConnectionProblem errors
for LDAP error 200 (network error).
When this condition is detected, we re-initialize our LDAP client
to get a fresh connection.
Also clean up some of the errors in the reconciler so that they
include the resources kind and name without stringify-ing the
entire resource.
Lastly, add both a connect and request timeout for our LDAP client,
allowing us to detect connection issues sooner.
Fixes#9808
* Reschedule pending renewals after succesful LDAP cert renewals
Prior to this change, if we encountered an LDAP network error and
re-initialized the LDAP client, we would schedule a new renewal
but also leave the existing renewal in place. Since each succesful
renewal schedules a subsequent renewal, we would be renewing twice
as often as necessary after recovering from this network error.
Each subsequent network error that we recover from would create
yet another set of periodic renewals. In the worst case we could
ultimately exhaust system resources by doing nothing but handling
LDAP cert renewals.
Fix this by using a single *time.Timer that is used to handle the
next LDAP cert renewal. When it comes time to schedule a renewal,
we use timer.Reset() to bump the existing renewal out rather than
schedule an additional one.
This introduced the need for a mutex, so I got rid of the atomic
load/store for ldapInitialized and put that behind the same mutex.
* Add retries to LDAP client initialization
Prior to this change, we had retry logic on the initial setup and
on receiving a network error during a query. We did not have retry
logic in place if the re-initialization failed. This would put the
system in a state where desktop discovery stops running until the
cert approaches expiration (up to a maximum of 5-6 hours)
Now, we schedule the next re-initialization to be either:
- 10 minutes later, in the case of an error
- After 2/3 of the cert's validity has elapsed (5-6 hours),
for the happy path
* Enable map key sorting in `utils.FastMarshal`
This enables map key sorting during JSON serialization to ensure
`CompareAndSwap` opertions work consistently if the resource in
question has embedded map values (e.g. labels).
Note that this is hypothetically a breaking change: it makes
comparisons consistent, so if it saves a variant that consistently
fails a `CompareAndSwap()`, it will... fail consistently rather
than sometimes work. In practice, today only `CertificateAuthority`
resources use `CompareAndSwap()` and under normal circumstances they
never have labels attached.
In the unlikely event that some resource is affected, a load/store
should save it with sorted key ordering and fix the problem.
* Log a warning if CAs fail a compare-and-swap with multiple labels
This attempts to mitigate compare-and-swap failures if CAs have
multiple labels by logging a warning instructing users to re-save
their CertificateAuthority resource.
* Fix copied comment typo and make the logged warning actionable
* Fix log.Warn() call with formatting directives
Takes resource parser and match search and
adds these filters to ListResources.
Allows resource filtering by labels, search keywords,
or with the predicate language.
Part of RFD 55
* Begin Rust implementation of MS_RDPECLIP
Includes encoding/decoding support for the Clipboard Capabilities
PDU, as specified in section 2.2.2.1.
* Add support for the format list PDU (and response)
This message is the notification from one endpoint to another that
the data on the local clipboard has been updated.
* Start integrating cliprdr client
We're receiving the start of the initialization flow from the server,
but it doesn't like our response just yet. Probably missing a header
of some kind.
* Allow providing flags for the channel PDU header
This didn't appear to fix anything, but we will want to be able
to adjust those flags based on the message we're sending.
* Continue to flesh out cliprdr client
- Add docs
- Build out handlers for remaining messages
- Set the SHOW_PROTOCOL channel header flag
(this was key to getting the RDP server to respond to us)
* Respond to format data request
* Add lint and test targets for Rust code
And fix lint warnings
* Suppress collapsible_if warning for role_tester
This is a giant crepe macro, so the lint warning isn't even accurate.
* Hook up Rust/Go callbacks for passing clipboard data
* Address first round of review feedback
* Address review feedback
* Mark RFD 49 as implemented
* Add more lint coverage
golanglint-ci doesn't pick up subdirectories with their own go.mod
which left certain directories unlinted. To get around this we can
run golanglint-ci directly against those submodules.
* Add desktop_clipboard role option
As described in RFD 49, desktop_clipboard defaults to true.
Since sharing clipboards with a remote machine requires a high level
of trust, the presence of a single role in a role set which disables
the clipboard will result in the feature being disabled.
(This is in contrast to session recording, for example, where all
roles in a set must disable recording to turn it off.)
* completing TestBoolOptions
Co-authored-by: Isaiah Becker-Mayer <isaiah@goteleport.com>
Created a predicate parser that's customized for our
ResourceWithLabels. Plan is to allow UI and CLI users
to use this predicate language to filter resources by
labels and json fields.
Also updates vulcand/predicate to latest v1.2.0
Part of RFD 55
This option defaults to true, as defined in RFD #0033.
The preset editor and audit roles disable desktop session recording,
as they aren't permitted access to desktops. The preset access role
enables desktop session recording. The implicit role applied to all
role sets also disables recording, otherwise it would be impossible
to disable.
`TestProxyReverseTunnel` has been consistently failing with the following errors.
--- FAIL: TestProxyReverseTunnel (11.76s)
sshserver_test.go:1127:
Error Trace: sshserver_test.go:1127
Error: Received unexpected error:
timeout waiting for announce to be sent
Test: TestProxyReverseTunnel
FAIL
FAIL github.com/gravitational/teleport/lib/srv/regular 68.907s
--- FAIL: TestProxyReverseTunnel (23.43s)
assertion_compare.go:332:
Error Trace: sshserver_test.go:1144
Error: "5.775333527" is not less than "5"
Test: TestProxyReverseTunnel
Messages: []
FAIL
FAIL github.com/gravitational/teleport/lib/srv/regular 96.861s
These both appear to be timing related. By removing `t.Parallel()` or
increasing the timeouts it was possible to stabilize this test and have it
consistently pass. However, looking at what `TestProxyReverseTunnel` actually
tested, I don't think it can actually be removed completely. I've outlined
what it tested and why this is no longer necessary and why we should remove it.
* Reverse tunnels can be established
This test was written prior to integration tests existing. Teleport now has
integration tests that are more extensive, robust, and stable which cover
reverse tunnel functionality like `TwoClustersProxy`, `TwoClustersTunnel`, and
`TrustedTunnelNode`. The only thing they were missing that
`TestProxyReverseTunnel` had was checking `LastConnected` time. This PR has
been updated to add that to integration tests.
* Connectivity can be established over reverse tunnels
Similar to the above, we have more extensive, robust, and stable integration
test coverage for establishing connectivity over a reverse tunnel now.
The only bit of functionality that integration don't appear to have is
connecting by DNS name _and_ IP address at sshserver_test.go#L1066-L1067.
However, we do now have a dedicated unit test for this in
`TestProxySubsys_getMatchingServer`.
* Labels are synchronized
While this test does schenonize dynamic labels, it never actually checks if
they were synchronized correctly. That functionality was removed many years
ago in https://github.com/gravitational/teleport/pull/250.
We do now have unit test coverage for dynamic labels at
lib/labels/labels_test.go.
* Dynamically resolve reverse tunnel address
The reverse tunnel address is currently a static string that is
retrieved from config and passed around for the duration of a
services lifetime. When the `tunnel_public_address` is changed
on the proxy and the proxy is then restarted, all established
reverse tunnels over the old address will fail indefinintely.
As a means to get around this, #8102 introduced a mechanism
that would cause nodes to restart if their connection to the
auth server was down for a period of time. While this did
allow the nodes to pickup the new address after the nodes
restarted it was meant to be a stop gap until a more robust
solution could be applid.
Instead of using a static address, the reverse tunnel address
is now resolved via a `reversetunnel.Resolver`. Anywhere that
previoulsy relied on the static proxy address now will fetch
the actual reverse tunnel address via the webclient by using
the Resolver. In addition this builds on the refactoring done
in #4290 to further simplify the reversetunnel package. Since
we no longer track multiple proxies, all the left over bits
that did so have been removed to accomodate using a dynamic
reverse tunnel address.
Adds teleport_reverse_tunnels_connected Prometheus metric which tracks reverse tunnels connected to the proxy server by type.
* Update prometheus help
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
* Update metrics wording
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Log a warning for admins when App Service fails due to an empty Proxy public_addr.
The warning is useful because the handler error tends to get swallowed by the
Web UI, plus it is a more useful for an admin.
Zac Bergquist