Skip to content

feat: auto detect tee env #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

feat: auto detect tee env #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e36bf5b
feat: auto detect tee env
Jan 28, 2025
80df1e4
feat: fetches validator attestation type from measurments, removes flag
Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/attested-get/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func runClient(cCtx *cli.Context) (err error) {
}

// Create validators based on the attestation type
attestationType, err := proxy.ParseAttestationType(attestationTypeStr)
attestationType, err := proxy.ParseAttestationType(log, attestationTypeStr)
if err != nil {
log.With("attestation-type", attestationType).Error("invalid attestation-type passed, see --help")
return err
Expand Down
28 changes: 9 additions & 19 deletions cmd/proxy-client/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ var flags []cli.Flag = []cli.Flag{
Value: "https://localhost:80",
Usage: "address to proxy requests to",
},
&cli.StringFlag{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could always simply deprecate the flag instead of removing it right away — the behavior should be the same and the flag could be ignored until we are confident we can safely remove it

Name: "server-attestation-type",
Value: string(proxy.AttestationAzureTDX),
Usage: "type of attestation to expect and verify (" + proxy.AvailableAttestationTypes + ")",
},
&cli.StringFlag{
Name: "server-measurements",
Usage: "optional path to JSON measurements enforced on the server",
Expand All @@ -45,8 +40,8 @@ var flags []cli.Flag = []cli.Flag{
},
&cli.StringFlag{
Name: "client-attestation-type",
Value: string(proxy.AttestationNone),
Usage: "type of attestation to present (" + proxy.AvailableAttestationTypes + ")",
Value: "auto",
Copy link
Collaborator

@Ruteri Ruteri Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the default should still be none — this is mostly still used to verify the server's attestation without providing client's own attestation. "auto" should be an option for the client to select if needed

Usage: "type of attestation to present (" + proxy.AvailableAttestationTypes + "). If not set, automatically detected.",
},
&cli.BoolFlag{
Name: "log-json",
Expand Down Expand Up @@ -96,32 +91,27 @@ func runClient(cCtx *cli.Context) error {
Version: common.Version,
})

if cCtx.String("server-attestation-type") != "none" && verifyTLS {
log.Error("invalid combination of --verify-tls and --server-attestation-type passed (only 'none' is allowed)")
return errors.New("invalid combination of --verify-tls and --server-attestation-type passed (only 'none' is allowed)")
if serverMeasurements != "" && verifyTLS {
log.Error("invalid combination of --verify-tls and --server-measurements passed (cannot add server measurements and verify default TLS at the same time)")
Copy link
Contributor Author

@fnerdman fnerdman Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't check for server-attestation-type anymore since the type was removed as it is automatically selected based on OID during handshake.
Checking if serverMeasurements are set is an alternative, as serverMeasurements will include the measurements and attestationTypes we approve.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the differences and purpose of the checks before and now after. But would using the auto detect type here help instead and check if it is not "none"?
What is this check for? (just for my understanding)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so as a client, you are building up a connection to a server. That server will either provide a ca signed TLS cert, or a self signed aTLS cert. This check is just to make sure what you are doing, and prevent a configuration where you accidentally set verifyTLS but actually you wanted to verify measurements. Taking this example, if this check wouldn't be there, the client would verify a signed TLS cert and not the TEE attestation, thus undermining security. It's just a clear separation for security purposes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment as elsewhere — no expected measurement is not the same as not expected attestation

return errors.New("invalid combination of --verify-tls and --server-measurements passed (cannot add server measurements and verify default TLS at the same time)")
}

clientAttestationType, err := proxy.ParseAttestationType(cCtx.String("client-attestation-type"))
// Auto-detect client attestation type if not specified
clientAttestationType, err := proxy.ParseAttestationType(log, cCtx.String("client-attestation-type"))
if err != nil {
log.With("attestation-type", cCtx.String("client-attestation-type")).Error("invalid client-attestation-type passed, see --help")
return err
}

serverAttestationType, err := proxy.ParseAttestationType(cCtx.String("server-attestation-type"))
if err != nil {
log.With("attestation-type", cCtx.String("server-attestation-type")).Error("invalid server-attestation-type passed, see --help")
return err
}

issuer, err := proxy.CreateAttestationIssuer(log, clientAttestationType)
if err != nil {
log.Error("could not create attestation issuer", "err", err)
return err
}

validators, err := proxy.CreateAttestationValidators(log, serverAttestationType, serverMeasurements)
validators, err := proxy.CreateAttestationValidatorsFromFile(log, serverMeasurements)
if err != nil {
log.Error("could not create attestation validators", "err", err)
log.Error("could not create attestation validators from file", "err", err)
return err
}

Expand Down
23 changes: 6 additions & 17 deletions cmd/proxy-server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ var flags []cli.Flag = []cli.Flag{
&cli.StringFlag{
Name: "server-attestation-type",
EnvVars: []string{"SERVER_ATTESTATION_TYPE"},
Value: string(proxy.AttestationAzureTDX),
Usage: "type of attestation to present (" + proxy.AvailableAttestationTypes + ")",
Value: "auto",
Usage: "type of attestation to present (" + proxy.AvailableAttestationTypes + "). If not set, automatically detected.",
},
&cli.StringFlag{
Name: "tls-certificate-path",
Expand All @@ -53,12 +53,6 @@ var flags []cli.Flag = []cli.Flag{
EnvVars: []string{"TLS_PRIVATE_KEY_PATH"},
Usage: "Path to private key file for the certificate. Only valid with --tls-certificate-path",
},
&cli.StringFlag{
Name: "client-attestation-type",
EnvVars: []string{"CLIENT_ATTESTATION_TYPE"},
Value: string(proxy.AttestationNone),
Usage: "type of attestation to expect and verify (" + proxy.AvailableAttestationTypes + ")",
},
&cli.StringFlag{
Name: "client-measurements",
EnvVars: []string{"CLIENT_MEASUREMENTS"},
Expand Down Expand Up @@ -132,21 +126,16 @@ func runServer(cCtx *cli.Context) error {
return errors.New("not all of --tls-certificate-path and --tls-private-key-path specified")
}

serverAttestationType, err := proxy.ParseAttestationType(serverAttestationTypeFlag)
// Auto-detect server attestation type if not specified
serverAttestationType, err := proxy.ParseAttestationType(log, cCtx.String("server-attestation-type"))
if err != nil {
log.With("attestation-type", cCtx.String("server-attestation-type")).Error("invalid server-attestation-type passed, see --help")
return err
}

clientAttestationType, err := proxy.ParseAttestationType(cCtx.String("client-attestation-type"))
if err != nil {
log.With("attestation-type", cCtx.String("client-attestation-type")).Error("invalid client-attestation-type passed, see --help")
return err
}

validators, err := proxy.CreateAttestationValidators(log, clientAttestationType, clientMeasurements)
validators, err := proxy.CreateAttestationValidatorsFromFile(log, clientMeasurements)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, this logic seems incorrect. doesn't this need attestation types to be present to actually verify the client attestation? this should not be loaded from a file, but instead from the client TCP connection!

the job of this proxy is NOT to gatekeep, it's to check the attestation and then pass it on for later services to reject or accept this measurement!

Copy link
Collaborator

@Ruteri Ruteri Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue is I think confusion of "none" attestation type with no expected measurements.
@fnerdman non-none attestation type with no expected measurements is a valid configuration with the meaning to authenticate the attestation without authorizing it, whereas now it's impossible (with a single measurements flag). What is impossible exactly is the difference between expecting no attestation (error if provided) and expecting an attestation, but not any particular one (error if not provided or if invalid) — although we could probably change "no measurements" to mean "error if provided but not valid". I think the products handle this case correctly — since we will simply not be forwarding any attestation report.

What I'd suggest is to re-introduce the client attestation type flag, and the configuration of "auto" client attestation type with no measurements retains the previous meaning — validate any submitted attestation without expecting any particular one.
The same applies to the client, as the error really is introduced in the first check of CreateAttestationValidatorsFromFile.
The alternative is to change the meaning of "none" attestation type and no measurements to "validate all provided attestations, no attestation — no problem"

if err != nil {
log.Error("could not create attestation validators", "err", err)
log.Error("could not create attestation validators from file", "err", err)
return err
}

Expand Down
76 changes: 58 additions & 18 deletions proxy/atls_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,39 @@ import (
type AttestationType string

const (
AttestationAuto AttestationType = "auto"
AttestationNone AttestationType = "none"
AttestationAzureTDX AttestationType = "azure-tdx"
AttestationDCAPTDX AttestationType = "dcap-tdx"
)

const AvailableAttestationTypes string = "none, azure-tdx, dcap-tdx"
const AvailableAttestationTypes string = "auto, none, azure-tdx, dcap-tdx"

// DetectAttestationType determines the attestation type based on environment
func DetectAttestationType() AttestationType {
// Check for TDX device files - these indicate DCAP TDX
_, tdxErr1 := os.Stat("/dev/tdx-guest")
_, tdxErr2 := os.Stat("/dev/tdx_guest")
if tdxErr1 == nil || tdxErr2 == nil {
return AttestationDCAPTDX
}

// Try Azure TDX attestation - if it works, we're in Azure TDX
issuer := azure_tdx.NewIssuer(nil) // nil logger for detection
_, err := issuer.Issue(context.Background(), []byte("test"), []byte("test"))
if err == nil {
return AttestationAzureTDX
}

return AttestationNone
}

func ParseAttestationType(attestationType string) (AttestationType, error) {
func ParseAttestationType(log *slog.Logger, attestationType string) (AttestationType, error) {
switch attestationType {
case string(AttestationAuto):
detectedType := DetectAttestationType()
log.With("detected_attestation", detectedType).Info("Auto-detected attestation type")
return detectedType, nil
Comment on lines +53 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: instead of changing the function signature to add logging for the extra new switch case, it could have been handled outside after calling the function and keep the signature as it was before for compatibility and avoid changing it everywhere it was called before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to forward the log var than adding logic for extracting this log emit of a subcondition that might not be hit.

Copy link
Collaborator

@Ruteri Ruteri Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "autodetect" should be done here, since it doesn't really make sense to do in some cases (you can't detect someone else's attestation type, while you can parse it just fine from a string)
I think the issue is that autodetect is not really an attestation type, I would probably distinguish between parsing attestation type string to a type from "resolving" auto to a type of attestation.
Resolving the attestation type would make much more sense in CreateAttestationIssuer if you'd want to still fit it in the currently existing functions.

case string(AttestationNone):
return AttestationNone, nil
case string(AttestationAzureTDX):
Expand All @@ -56,8 +80,8 @@ func CreateAttestationIssuer(log *slog.Logger, attestationType AttestationType)
}
}

func CreateAttestationValidators(log *slog.Logger, attestationType AttestationType, jsonMeasurementsPath string) ([]atls.Validator, error) {
if attestationType == AttestationNone {
func CreateAttestationValidatorsFromFile(log *slog.Logger, jsonMeasurementsPath string) ([]atls.Validator, error) {
if jsonMeasurementsPath == "" {
return nil, nil
}

Expand All @@ -72,26 +96,42 @@ func CreateAttestationValidators(log *slog.Logger, attestationType AttestationTy
return nil, err
}

switch attestationType {
case AttestationAzureTDX:
validators := []atls.Validator{}
for _, measurement := range parsedMeasurements {
// Group validators by attestation type
validatorsByType := make(map[AttestationType][]atls.Validator)

for _, measurement := range parsedMeasurements {
attestationType, err := ParseAttestationType(log, measurement.AttestationType)
if err != nil {
return nil, fmt.Errorf("invalid attestation type %s in measurements file", measurement.AttestationType)
}

switch attestationType {
case AttestationAzureTDX:
attConfig := config.DefaultForAzureTDX()
attConfig.SetMeasurements(measurement.Measurements)
validators = append(validators, azure_tdx.NewValidator(attConfig, AttestationLogger{Log: log}))
}
return []atls.Validator{NewMultiValidator(validators)}, nil
case AttestationDCAPTDX:
validators := []atls.Validator{}
for _, measurement := range parsedMeasurements {
validatorsByType[attestationType] = append(
validatorsByType[attestationType],
azure_tdx.NewValidator(attConfig, AttestationLogger{Log: log}),
)
case AttestationDCAPTDX:
attConfig := &config.QEMUTDX{Measurements: measurements.DefaultsFor(cloudprovider.QEMU, variant.QEMUTDX{})}
attConfig.SetMeasurements(measurement.Measurements)
validators = append(validators, dcap_tdx.NewValidator(attConfig, AttestationLogger{Log: log}))
validatorsByType[attestationType] = append(
validatorsByType[attestationType],
dcap_tdx.NewValidator(attConfig, AttestationLogger{Log: log}),
)
default:
return nil, fmt.Errorf("unsupported attestation type %s in measurements file", measurement.AttestationType)
}
return []atls.Validator{NewMultiValidator(validators)}, nil
default:
return nil, errors.New("invalid attestation-type passed in")
}

// Create a MultiValidator for each attestation type
var validators []atls.Validator
for _, typeValidators := range validatorsByType {
validators = append(validators, NewMultiValidator(typeValidators))
}

return validators, nil
}

func ExtractMeasurementsFromExtension(ext *pkix.Extension, v variant.Variant) (map[uint32][]byte, error) {
Expand Down