Windows TPM enrollment support by strideynet · Pull Request #25801 · gravitational/teleport

strideynet · 2023-05-08T13:58:21Z

As per https://github.com/gravitational/teleport.e/pull/911
Closes https://github.com/gravitational/teleport.e/issues/1163
Associated with https://github.com/gravitational/teleport.e/pull/1159 .

DeviceCollectedData {
  "collect_time": "2023-05-11T15:29:03.305882500Z",
  "os_type": "OS_TYPE_WINDOWS",
  "serial_number": "winaia_1337",
  "model_identifier": "21J50013US",
  "os_version": "10.0.22621",
  "os_username": "LAPTOP-LS1NN4BK\\noahstride",
  "reported_asset_tag": "winaia_1337",
  "system_serial_number": "PF47WND6",
  "base_board_serial_number": "L1HF2CM03ZT"
}

…tpm-enrollment

codingllama

Looks great, the vast majority of comments are inconsequential.

Apologies for the delay and thanks for splitting the other parts.

codingllama

Looks great, the vast majority of comments are inconsequential.

Apologies for the delay and thanks for splitting the other parts.

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

strideynet · 2023-05-22T08:00:12Z

@codingllama ready for you to take another look 🦦

codingllama · 2023-05-22T15:09:43Z

+	}
+}
+
+// getDeviceSerial returns the serial number of the device using PowerShell to


Thanks for the detailed explanation, sounds good.

public-teleport-github-review-bot · 2023-05-22T17:09:17Z

@strideynet See the table below for backport results.

Branch	Result
branch/v13	Failed

zmb3 · 2023-05-22T15:56:19Z

+		if os.IsNotExist(err) {
+			// If it doesn't exist, we can create it and return as we know
+			// the perms are correct as we created it.
+			if err := os.Mkdir(deviceStateDirPath, 700); err != nil {


How about MkdirAll?

zmb3 · 2023-05-22T15:58:49Z

+	}
+}
+
+// getDeviceSerial returns the serial number of the device using PowerShell to


cc @probakowski who has some experience calling win32 APIs via syscall. Przemko, do you think that would make sense here?

zmb3 · 2023-05-22T15:59:43Z

+	// PF47WND6
+	out, err := cmd.Output()
+	if err != nil {
+		return "", trace.Wrap(err)


I find it helpful to include out in the error message here, otherwise when this fails you'll just get an error message about a non-zero exit code and have no idea what went wrong.

* Start implementation of Windows TPM enrollment * Basic device data collection for windows * Add AK get/creatiom * Add helpers for converting tpm protos * Don't create AK in inappropriate circumstances * Furhter simplify AK load/create * Add tests for proto/attest conversions * Ensure that digestalg varies between test cases * More accurate proto field name * Missing license header * Add credential fingerprinting function * Add getDeviceCredential implementation for windows * Add dependencies so this builds * Fix generation of credential id * Introduce AKPublic field * Collect other key data * Add some additional debug logging * Add more specific serial number fields to dcd * Use faster powershell call for determining OS version * Fix missing field in DeviceFromResource * Add link to to-do issue * Add packages necessary for enterprise submodule * Fix import orders * bump go-tpm-tools to latest versions * Tidy up returned errors * Add failure case test for Linux enrollment * move linux device fake to lib/devicetrust/testenv * Add test to exercise `RunCeremony` * Tidier assertion messages Co-authored-by: Alan Parra <alan.parra@goteleport.com> * Further simplifcations of test assertions/errors Co-authored-by: Alan Parra <alan.parra@goteleport.com> * Apply suggestions from code review Co-authored-by: Alan Parra <alan.parra@goteleport.com> * Further fixes as per the llama's suggestions * Further simplification and header on logs * Use BadParameter rather than platform unsupported * Add further notes on RSAness of `go-attestation` * Minor adjustments to comments * Unused import removed * License headers * rename `mustRandomBytes` -> `randomBytes` --------- Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Start implementation of Windows TPM enrollment * Basic device data collection for windows * Add AK get/creatiom * Add helpers for converting tpm protos * Don't create AK in inappropriate circumstances * Furhter simplify AK load/create * Add tests for proto/attest conversions * Ensure that digestalg varies between test cases * More accurate proto field name * Missing license header * Add credential fingerprinting function * Add getDeviceCredential implementation for windows * Add dependencies so this builds * Fix generation of credential id * Introduce AKPublic field * Collect other key data * Add some additional debug logging * Add more specific serial number fields to dcd * Use faster powershell call for determining OS version * Fix missing field in DeviceFromResource * Add link to to-do issue * Add packages necessary for enterprise submodule * Fix import orders * bump go-tpm-tools to latest versions * Tidy up returned errors * Add failure case test for Linux enrollment * move linux device fake to lib/devicetrust/testenv * Add test to exercise `RunCeremony` * Tidier assertion messages * Further simplifcations of test assertions/errors * Apply suggestions from code review * Further fixes as per the llama's suggestions * Further simplification and header on logs * Use BadParameter rather than platform unsupported * Add further notes on RSAness of `go-attestation` * Minor adjustments to comments * Unused import removed * License headers * rename `mustRandomBytes` -> `randomBytes` --------- Co-authored-by: Alan Parra <alan.parra@goteleport.com>

strideynet mentioned this pull request May 8, 2023

Implement Windows TPM support for Device Trust #22313

Closed

strideynet added 13 commits May 10, 2023 12:44

Start implementation of Windows TPM enrollment

813695d

Basic device data collection for windows

e6137d1

Add AK get/creatiom

b606716

Add helpers for converting tpm protos

2ca61e5

Don't create AK in inappropriate circumstances

54efbc2

Furhter simplify AK load/create

11cdee7

Add tests for proto/attest conversions

d0a5bcb

Ensure that digestalg varies between test cases

a1eca1e

More accurate proto field name

4960b43

Missing license header

90b9c49

Add credential fingerprinting function

57509b9

Add getDeviceCredential implementation for windows

3b048ec

Add dependencies so this builds

2c958ba

strideynet force-pushed the strideynet/windows-tpm-enrollment branch from a5cafe7 to 2c958ba Compare May 10, 2023 11:45

strideynet added 8 commits May 10, 2023 15:28

Fix generation of credential id

bcf5f43

Introduce AKPublic field

5a4ec19

Collect other key data

410dc76

Add some additional debug logging

9e5bae4

Add more specific serial number fields to dcd

91487c7

Use faster powershell call for determining OS version

a37a6c6

Fix missing field in DeviceFromResource

ed98cbd

Add link to to-do issue

d5b2202

strideynet marked this pull request as ready for review May 15, 2023 11:41

github-actions Bot added the size/lg label May 15, 2023

github-actions Bot requested review from codingllama and lxea May 15, 2023 11:42

strideynet added 3 commits May 15, 2023 13:41

Merge remote-tracking branch 'origin/master' into strideynet/windows-…

f743ce4

…tpm-enrollment

Add packages necessary for enterprise submodule

990b443

Fix import orders

dc06f74

strideynet requested a review from codingllama May 17, 2023 12:20

codingllama reviewed May 17, 2023

View reviewed changes

strideynet and others added 8 commits May 17, 2023 22:37

Tidier assertion messages

c69f99c

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

Further simplifcations of test assertions/errors

14f5d72

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

Apply suggestions from code review

a422d2a

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

Further fixes as per the llama's suggestions

0acf776

Further simplification and header on logs

972eaa9

Use BadParameter rather than platform unsupported

56bf50f

Add further notes on RSAness of go-attestation

027de3b

Minor adjustments to comments

a9122c0

strideynet requested a review from codingllama May 22, 2023 07:59

Unused import removed

78fc1f9

codingllama reviewed May 22, 2023

View reviewed changes

Comment thread lib/devicetrust/testenv/fake_device_service.go Outdated

Comment thread lib/devicetrust/enroll/enroll.go Outdated

codingllama approved these changes May 22, 2023

View reviewed changes

strideynet added 2 commits May 22, 2023 17:22

License headers

188e7ef

rename mustRandomBytes -> randomBytes

751017c

tigrato approved these changes May 22, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed the request for review from lxea May 22, 2023 16:43

strideynet added this pull request to the merge queue May 22, 2023

Merged via the queue into master with commit 13dfbbc May 22, 2023

strideynet deleted the strideynet/windows-tpm-enrollment branch May 22, 2023 17:08

zmb3 reviewed May 22, 2023

View reviewed changes

strideynet mentioned this pull request May 22, 2023

[v13] Windows TPM enrollment support (#25801) #26736

Merged

r0mant mentioned this pull request Jul 14, 2023

Release 13.2.2 #29158

Closed

Conversation

strideynet commented May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codingllama left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codingllama left a comment

Choose a reason for hiding this comment

Uh oh!

strideynet commented May 22, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codingllama May 22, 2023

Choose a reason for hiding this comment

Uh oh!

public-teleport-github-review-bot Bot commented May 22, 2023

Uh oh!

Uh oh!

zmb3 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

zmb3 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

zmb3 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

strideynet commented May 8, 2023 •

edited

Loading