WIP: Added vts and provisioning plugins for AWS Nitro enclave attestation … #58

dreemkiller · 2022-11-12T19:30:19Z

…documents

…documents Signed-off-by: Derek D. Miller <[email protected]>

Signed-off-by: Derek D. Miller <[email protected]>

thomas-fossati

hi @dreemkiller, I'm leaving an initial set of comments about the provisioning path. If you want we can have a chat to go through the high level design aspects.

thomas-fossati · 2022-11-16T15:53:27Z

provisioning/plugins/corim-nitro-decoder/README.md

+## Trust Anchor
+
+```json
+{
+  "scheme": "AWS_NITRO",
+  "type": "VERIFICATION_KEY",
+  "attributes": {
+    "psa.hw-model": "RoadRunner",
+    "psa.hw-vendor": "ACME",
+    "psa.iak-pub": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE6Vwqe7hy3O8Ypa+BUETLUjBNU3rEXVUyt9XHR7HJWLG7XTKQd9i1kVRXeBPDLFnfYru1/euxRnJM7H9UoFDLdA==",
+    "psa.impl-id": "IllXTnRaUzFwYlhCc1pXMWxiblJoZEdsdmJpMXBaQzB3TURBd01EQXdNREU9Ig==",
+    "psa.inst-id": "AUyj5PUL8kjDl4cCDWj/0FyIdndRvyZFypI/V6mL7NKW"
+  }
+}
+```


there is no need to for an explicit trust-anchor endorsement, I think. The CN=aws.nitro-enclaves, C=US, O=Amazon, OU=AWS cert can be embedded in the package that provides the attestation document verification.

thomas-fossati · 2022-11-16T15:55:26Z

provisioning/plugins/corim-nitro-decoder/README.md

+{
+  "scheme": "AWS_NITRO",
+  "type": "REFERENCE_VALUE",
+  "attributes": {
+    "psa.hw-model": "RoadRunner",
+    "psa.hw-vendor": "ACME",
+    "psa.impl-id": "IllXTnRaUzFwYlhCc1pXMWxiblJoZEdsdmJpMXBaQzB3TURBd01EQXdNREU9Ig==",
+    "psa.measurement-desc": 1,
+    "psa.measurement-type": "BL",
+    "psa.measurement-value": "h0KPxSKAPTEGXnvOPPA/5HUJZjHl4Hu9eg/eYMTPJcc=",
+    "psa.signer-id": "rLsRx+TaIXIFUjzkzhokWuGiOa48a/2eeHH35di66Gs=",
+    "psa.version": "2.1.0"
+  }
+}


this isn't what you want. (looks like a leftover?)
what you really want is to define a way to use CoMID to transport the expected PCR values and some kind of enclave identifier.

Definitely a leftover.
I am currently not transporting any expected PCR values. That's not how Veracruz attestation works (they are embedded in the generated certificate and the client authenticates them). I've thought a bit how it might be done for other systems, but yes, we need to figure out how to do that for more general use-cases.

we need to figure out how to do that for more general use-cases.

👍 definitely an important item for discussion.

To support both use cases we need to create a non-ambiguous signal that the verifier plugin can use to distinguish between "don't bother checking PCRs for this workload" and "no matching PCRs for this workload"

thomas-fossati · 2022-11-16T15:56:54Z

provisioning/plugins/corim-nitro-decoder/classattributes.go

+	Vendor string
+	Model  string


is this information available in the attestation document?
here you need something that can uniquely correlate the incoming evidence (i.e., the attestation document) with the expected reference values. would module_id work for this purpose?

Having thought a bit more about precise enclave identification, it seems like PCR0 would do the job?

if so, we'd need a new instance-id type in comid since we don't currently have a place for a sha-384 string.

PCRs 0-2 are a function of the input image (not the platform). They certainly could be included as part of a COSWID, but I'm not sure of the use-case.
Putting PCR8 in a COSWID would effectively tie the attestation to a specific AWS Customer, which may be valuable.

PCR3 ties you to a specific IAM role. Perhaps interesting.

PCR4 ties you to a specific AWS EC2 instance, which I have no idea why you would want to do this.

PCRs 0-2 are a function of the input image (not the platform).

sure, but what we need is a unique identifier for the workload itself. The assurance that it's run on genuine Nitro is derived from the cryptographic verification, isn't it?

thomas-fossati · 2022-11-16T16:02:29Z

provisioning/plugins/corim-nitro-decoder/extractor.go

+type Extractor struct{}
+
+func (o Extractor) SwCompExtractor(rv comid.ReferenceValue) ([]*proto.Endorsement, error) {
+	return nil, fmt.Errorf("Not implemented, not needed?")


this is where you'd read the expected PCRs from the CoMID, so it looks like it's needed

I think needed for some use-cases, but not all (see my above comments about Veracruz's needs).

👍 this needs designing

thomas-fossati · 2022-11-16T16:03:54Z

provisioning/plugins/corim-nitro-decoder/extractor.go

+	// return structpb.NewStruct(swAttrs)
+}
+
+func (o Extractor) TaExtractor(avk comid.AttestVerifKey) (*proto.Endorsement, error) {


this instead should not be needed: the trust anchor is fixed and can be embedded in the package that provides the verification logics

The trust anchor is a long-lived CA Certificate. The way it currently works is that certificate is passed into the AuthenticateDocument function call in go-nitro-enclave-attestation-document: https://github.com/veracruz-project/go-nitro-enclave-attestation-document/blob/0893a6c144113b9a29f3111101f64b4d6cf1c999/lib.go#L37

I did not want to embed a certificate, no matter how long-lived, in the logic of the module. Thus, it is being provided as part of the provisioning process.

thomas-fossati · 2022-11-16T16:04:45Z

provisioning/plugins/corim-nitro-decoder/swcompattributes.go

+}
+
+func (o *NitroSwCompAttributes) FromMeasurement(m comid.Measurement) error {
+	return fmt.Errorf("Not implemented, not needed?")


ditto, this is needed, but first you need to define your CoMID profile to carry the PCRs

thomas-fossati · 2022-11-16T16:07:55Z

provisioning/plugins/corim-nitro-decoder/test-harness/AWSNitroComid.json

+                "type": "uuid",
+                "value": "51d1a77a-9908-49e0-92df-e9c872a9b0bd"


what does this uuid represent?

Just randomly generated when I was getting started. I do not use it for anything.

ok. the environment identifier is used (indirectly) as a lookup key for the endorsements. so it should be somewhat bound to the incoming evidence.

thomas-fossati · 2022-11-16T16:11:23Z

provisioning/plugins/corim-nitro-decoder/test-harness/AWSNitroComid.json

+          },
+          "verification-keys": [
+            {
+              "key": "MIICETCCAZagAwIBAgIRAPkxdWgbkK/hHUbMtOTn+FYwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCVVMxDzANBgNVBAoMBkFtYXpvbjEMMAoGA1UECwwDQVdTMRswGQYDVQQDDBJhd3Mubml0cm8tZW5jbGF2ZXMwHhcNMTkxMDI4MTMyODA1WhcNNDkxMDI4MTQyODA1WjBJMQswCQYDVQQGEwJVUzEPMA0GA1UECgwGQW1hem9uMQwwCgYDVQQLDANBV1MxGzAZBgNVBAMMEmF3cy5uaXRyby1lbmNsYXZlczB2MBAGByqGSM49AgEGBSuBBAAiA2IABPwCVOumCMHzaHDimtqQvkY4MpJzbolL//Zy2YlES1BR5TSksfbb48C8WBoyt7F2Bw7eEtaaP+ohG2bnUs990d0JX28TcPQXCEPZ3BABIeTPYwEoCWZEh8l5YoQwTcU/9KNCMEAwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUkCW1DdkFR+eWw5b6cp3PmanfS5YwDgYDVR0PAQH/BAQDAgGGMAoGCCqGSM49BAMDA2kAMGYCMQCjfy+Rocm9Xue4YnwWmNJVA44fA0P5W2OpYow9OYCVRaEevL8uO1XYru5xtMPWrfMCMQCi85sWBbJwKKXdS6BptQFuZbT73o/gBh1qUxl/nNr12UO8Yfwr6wPLb+6NIwLz3/Y="


as I said above, this does not need to be an external endorsement: it's effectively an immutable root that is common to any possible nitro-based enclave, as such it belongs to the verification package

I disagree. While it's long-lived, it's possible that AWS may update it or even revoke it. That sort of thing does not belong in the package.

Additionally, considering your suggestion that we use an alternate CA cert for testing (and generate "fake" AWS Nitro attestation documents for this testing), embedding it in the package seems like it would complicate things.

I disagree. While it's long-lived, it's possible that AWS may update it or even revoke it. That sort of thing does not belong in the package.

there is no way to revoke a root CA cert (it's the classic chicken & egg). You can only remove it from the "root store". and one can quite naturally consider the Go package the "root store" for the nitro attestation PKI.

Additionally, considering your suggestion that we use an alternate CA cert for testing (and generate "fake" AWS Nitro attestation documents for this testing), embedding it in the package seems like it would complicate things.

that sounds like an implementation detail, e.g. one could do:

// verify.go func verifyWithRoot(evidence []bytes, rootCA x509.Certificate) error { // ... } func Verify(evidence []bytes) error { return verifyWithRoot(evidence, AWSRootCA) }

and

// verify_test.go func TestVerify_withAltRoot_fail(t *testing.T) { // ... expectedErr := `cannot anchor path onto a trusted root` assert.EqualError(t, verifyWithRoot(testEvidence, altRootCA), expectedErr) }

Yes, it's not revokable, but that doesn't mean it will not be revoked. It would not be too difficult for them to send a notice to all AWS users that have used NItro enclaves. Certainly messy, but possible.

Additionally, AWS may in the future want to use different root CA certs for different situations (consider their US government cloud, which I imagine already does, if it supports Nitro at all).
I still feel that the CA Cert does not belong in the authentication library, it should be provisioned into the Veraison infra.

If the CA cert was in the authentication library, Veracruz has zero use for Veraison in the nitro use-case, because we are basically only relying on Veraison to handle provisioning and maintenance of that CA Cert.

thomas-fossati · 2022-11-16T16:12:31Z

provisioning/plugins/corim-nitro-decoder/test-harness/AWSNitroComidDualKey.json

+              "key": "MIICETCCAZagAwIBAgIRAPkxdWgbkK/hHUbMtOTn+FYwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCVVMxDzANBgNVBAoMBkFtYXpvbjEMMAoGA1UECwwDQVdTMRswGQYDVQQDDBJhd3Mubml0cm8tZW5jbGF2ZXMwHhcNMTkxMDI4MTMyODA1WhcNNDkxMDI4MTQyODA1WjBJMQswCQYDVQQGEwJVUzEPMA0GA1UECgwGQW1hem9uMQwwCgYDVQQLDANBV1MxGzAZBgNVBAMMEmF3cy5uaXRyby1lbmNsYXZlczB2MBAGByqGSM49AgEGBSuBBAAiA2IABPwCVOumCMHzaHDimtqQvkY4MpJzbolL//Zy2YlES1BR5TSksfbb48C8WBoyt7F2Bw7eEtaaP+ohG2bnUs990d0JX28TcPQXCEPZ3BABIeTPYwEoCWZEh8l5YoQwTcU/9KNCMEAwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUkCW1DdkFR+eWw5b6cp3PmanfS5YwDgYDVR0PAQH/BAQDAgGGMAoGCCqGSM49BAMDA2kAMGYCMQCjfy+Rocm9Xue4YnwWmNJVA44fA0P5W2OpYow9OYCVRaEevL8uO1XYru5xtMPWrfMCMQCi85sWBbJwKKXdS6BptQFuZbT73o/gBh1qUxl/nNr12UO8Yfwr6wPLb+6NIwLz3/Y="
+            },
+            {
+              "key": "BIICETCCAZagAwIBAgIRAPkxdWgbkK/hHUbMtOTn+FYwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCVVMxDzANBgNVBAoMBkFtYXpvbjEMMAoGA1UECwwDQVdTMRswGQYDVQQDDBJhd3Mubml0cm8tZW5jbGF2ZXMwHhcNMTkxMDI4MTMyODA1WhcNNDkxMDI4MTQyODA1WjBJMQswCQYDVQQGEwJVUzEPMA0GA1UECgwGQW1hem9uMQwwCgYDVQQLDANBV1MxGzAZBgNVBAMMEmF3cy5uaXRyby1lbmNsYXZlczB2MBAGByqGSM49AgEGBSuBBAAiA2IABPwCVOumCMHzaHDimtqQvkY4MpJzbolL//Zy2YlES1BR5TSksfbb48C8WBoyt7F2Bw7eEtaaP+ohG2bnUs990d0JX28TcPQXCEPZ3BABIeTPYwEoCWZEh8l5YoQwTcU/9KNCMEAwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUkCW1DdkFR+eWw5b6cp3PmanfS5YwDgYDVR0PAQH/BAQDAgGGMAoGCCqGSM49BAMDA2kAMGYCMQCjfy+Rocm9Xue4YnwWmNJVA44fA0P5W2OpYow9OYCVRaEevL8uO1XYru5xtMPWrfMCMQCi85sWBbJwKKXdS6BptQFuZbT73o/gBh1qUxl/nNr12UO8Yfwr6wPLb+6NIwLz3/Y="


curiosity: what is this?

The first one is the CA key. The 2nd one is garbage to enable a "Dual Key" test. I don't think there's an actual use-case here.

Signed-off-by: Derek D. Miller <[email protected]>

thomas-fossati · 2024-08-27T08:20:44Z

Can we declare this OBE and close it?

dreemkiller added 2 commits November 12, 2022 18:55

Added vts and provisioning plugins for AWS Nitro enclave attestation …

206f974

…documents Signed-off-by: Derek D. Miller <[email protected]>

Code cleanup and improved test coverage for the AWS Nitro plugins

e61cd60

Signed-off-by: Derek D. Miller <[email protected]>

dreemkiller force-pushed the add_aws_nitro branch from 51eba16 to e61cd60 Compare November 13, 2022 00:55

dreemkiller added 2 commits November 12, 2022 19:22

Added more test cases for the VTS Nitro plugin

e42b319

Signed-off-by: Derek D. Miller <[email protected]>

Removed some possibly unneeded code from the AWS Nitro plugins

08e530e

Signed-off-by: Derek D. Miller <[email protected]>

thomas-fossati reviewed Nov 16, 2022

View reviewed changes

dreemkiller added 3 commits November 28, 2022 16:42

Added document generation to the AWS Nitro tests

f1b8839

Signed-off-by: Derek D. Miller <[email protected]>

Removed some no-longer-needed test files

d3ddbf1

Signed-off-by: Derek D. Miller <[email protected]>

Renamed some fields. Added check for null SoftwareId

aab1f15

Signed-off-by: Derek D. Miller <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Added vts and provisioning plugins for AWS Nitro enclave attestation … #58

WIP: Added vts and provisioning plugins for AWS Nitro enclave attestation … #58

dreemkiller commented Nov 12, 2022

thomas-fossati left a comment

thomas-fossati Nov 16, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022 •

edited

Loading

thomas-fossati Nov 17, 2022

thomas-fossati Nov 16, 2022

thomas-fossati Nov 16, 2022 •

edited

Loading

thomas-fossati Nov 16, 2022 •

edited

Loading

dreemkiller Nov 16, 2022

thomas-fossati Nov 17, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022

thomas-fossati Nov 17, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022

thomas-fossati Nov 16, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022

thomas-fossati Nov 17, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022

thomas-fossati Nov 17, 2022

thomas-fossati Nov 17, 2022 •

edited

Loading

dreemkiller Nov 17, 2022

thomas-fossati Nov 16, 2022

dreemkiller Nov 16, 2022

thomas-fossati commented Aug 27, 2024

		"type": "uuid",
		"value": "51d1a77a-9908-49e0-92df-e9c872a9b0bd"

WIP: Added vts and provisioning plugins for AWS Nitro enclave attestation … #58

Are you sure you want to change the base?

WIP: Added vts and provisioning plugins for AWS Nitro enclave attestation … #58

Conversation

dreemkiller commented Nov 12, 2022

thomas-fossati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreemkiller Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

thomas-fossati Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati commented Aug 27, 2024

dreemkiller Nov 16, 2022 •

edited

Loading

thomas-fossati Nov 16, 2022 •

edited

Loading

thomas-fossati Nov 16, 2022 •

edited

Loading

thomas-fossati Nov 17, 2022 •

edited

Loading