Skip to content

Commit

Permalink
Convert lib/devicetrust to use slog (#50458)
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored Dec 19, 2024
1 parent 6b80477 commit 47c0216
Show file tree
Hide file tree
Showing 13 changed files with 110 additions and 79 deletions.
8 changes: 4 additions & 4 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -3021,7 +3021,7 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
if err != nil {
return nil, trace.Wrap(err)
}
if err := a.verifyUserDeviceForCertIssuance(req.Usage, readOnlyAuthPref.GetDeviceTrust()); err != nil {
if err := a.verifyUserDeviceForCertIssuance(ctx, req.Usage, readOnlyAuthPref.GetDeviceTrust()); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -3417,14 +3417,14 @@ func (a *ServerWithRoles) GetAccessRequestAllowedPromotions(ctx context.Context,
// is not paramount to the access system itself, but it stops bad attempts from
// progressing further and provides better feedback than other protocol-specific
// failures.
func (a *ServerWithRoles) verifyUserDeviceForCertIssuance(usage proto.UserCertsRequest_CertUsage, dt *types.DeviceTrust) error {
func (a *ServerWithRoles) verifyUserDeviceForCertIssuance(ctx context.Context, usage proto.UserCertsRequest_CertUsage, dt *types.DeviceTrust) error {
// Ignore App or WindowsDeskop requests, they do not support device trust.
if usage == proto.UserCertsRequest_App || usage == proto.UserCertsRequest_WindowsDesktop {
return nil
}

identity := a.context.Identity.GetIdentity()
return trace.Wrap(dtauthz.VerifyTLSUser(dt, identity))
return trace.Wrap(dtauthz.VerifyTLSUser(ctx, dt, identity))
}

func (a *ServerWithRoles) CreateResetPasswordToken(ctx context.Context, req authclient.CreateUserTokenRequest) (types.UserToken, error) {
Expand Down Expand Up @@ -6813,7 +6813,7 @@ func (a *ServerWithRoles) enforceGlobalModeTrustedDevice(ctx context.Context) er
return trace.Wrap(err)
}

err = dtauthz.VerifyTLSUser(readOnlyAuthPref.GetDeviceTrust(), a.context.Identity.GetIdentity())
err = dtauthz.VerifyTLSUser(ctx, readOnlyAuthPref.GetDeviceTrust(), a.context.Identity.GetIdentity())
return trace.Wrap(err)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/authz/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func (a *authorizer) Authorize(ctx context.Context) (authCtx *Context, err error

// Device Trust: authorize device extensions.
if !a.disableGlobalDeviceMode {
if err := dtauthz.VerifyTLSUser(authPref.GetDeviceTrust(), authContext.Identity.GetIdentity()); err != nil {
if err := dtauthz.VerifyTLSUser(ctx, authPref.GetDeviceTrust(), authContext.Identity.GetIdentity()); err != nil {
return nil, trace.Wrap(err)
}
}
Expand Down
18 changes: 9 additions & 9 deletions lib/devicetrust/authz/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
package authz

import (
"context"
"log/slog"

"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport"
Expand All @@ -45,8 +47,8 @@ func IsTLSDeviceVerified(ext *tlsca.DeviceExtensions) bool {

// VerifyTLSUser verifies if the TLS identity has the required extensions to
// fulfill the device trust configuration.
func VerifyTLSUser(dt *types.DeviceTrust, identity tlsca.Identity) error {
return verifyDeviceExtensions(dt, identity.Username, IsTLSDeviceVerified(&identity.DeviceExtensions))
func VerifyTLSUser(ctx context.Context, dt *types.DeviceTrust, identity tlsca.Identity) error {
return verifyDeviceExtensions(ctx, dt, identity.Username, IsTLSDeviceVerified(&identity.DeviceExtensions))
}

// IsSSHDeviceVerified returns true if cert contains all required device
Expand Down Expand Up @@ -83,24 +85,22 @@ func HasDeviceTrustExtensions(extensions []string) bool {

// VerifySSHUser verifies if the SSH certificate has the required extensions to
// fulfill the device trust configuration.
func VerifySSHUser(dt *types.DeviceTrust, cert *ssh.Certificate) error {
func VerifySSHUser(ctx context.Context, dt *types.DeviceTrust, cert *ssh.Certificate) error {
if cert == nil {
return trace.BadParameter("cert required")
}

username := cert.KeyId
return verifyDeviceExtensions(dt, username, IsSSHDeviceVerified(cert))
return verifyDeviceExtensions(ctx, dt, username, IsSSHDeviceVerified(cert))
}

func verifyDeviceExtensions(dt *types.DeviceTrust, username string, verified bool) error {
func verifyDeviceExtensions(ctx context.Context, dt *types.DeviceTrust, username string, verified bool) error {
mode := dtconfig.GetEnforcementMode(dt)
switch {
case mode != constants.DeviceTrustModeRequired:
return nil // OK, extensions not enforced.
case !verified:
log.
WithField("User", username).
Debug("Device Trust: denied access for unidentified device")
slog.DebugContext(ctx, "Device Trust: denied access for unidentified device", "user", username)
return trace.Wrap(ErrTrustedDeviceRequired)
default:
return nil
Expand Down
5 changes: 3 additions & 2 deletions lib/devicetrust/authz/authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package authz_test

import (
"context"
"testing"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -131,7 +132,7 @@ func testIsDeviceVerified(t *testing.T, name string, fn func(ext *tlsca.DeviceEx

func TestVerifyTLSUser(t *testing.T) {
runVerifyUserTest(t, "VerifyTLSUser", func(dt *types.DeviceTrust, ext *tlsca.DeviceExtensions) error {
return authz.VerifyTLSUser(dt, tlsca.Identity{
return authz.VerifyTLSUser(context.Background(), dt, tlsca.Identity{
Username: "llama",
DeviceExtensions: *ext,
})
Expand All @@ -140,7 +141,7 @@ func TestVerifyTLSUser(t *testing.T) {

func TestVerifySSHUser(t *testing.T) {
runVerifyUserTest(t, "VerifySSHUser", func(dt *types.DeviceTrust, ext *tlsca.DeviceExtensions) error {
return authz.VerifySSHUser(dt, &ssh.Certificate{
return authz.VerifySSHUser(context.Background(), dt, &ssh.Certificate{
KeyId: "llama",
Permissions: ssh.Permissions{
Extensions: map[string]string{
Expand Down
25 changes: 15 additions & 10 deletions lib/devicetrust/enroll/enroll.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"context"
"errors"
"io"
"log/slog"

"github.com/gravitational/trace"
"github.com/gravitational/trace/trail"
log "github.com/sirupsen/logrus"

devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
"github.com/gravitational/teleport/lib/devicetrust"
Expand Down Expand Up @@ -94,8 +94,7 @@ func (c *Ceremony) RunAdmin(

rewordAccessDenied := func(err error, action string) error {
if trace.IsAccessDenied(trail.FromGRPC(err)) {
log.WithError(err).Debug(
"Device Trust: Redacting access denied error with user-friendly message")
slog.DebugContext(ctx, "Device Trust: Redacting access denied error with user-friendly message", "error", err)
return trace.AccessDenied(
"User does not have permissions to %s. Contact your cluster device administrator.",
action,
Expand All @@ -115,9 +114,12 @@ func (c *Ceremony) RunAdmin(
for _, dev := range findResp.Devices {
if dev.OsType == osType {
currentDev = dev
log.Debugf(
"Device Trust: Found device %q/%v, id=%q",
currentDev.AssetTag, devicetrust.FriendlyOSType(currentDev.OsType), currentDev.Id,
slog.DebugContext(ctx, "Device Trust: Found device",
slog.Group("device",
slog.String("asset_tag", currentDev.AssetTag),
slog.String("os", devicetrust.FriendlyOSType(currentDev.OsType)),
slog.String("id", currentDev.Id),
),
)
break
}
Expand Down Expand Up @@ -148,10 +150,13 @@ func (c *Ceremony) RunAdmin(
if err != nil {
return currentDev, outcome, trace.Wrap(rewordAccessDenied(err, "create device enrollment tokens"))
}
log.Debugf(
"Device Trust: Created enrollment token for device %q/%s",
currentDev.AssetTag,
devicetrust.FriendlyOSType(currentDev.OsType))
slog.DebugContext(ctx, "Device Trust: Created enrollment token for device",
slog.Group("device",
slog.String("asset_tag", currentDev.AssetTag),
slog.String("os", devicetrust.FriendlyOSType(currentDev.OsType)),
slog.String("id", currentDev.Id),
),
)
}
token := currentDev.EnrollToken.GetToken()

Expand Down
7 changes: 4 additions & 3 deletions lib/devicetrust/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
package devicetrust

import (
"context"
"errors"
"io"
"log/slog"

"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand All @@ -48,14 +49,14 @@ func HandleUnimplemented(err error) error {
const notSupportedMsg = "device trust not supported by remote cluster"

if errors.Is(err, io.EOF) {
log.Debug("Device Trust: interpreting EOF as an older Teleport cluster")
slog.DebugContext(context.Background(), "Device Trust: interpreting EOF as an older Teleport cluster")
return trace.NotImplemented(notSupportedMsg)
}

for e := err; e != nil; {
switch s, ok := status.FromError(e); {
case ok && s.Code() == codes.Unimplemented:
log.WithError(err).Debug("Device Trust: interpreting gRPC Unimplemented as OSS or older Enterprise cluster")
slog.DebugContext(context.Background(), "Device Trust: interpreting gRPC Unimplemented as OSS or older Enterprise cluster", "error", err)
return trace.NotImplemented(notSupportedMsg)
case ok:
return err // Unexpected status error.
Expand Down
5 changes: 3 additions & 2 deletions lib/devicetrust/native/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
package native

import (
"context"
"log/slog"
"runtime"
"sync"
"time"

"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"

Expand Down Expand Up @@ -88,7 +89,7 @@ func readCachedDeviceDataUnderLock(mode CollectDataMode) (cdd *devicepb.DeviceCo
return nil, false
}

log.Debug("Device Trust: Using in-process cached device data")
slog.DebugContext(context.Background(), "Device Trust: Using in-process cached device data")
cdd = proto.Clone(cachedDeviceData.value).(*devicepb.DeviceCollectedData)
cdd.CollectTime = timestamppb.Now()
return cdd, true
Expand Down
7 changes: 4 additions & 3 deletions lib/devicetrust/native/device_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ import "C"

import (
"bytes"
"context"
"crypto/sha256"
"crypto/x509"
"errors"
"fmt"
"io/fs"
"log/slog"
"os/exec"
"os/user"
"strings"
Expand All @@ -40,7 +42,6 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/timestamppb"

devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
Expand Down Expand Up @@ -139,7 +140,7 @@ func collectDeviceData(_ CollectDataMode) (*devicepb.DeviceCollectedData, error)
defer wg.Done()
out, err := spec.fn()
if err != nil {
log.WithError(err).Warnf("Device Trust: Failed to get %v", spec.desc)
slog.WarnContext(context.Background(), "Device Trust: Failed to get device details", "details", spec.desc, "error", err)
return
}
*spec.out = out
Expand Down Expand Up @@ -179,7 +180,7 @@ func getJamfBinaryVersion() (string, error) {
// Jamf binary may not exist. This is alright.
pathErr := &fs.PathError{}
if errors.As(err, &pathErr) {
log.Debugf("Device Trust: Jamf binary not found: %q", pathErr.Path)
slog.DebugContext(context.Background(), "Device Trust: Jamf binary not found", "binary_path", pathErr.Path)
return "", nil
}

Expand Down
34 changes: 20 additions & 14 deletions lib/devicetrust/native/device_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"os/exec"
"os/user"
Expand All @@ -34,9 +35,9 @@ import (

"github.com/google/go-attestation/attest"
"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/gravitational/teleport"
devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
"github.com/gravitational/teleport/lib/linux"
)
Expand Down Expand Up @@ -104,9 +105,10 @@ func rewriteTPMPermissionError(err error) error {
if !errors.As(err, &pathErr) || pathErr.Path != "/dev/tpmrm0" {
return err
}
log.
WithError(err).
Debug("TPM: Replacing TPM permission error with a more friendly one")
slog.DebugContext(context.Background(), "Replacing TPM permission error with a more friendly one",
teleport.ComponentKey, "TPM",
"error", err,
)

return errors.New("" +
"Failed to open the TPM device. " +
Expand Down Expand Up @@ -141,7 +143,10 @@ func collectDeviceData(mode CollectDataMode) (*devicepb.DeviceCollectedData, err
go func() {
osRelease, err := cddFuncs.parseOSRelease()
if err != nil {
log.WithError(err).Debug("TPM: Failed to parse /etc/os-release file")
slog.DebugContext(context.Background(), "Failed to parse /etc/os-release file",
teleport.ComponentKey, "TPM",
"error", err,
)
// err swallowed on purpose.

osRelease = &linux.OSRelease{}
Expand Down Expand Up @@ -187,42 +192,45 @@ func collectDeviceData(mode CollectDataMode) (*devicepb.DeviceCollectedData, err
}

func readDMIInfoAccordingToMode(mode CollectDataMode) (*linux.DMIInfo, error) {
ctx := context.Background()
logger := slog.With(teleport.ComponentKey, "TPM")

dmiInfo, err := cddFuncs.dmiInfoFromSysfs()
if err == nil {
return dmiInfo, nil
}

log.WithError(err).Warn("TPM: Failed to read device model and/or serial numbers")
logger.WarnContext(ctx, "Failed to read device model and/or serial numbers", "error", err)
if !errors.Is(err, fs.ErrPermission) {
return dmiInfo, nil // original info
}

switch mode {
case CollectedDataNeverEscalate, CollectedDataMaybeEscalate:
log.Debug("TPM: Reading cached DMI info")
logger.DebugContext(ctx, "Reading cached DMI info")

dmiCached, err := cddFuncs.readDMIInfoCached()
if err == nil {
return dmiCached, nil // successful cache hit
}

log.WithError(err).Debug("TPM: Failed to read cached DMI info")
logger.DebugContext(ctx, "Failed to read cached DMI info", "error", err)
if mode == CollectedDataNeverEscalate {
return dmiInfo, nil // original info
}

fallthrough

case CollectedDataAlwaysEscalate:
log.Debug("TPM: Running escalated `tsh device dmi-info`")
logger.DebugContext(ctx, "Running escalated `tsh device dmi-info`")

dmiInfo, err = cddFuncs.readDMIInfoEscalated()
if err != nil {
return nil, trace.Wrap(err) // actual failure, abort
}

if err := cddFuncs.saveDMIInfoToCache(dmiInfo); err != nil {
log.WithError(err).Warn("TPM: Failed to write DMI cache")
logger.WarnContext(ctx, "Failed to write DMI cache", "error", err)
// err swallowed on purpose.
}
}
Expand Down Expand Up @@ -250,9 +258,7 @@ func readDMIInfoCached() (*linux.DMIInfo, error) {
return nil, trace.Wrap(err)
}
if dec.More() {
log.
WithField("Path", path).
Warn("DMI cache file contains multiple JSON entries, only one expected")
slog.WarnContext(context.Background(), "DMI cache file contains multiple JSON entries, only one expected", "path", path)
// Warn but keep going.
}

Expand Down Expand Up @@ -320,7 +326,7 @@ func saveDMIInfoToCache(dmiInfo *linux.DMIInfo) error {
if err := f.Close(); err != nil {
return trace.Wrap(err, "closing dmi.json after write")
}
log.Debug("TPM: Saved DMI information to local cache")
slog.DebugContext(context.Background(), "Saved DMI information to local cache", teleport.ComponentKey, "TPM")

return nil
}
Loading

0 comments on commit 47c0216

Please sign in to comment.