This is the _last_ step required to migrate from logrus to slog.
All components in the repository have been migrated to use slog
allowing the logrus formatter to be deleted.
The slog handler tests that validate the output have been updated
to assert the format directly instead of comparing it to the output
from the logrus formatter.
This change is a change in how we are handling the untrusted input. Instead of adopting a strategy of quoting (which was incomplete), this instead escapes any special characters.
Although github.com/google/safetext is new, and without any current tags for us to reference, I believe it would be better to reference their implementation than to update the UnixShellQuote to handle the newly identified cases.
* Quote user supplied inputs provided to scripts to avoid RCE
This change introduces the func `utils.UnixShellQuote` which will quote any inputs which could potentially allow execution or script escape.
This is utilized to ensure that scripts produced from a potential Phishing link could not contain code execution which may expose a user.
* awsAccessGraphOIDCSync: Ensure role parameter is quoted correctly
* join_tokens: Move shell quote to `getJoinScript` rather than where parameters are extracted
This will increase safety moving forward, but it requires a more conservative quoting strategy.
* Bump github.com/gravitational/trace from 1.3.1 to 1.4.0
## Changelog
- Bump google.golang.org/grpc from 1.43.0 to 1.53.0 by @dependabot in #94
- Bump golang.org/x/net from 0.8.0 to 0.17.0 by @dependabot in #100
- Bump google.golang.org/grpc from 1.53.0 to 1.56.3 by @dependabot in #101
- Bump golang.org/x/crypto from 0.14.0 to 0.17.0 by @dependabot in #105
## Breaking Changes
- Remove logrus dependency by @rosstimothy in #107
- Remove references to UDP logrus hook from README by @rosstimothy in #108
Full Changelog: https://github.com/gravitational/trace/compare/v1.3.1...v1.4.0
* remove final references to removed trace functionality
* fix: formatter error handling
When creating a reverse proxy the ErrorLog was being created
with the writer returned from logrus.WriterLevel. This writer
uses an io.Pipe and a goroutine to get any log messages written
from a standard log.Logger to our custom formated logrus.Entry.
The ErrorLog only ever omits output in a few very rare (hopefully)
error cases, which means we were creating an extra goroutine per
reverse proxy which in most cases would never be used to produce
any added output. Additionally, now that we are starting to support
log/slog, any output from a log.Logger will now be formatted via
our registered default slog handler. Meaning that we can still
get the benefits of producing output from the ErrorLog in the
desired format without the penalty incurred by using logrus.WriterLevel.
Partially addresses #36541.
* Update slog handlers to support configured extra_fields
In addition to configuring output format users can configure the
fields which are included in log messages. Prior to this our slog
handlers didn't support this feature and always output the default
fields. Now, both the slog and logrus output is identical no matter
the extra_fields configuration as verified by TestExtraFields. It
was also found that due to the way the logurs json formatter was
configuring itself in CheckAndSetDefaults the timestamp was always
included in the final output even if it was not included in the
configured extra_fields.
* fix: update component comment
In all places that a logrus logger is being initialized an equivalent
slog logger is now being intialized as well. The TeleportProcess
was also updated to have a slog.Logger which is created with the
same fields that its logrus equivalent has. This does not convert
any logging to actually use slog yet, but does pave the way for
conversions to start.
Implements two new slog.Handlers that attempt to output text in the
same format as existing logrus.TextFormatter implementations. The
most notable differences are that the order of fields in the json
output does NOT match the logrus order and the additional items
in text output are NOT sorted alphabetically like they are in logrus.
Tests added validate the output roughly matches between the two
loggers given the differences noted above. A benchmark test is also
included to demonstrate the improvements slog offers over logrus.
This PR replaces the usage of `gravitational/oxy.Forwarder` in our codebase with `httputil.ReverseProxy`.
`httputil.ReverseProxy` is part of Go's standard library and thus it is
maintained by the Go team. Converting everything to use the std lib
allows us to drop our `oxy` fork.
There are some operational changes between the two versions:
1. `httputil.ReverseProxy` handles WebSockets upgrades internally and
reuses the `Transport` provided for the initial request. Thus, it does
not require any special dialer or rewriter mechanisms for WebSockets only.
2. `httputil.ReverseProxy` calls the `ErrorHandler` with the request
after it was rewritten and after roundtrip was called. This was a
problem because our `web.transport` deletes Teleport associated
cookies and when the request failed, the request received by the
`ErrorHandler` missed the cookies to generate a new session. This was
fixed by cloning the request at `web.transport` round tripper.
3. `httputil.ReverseProxy` uses std's `log.Logger`. `logrus` exposes a
method `Writer` that allows it to be used with standard lib logger.
The API is similar to what our `oxy` fork has available.
Fixes#27412
* check for correct kube listen address in starting message
* fix for ssh proxy and usage of String function
* remove consolef output and simplify output
* remove consolef output
* Remove consolef from services for startup
* fix imports
* remove all consolef usage
In “tsh login” show only alerts with on-login label.
In “tsh status” show only alerts with “high” severity. Which license warning should match.
In all “tctl” commands show only alerts with “high” severity.
Update metalinter, fix a few lint warnings and replace deprecated linters.
`deadcode`, `structcheck` and `varcheck` are abandoned and now replaced by [`unused`][1].
Since 1.19, `go fmt` reformats godocs according to https://go.dev/doc/comment. I've done a bulk-reformatting of the codebase to keep the linter happy. Backporting is mostly harmless (the exception being `lib/services/role_test.go`, that for some reason breaks the _old_ linter using the new format).
[1]: https://golangci-lint.run/usage/linters/
* Bump golangci-lint version
* Replace abandoned linters
* Fix bodyclose on lib/auth/github.com
* Fix bodyclose on lib/kube/proxy/streamproto/proto_test.go
* Fix bodyclose on lib/srv/alpnproxy/proxy_test.go
* Fix bodyclose on lib/web/conn_upgrade_test.go
* Silence staticcheck on lib/kube/proxy/forwarder_test.go
* Silence staticcheck on lib/utils/certs_test.go
* Address BuildNameToCertificate deprecation warnings
* Run `go fmt ./...`
* Run `go fmt ./...` on api/
* Ignore formatting in role_test.go
* Remove redundant initializers in lib/srv/uacc/
* Update e/
* start work on self signed tsh fixes
* fix go sum
* Adjust error formatting
* Complete less explicit error checks last
* Adjust PR feedback
* Further PR review
* Support darwin and linux certificate errors
* aliases are read from global and user configs.
* we prevent Kingpin from terminating `tsh`; we handle parsing errors better.
* added support for `TELEPORT_DEBUG` env variable, changed how logging is initialized.
* debugging aliases is possible via `TELEPORT_DEBUG=1` env variable; `--debug` is ineffective as it comes into play too late.
* if alias definition calls `tsh`, we call the `Run()` function directly instead of spawning fresh `tsh`; this improves the UX.
* alias loops are detected and a proper error is shown.
* all flags are made repeatable; if only one value for a given flag is possible, the last instance of the flag will be effective.
Co-authored-by: Marek Smoliński <marek@goteleport.com>
* Add certificate renewal bot
This adds a new `tbot` tool to continuously renew a set of
certificates after registering with a Teleport cluster using a
similar process to standard node joining.
This makes some modifications to user certificate generation to allow
for certificates that can be renewed beyond their original TTL, and
exposes new gRPC endpoints:
* `CreateBotJoinToken` creates a join token for a bot user
* `GenerateInitialRenewableUserCerts` exchanges a token for a set of
certificates with a new `renewable` flag set
A new `tctl` command, `tctl bots add`, creates a bot user and calls
`CreateBotJoinToken` to issue a token. A bot instance can then be
started using a provided command.
* Cert bot refactoring pass
* Use role requests to split renewable certs from end-user certs
* Add bot configuration file
* Use `teleport.dev/bot` label
* Remove `impersonator` flag on initial bot certs
* Remove unnecessary `renew` package
* Misc other cleanup
* Do not pass through `renewable` flag when role requests are set
This adds additional restrictions on when a certificate's `renewable`
flag is carried over to a new certificate. In particular, it now also
denies the flag when either role requests are present, or the
`disallowReissue` flag has been previously set.
In practice `disallow-reissue` would have prevented any undesired
behavior but this improves consistency and resolves a TODO.
* Various tbot UX improvements; render SSH config
* Fully flesh out config template rendering
* Fix rendering for SSH configuration templates
* Added `String()` impls for destination types
* Improve certificate renewal logging; show more detail
* Properly fall back to default (all) roles
* Add mode hints for files
* Add/update copyright headers
* Add stubs for tbot init and watch commands
* Add gRPC endpoints for managing bots
* Add `CreateBot`, `DeleteBot`, and `GetBotUsers` gRPC endpoints
* Replace `tctl bot (add|rm|ls)` implementations with gRPC calls
* Define a few new constants, `DefaultBotJoinTTL`, `BotLabel`,
`BotGenerationLabel`
* Fix outdated destination flag in example tbot command
* Bugfix pass for demo
* Fixed a few nil pointer derefs when using config from CLI args
* Properly create destination if `--destination-dir` flag is used
* Remove improper default on CLI flag
* `DestinationConfig` is now a list of pointers
* Address first wave of review feedback
Fixes the majority of smaller issues caught by reviewers, thanks all!
* Add doc comments for bot.go functions
* Return the token TTL from CreateBot
* Split initial user cert issuance from `generateUserCerts()`
Issuing initial renewable certificate ended up requiring a lot of
hacks to skip checks that prevented anonymous bots from getting
certs even though we'd verified their identity elsewhere (via token).
This reverts all those hacks and splits initial bot cert logic into a
dedicated `generateInitialRenewableUserCerts()` function which should
make the whole process much easier to follow.
* Set bot traits to silence log messages
* tbot log message consistency pass
* Implement `tbot init` subcommand
This adds a new CLI subcommand to initialize a tbot destination
directory by creating required files ahead of time and assigning
proper permissions (and ACLs, where possible).
* Resolve lints
* Add config tests
* Remove CreateBotJoinToken endpoint
Users should instead use the CreateBot/DeleteBot endpoints.
* Create a fresh private key for every impersonated identity renewal
* Hide `config` subcommand
* Rename bot label prefix to `teleport.internal/`
* Use types.NewRole() to create bot roles
* Clean up error handling in custom YAML unmarshallers
Also, add notes about the supported YAML shapes.
* Fetch proxy host via gRPC Ping() instead of GetProxies()
* Update lib/auth/bot.go
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
* Fix some review comments
* Add renewable certificate generation checks (#10098)
* Add renewable certificate generation checks
This adds a new validation check for renewable certificates that
maintains a renewal counter as both a certificate extension and a
user label. This counter is used to ensure only a single certificate
lineage can exist: for example, if a renewable certificate is stolen,
only one copy of the certificate can be renewed as the generation
counter will not match
When renewing a certificate, first the generation counter presented
by the user (via their TLS identity) is compared to a value stored
with the associated user (in a new `teleport.dev/bot-generation`
label field). If they aren't equal, the renewal attempt fails.
Otherwise, the generation counter is incremented by 1, stored to the
database using a `CompareAndSwap()` to ensure atomicity, and set on
the generated certificate for use in future renewals.
* Add unit tests for the generation counter
This adds new unit tests to exercise the generation counter checks.
Additionally, it fixes two other renewable cert tests that were
failing.
* Remove certRequestGeneration() function
* Emit audit event when cert generations don't match
* Fully implement `tctl bots lock`
* Show bot name in `tctl bots ls`
* Lock bots when a cert generation mismatch is found
* Make CompareFailed respones from validateGenerationLabel() more actionable
* Update lib/services/local/users.go
Co-authored-by: Nic Klaassen <nic@goteleport.com>
* Backend changes for tbot IoT and AWS joining (#10360)
* backend changes
* add token permission check
* pass ctx from caller
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
* fix comment typo
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
* use UserMetadata instead of Identity in RenewableCertificateGenerationMismatch event
* Client changes for tbot IoT joining (#10397)
* client changes
* delete replaced APIs
* delete unused tbot/auth.go
* add license header
* don't unecessarily fetch host CA
* log fixes
* s/tunnelling/tunneling/
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
* auth server addresses may be proxies
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
* comment typo fix
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
* move *Server methods out of auth_with_roles.go (#10416)
Co-authored-by: Tim Buckley <tim@goteleport.com>
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
Co-authored-by: Tim Buckley <tim@goteleport.com>
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Co-authored-by: Tim Buckley <tim@goteleport.com>
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
Co-authored-by: Nic Klaassen <nic@goteleport.com>
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
* Address another batch of review feedback
* Addres another batch of review feedback
Add `Role.SetMetadata()`, simplify more `trace.WrapWithMessage()`
calls, clear some TODOs and lints, and address other misc feedback
items.
* Fix lint
* Add missing doc comments to SaveIdentity / LoadIdentity
* Remove pam tag from tbot build
* Update note about bot lock deletion
* Another pass of review feedback
Ensure all requestable roles exist when creating a bot, adjust the
default renewable cert TTL down to 1 hour, and check types during
`CompareAndSwapUser()`
* Remove ModeHint
* Rename Identity.Cert and Identity.XCert
* Add `symlinks` flag to tbot config
The optional symlinks flag for directory destinations allows users to
opt in / out of whichever symlink attack hardening mode is selected
by default.
* Add mostly-working secure implementation of botfs.Create/Write
This adds symlink mode selection (secure, try-secure, insecure) and
Linux `Create()`/`Write()` implementations to open files safely.
* Add configurable ACL modes and verify ACL support in tbot init
* Initialize destinations at startup and test before renewal
This initializes destinations at startup (to create directories if
not using `tbot init`) and tests them to ensure the bot can write
_before_ attempting to renew certificates; this should prevent most
accidental generation counter locks.
* Hide watch for now
* Issue a new identity if a token change is detected
* Warn if identity appears to be expired on startup
* Fully implement ACL Verify and Configure
- Fully implements ACL support for Linux
- Adds bot-side verification support to ensure ACLs are configured
properly at runtime.
- Gracefully falls back to no ACLs if the platform / filesystem
doesn't support them
- Clear up outstanding lints
* Make `tbot init` work without a config file
* Show init instructions in tctl bots add
Also:
- Make --bot-user a flag in init (the tctl instructions were
confusing otherwise)
- Handle IsOwnedBy sanely on unsupported platforms
- Add Bold colorizing support
* Clear some TODOs and rephrase tctl help
* Fix typo
* Fix token hash detection bug
* Actually read and write certs with symlink enforcement
Also, fix a config loading bug where CheckAndSetDefaults() wasn't
being called in all cases with CLI destinations.
* Add workaround for OpenSSH permissions check with ACLs
OpenSSH has an overly-paranoid permissions check that forces key
files to be exclusively owner-readable. Unfortunately, for POSIX
compatibility purposes, when ACLs are set, the ACL mask is set as
the group permissions. This effectively makes any ACL incompatible
with OpenSSH.
However, OpenSSH's check does have an escape hatch: it only applies
if the current user is the owner of the file. Therefore, this change
tweaks the `tbot init` flow to create files as root, owned by a
separate user (either `nobody` or even the bot user), with ACL
permissions granting both the bot and reader user access to the
certificates. This effectively bypasses OpenSSH's permissions check
and should preserve our security boundaries.
* Fix lints
* Fix an improper directory chmod to 0600 if ACL test fails
* First pass of tbot init unit tests
* Add symlink tests and fix bug with resolving the default owner
* Fix err misuse
* Fix an ACL error if the bot or reader user is the owner.
* Fix typo
* Fix missing error case in VerifyACL causing unreadable directories
* Address review feedback
- Rename ACLOn -> ACLRequired
- Simplify fs_linux.Read()
- Add missing fs_other.Read()
- Hoist renewal loop logic into its own function
- A few misc bugfixes
* Apply suggestions from code review
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
* Address review feedback
- Only log syscall warning once
- Formatting and wording changes
- Improve error handling for `--clean`
* Fix lint error
* Fix imports in fs_other
* Fix possible nil pointer deref if storage is unset
* Use the bot user as default owner
This is more likely to be a safe owner choice than `nobody:nobody`.
* Apply suggestions from code review
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
* Code review fixes
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
Co-authored-by: Nic Klaassen <nic@goteleport.com>
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
* Disable colorized error formatting on Windows
This disables colorized error formatting on Windows. While recent
versions of Windows can render ANSI sequences, this functionality
needs to be explicitly enabled at runtime and then reset before
exiting. As it stands today, Windows clients see the raw ANSI
escapes, for example:
←[31mERROR: ←[0mfailed connecting to node example [...]
Due to compatibility concerns and `tsh`'s lack of a single exit
point, this just disables ANSI escapes when
`runtime.GOOS == "windows"`. The `ERROR:` text won't be colorized,
but also won't be surrounded by garbage characters.
* Add TODO note about why we disable colors on Windows.
Purpose is to allow users with admin privilege that are able to view audit logs,
to be able to debug SSO login failures from the UI as much as possible
* Return generic error message for sso console login failures to hide
sensitive data from reaching client. Previously errors were returning as
empty messages b/c of a trace bug.
* Remove emit event for createOIDCClient to allow outer caller to
emit event and prevent double emits on error.
* Temporarily direct users to check teleports log on errors that come back
empty to tsh client.
* Update logrus package to fix data races
* Introduce a logger that uses the test context to log the messages so they are output if a test fails for improved trouble-shooting.
* Revert introduction of test logger - simply leave logger configuration at debug level outputting to stderr during tests.
* Run integration test for e as well
* Use make with a cap and append to only copy the relevant roles.
* Address review comments
* Update integration test suite to use test-local logger that would only output logs iff a specific test has failed - no logs from other test cases will be output.
* Revert changes to InitLoggerForTests API
* Create a new logger instance when applying defaults or merging with file service configuration
* Introduce a local logger interface to be able to test file configuration merge.
* Fix kube integration tests w.r.t log
* Move goroutine profile dump into a separate func to handle parameters consistently for all invocations
* Implement kubernetes_service registration and sratup
The new service now starts, registers (locally or via a join token) and
heartbeats its presence to the auth server.
This service can handle k8s requests (like a proxy) but not to remote
teleport clusters. Proxies will be responsible for routing those.
The client (tsh) will not yet go to this service, until proxy routing is
implemented. I manually tweaked server addres in kubeconfig to test it.
You can also run `tctl get kube_service` to list all registered
instances. The self-reported info is currently limited - only listening
address is set.
* Address review feedback
* return only trace.NotFound
* supressed error message when user does not have ~/.tsh directory
* return only trace.NotFound
* handled permission issue and file path not found error
* formatting
* nit
* Make milv happy
* Revert "Make milv happy"
This reverts commit 5a24e9f725.
Co-authored-by: Ben Arent <ben@gravitational.com>
* Improve help text and error messages for tctl rm, fixes#2594
* Change 'kind' to 'type' for consistency
* Changed examples from role/admin to connector/github
* Added link to Teleport Enterprise
* Update e ref
This commit expands the usage of the caching layer
for auth server API:
* Introduces in-memory cache that is used to serve all
Auth server API requests. This is done to achieve scalability
on 10K+ node clusters, where each node fetches certificate authorities,
roles, users and join tokens. It is not possible to scale
DynamoDB backend or other backends on 10K reads per seconds
on a single shard or partition. The solution is to introduce
an in-memory cache of the backend state that is always used
for reads.
* In-memory cache has been expanded to support all resources
required by the auth server.
* Experimental `tctl top` command has been introduced to display
common single node metrics.
Replace SQLite Memory Backend with BTree
SQLite in memory backend was suffering from
high tail latencies under load (up to 8 seconds
in 99.9%-ile on load configurations).
This commit replaces the SQLite memory caching
backend with in-memory BTree backend that
brought down tail latencies to 2 seconds (99.9%-ile)
and brought overall performance improvement.
This commit introduces several key changes to
Teleport backend and API infrastructure
in order to achieve scalability improvements
on 10K+ node deployments.
Events and plain keyspace
--------------------------
New backend interface supports events,
pagination and range queries
and moves away from buckets to
plain keyspace, what better aligns
with DynamoDB and Etcd featuring similar
interfaces.
All backend implementations are
exposing Events API, allowing
multiple subscribers to consume the same
event stream and avoid polling database.
Replacing BoltDB, Dir with SQLite
-------------------------------
BoltDB backend does not support
having two processes access the database at the
same time. This prevented Teleport
using BoltDB backend to be live reloaded.
SQLite supports reads/writes by multiple
processes and makes Dir backend obsolete
as SQLite is more efficient on larger collections,
supports transactions and can detect data
corruption.
Teleport automatically migrates data from
Bolt and Dir backends into SQLite.
GRPC API and protobuf resources
-------------------------------
GRPC API has been introduced for
the auth server. The auth server now serves both GRPC
and JSON-HTTP API on the same TLS socket and uses
the same client certificate authentication.
All future API methods should use GRPC and HTTP-JSON
API is considered obsolete.
In addition to that some resources like
Server and CertificateAuthority are now
generated from protobuf service specifications in
a way that is fully backward compatible with
original JSON spec and schema, so the same resource
can be encoded and decoded from JSON, YAML
and protobuf.
All models should be refactored
into new proto specification over time.
Streaming presence service
--------------------------
In order to cut bandwidth, nodes
are sending full updates only when changes
to labels or spec have occured, otherwise
new light-weight GRPC keep alive updates are sent
over to the presence service, reducing
bandwidth usage on multi-node deployments.
In addition to that nodes are no longer polling
auth server for certificate authority rotation
updates, instead they subscribe to event updates
to detect updates as soon as they happen.
This is a new API, so the errors are inevitable,
that's why polling is still done, but
on a way slower rate.
This commit fixes#1803, fixes#1889
* Adds support for public_addr for Proxy and Auth
* Parameter advertise_ip now supports host:port format
* Fixes incorrect output for tctl get proxies
* Fixes duplicate output of some error messages.
STeve (Xin) Huang