Add vTPM specification by stefanberger · Pull Request #920 · opencontainers/runtime-spec

stefanberger · 2017-09-08T19:00:41Z

Add the vTPM specification to the documentation, config.go, and
schema description. The following is an example of a vTPM description
that is found under the path /linux/resources/vtpms:

"vtpms": [
{
"VTPMVersion": "1.2",
"CreateCertificates" : false
}
]

Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com

crosbymichael · 2017-09-08T19:08:08Z

@stefanberger is this interface stable from the kernel POV?

crosbymichael · 2017-09-08T19:13:32Z

+// VTPM is used to hold the configuration state of a VTPM
+type VTPM struct {
+	// The directory where the TPM emulator writes the TPM state to
+	Statepath string `json:"statepath"`


StatePath string json:"statePath"

crosbymichael · 2017-09-08T19:13:46Z

+	// The directory where the TPM emulator writes the TPM state to
+	Statepath string `json:"statepath"`
+	// Whether to create a certificate for the VTPM
+	Createcerts bool `json:"createcerts"`


CreateCerts bool json:"createCerts"

crosbymichael · 2017-09-08T19:14:04Z

+	// Whether to create a certificate for the VTPM
+	Createcerts bool `json:"createcerts"`
+	// Version of the TPM
+	Vtpmversion string `json:"vtpmversion"`


VTPMVersion string json:"vtpmVersion"

crosbymichael · 2017-09-08T19:14:36Z

+ },
+ "vtpms": [
+ {
+ "Statepath": "/var/run/runc/ubuntu/tpm12_1",


these should be the same as the json tags on the go struct

crosbymichael · 2017-09-08T19:14:52Z

+```json
+ "vtpms": [
+ {
+ "Statepath": "/var/run/runc/ubuntu/tpm12_1",


lowercase like the json tags

stefanberger · 2017-09-08T19:33:37Z

@crosbymichael I updated the patches to reflect the requested changes.

I added 3 fields to the VTPM struct in config.go. In runc I have a lot more fields. I suppose this is ok.

stefanberger · 2017-09-08T19:35:54Z

@crosbymichael The kernel has a vTPM proxy driver that we would be using in runc. Its interface is stable. New ioctl's may be added, but that shouldn't be a problem.

The interface of the TPM emulator implementation runc is using, swtpm, is also stable.

crosbymichael · 2017-09-11T18:02:50Z

@stefanberger why does runc have more fields than the spec?

stefanberger · 2017-09-11T18:33:11Z

@crosbymichael It's keeping data such as the created major/minor numbers of the character device, file descriptor, etc. Here's where it resides then. https://github.com/stefanberger/runc/blob/vtpm/libcontainer/vtpm/vtpm.go#L21

crosbymichael · 2017-09-11T19:22:49Z

@stefanberger ok, so those are implementation details, not input from the user

wking

Assorted minor copy-edits.

wking · 2017-09-11T19:22:59Z

+## <a name="configLinuxVTPMs" />vTPMs
+
+**`vtpms`** (array of objects, OPTIONAL) lists a number of emulated TPMs that
+will be made available to the container.


nit: this sentence is split over two lines, but it should be a single line.

wking · 2017-09-11T19:26:36Z

 }
 ```

+## <a name="configLinuxVTPMs" />vTPMs


nit: your header depth is off by one. The preceding and following headers are both ###. And once you bump this up by one, you'll want to bump up your example header too.

wking · 2017-09-11T19:30:22Z

+
+Each entry has the following structure:
+
+* **`statePath`** *(string, REQUIRED)* - full path to a directory where the vTPM is to write its persistent state into


nit: suggested copy edit:
statePath(string, REQUIRED) - a directory for persisting vTPM state. This value MUST be an absolute path.
to echo our existing language here and here.

wking · 2017-09-11T20:06:09Z

+* **`vtpmVersion`** *(string, OPTIONAL)* - The version of TPM to emulate; either 1.2 or 2; default is 1.2
+* **`createCerts`** *(boolean, OPTIONAL)* - Whether to create certificates for the vTPM
+
+The `statePath` MUST be unique per container.


nit: suggested copy edit:
If the vtpms array contains duplicated entries with the same statePath, the runtime MUST generate an error.
to echo (ish) our existing language here.

wking · 2017-09-11T20:07:15Z

+	// Whether to create a certificate for the VTPM
+	CreateCerts bool `json:"createCerts"`
+	// Version of the TPM
+	VTPMversion string `json:"vtpmVersion"`


This (and CreatCerts) should be omitempty because they are optional.

wking · 2017-09-11T20:09:14Z

 	// for handling resource constraints (e.g., L3 cache) for the container
 	IntelRdt *LinuxIntelRdt `json:"intelRdt,omitempty"`
+	// VTPM configuration
+	VTPMS []*VTPM `json:"vtpms"`


None of our other arrays are arrays of pointers. Can we use []VTPM here?
And with aggressive namespacing (#567) we probably want []LinuxVTPM or some such.

Also added 'omitempty' now.

The runtime-spec and runc differ significantly in this regard. In runc there are arrays with pointers, such as Mounts []*Mount and Devices []*Device. How do the runtime-spec and runc relate to each other in regards to these files ? If I don't make these changes in runc but leave it at *VTPM, I suppose this will be alright (not to create more churn...)?

wking · 2017-09-11T20:12:13Z

+
+* **`statePath`** *(string, REQUIRED)* - full path to a directory where the vTPM is to write its persistent state into
+* **`vtpmVersion`** *(string, OPTIONAL)* - The version of TPM to emulate; either 1.2 or 2; default is 1.2
+* **`createCerts`** *(boolean, OPTIONAL)* - Whether to create certificates for the vTPM


I think we also want to say “Defaults to false.”, although existing uses (here and here) have tacked that onto the first sentence-ish and not backticked the JSON literal.

I reformulated that a bit to "If true then create certificates for the vTPM, default to false."

wking · 2017-09-11T20:24:02Z

+
+Each entry has the following structure:
+
+* **`statePath`** *(string, REQUIRED)* - full path to a directory where the vTPM is to write its persistent state into


From opencontainers/runc#1591, it looks like you're writing a swtpmPID and log files to this directory. But this spec PR doesn't explain that, so folks consuming a generic runtime (which may or may not be runc) won't be able to rely on that. Do you have a use-case for the runtime-caller reaching into this directory to check those files? If so, I think you should specify the files here. If not, I think we can drop this property from runtime-spec, and generate it on the fly (or based on runtime-specific options, like runc's existing --root option) instead of having it be configured here.

crosbymichael · 2017-09-11T20:47:17Z

+// VTPM is used to hold the configuration state of a VTPM
+type LinuxVTPM struct {
+	// The directory where the TPM emulator writes the TPM state to
+	StatePath string `json:"statePath"`


Do you really need a state path at all? Can this just be an implementation detail of the runtime and have it handle it internally?

Yes, it should be possible to remove it.

wking · 2017-09-11T20:48:10Z

+* **`vtpmVersion`** *(string, OPTIONAL)* - The version of TPM to emulate, either 1.2 or 2; default is 1.2.
+* **`createCerts`** *(boolean, OPTIONAL)* - If true then create certificates for the vTPM, defaults to false.
+
+The `statePath` MUST be unique per container. If the `vtpms` array contains duplicate entries with the same `statePath`, the runtime MUST generate an error.


These two sentences should be on two lines.
And I think the two sentences you have for the statePath list entry above should be as well:
* **`statePath`** *(string, REQUIRED)* - a directory for persisting vTPM state. This value MUST be an absolute path.
(with a four-space indent, #495, #846), although the current spec is not always consistent about list entries (e.g. here we have two sentences in one list-entry line).
Also, existing examples use a link for “generate an error” (e.g. see here).

Thanks for your patience. Fixed.

crosbymichael · 2017-09-11T21:10:42Z

@stefanberger

Is this needed at the runtime level because of interactions with the tpm and user namespaces? Sorry, I'm just trying to understand this more and hope I'm not asking stupid questions.

stefanberger · 2017-09-11T21:12:35Z

@crosbymichael I am not sure what your latest comment ('Is this needed at the runtime level...')is referring to.

crosbymichael · 2017-09-11T21:13:27Z

@stefanberger why couldn't this be done outside of the runtime(runc) and just configured the spec to add the new device to the container?

wking · 2017-09-11T21:13:41Z

Looking over this PR and opencontainers/runc#1591, I have some slight concerns about making the vtmps object so opaque. As it stands in the runc PR, these parameters will be backed by swtpm, and the API we're exposing is basically “things swtpm needs the runtime-caller to decide”. But the swtpm config is different from the kernel API. We could have a thinner runtime-spec interface if we stayed closer to the kernel API. You could just configure the number of vtpms you want ("vtpms": 2) and have the runtime create the appropriate character devices for the container and pipe out the file descriptors to the caller (like we already do for pseudoterminal masters with --console-socket, although we have yet to land a spec for that). We could re-use the existing console socket (with a more generic name?) and just define a new request-message type for the vTPM descriptors. Then runtime callers could pass that file descriptor to their emulator of choice and manage the emulator's lifecycle independently.

On the other hand, with that increased flexibility, you'd have more moving parts. If we're ok claiming that the swtpm or other runtime-chosen emulator is a good enough for anyone using vtpms, then that's fine too. I guess folks who were not satisfied with their runtime's choice could always create the vTPM and push the appropriate device into their rootfs before create-ing the container if they needed more flexibility.

wking · 2017-09-11T21:16:36Z

Is this needed at the runtime level because of interactions with the tmp and user namespaces?

Is there a tpm namespace? I agree that handing this outside of the runtime seems cleanest.

stefanberger · 2017-09-11T21:20:54Z

@wking So the kernel API does take a VTPMVersion as a parameter of an ioctl() but doesn't care about creation of certificates, which is a configuration parameter to the emulator. So in the case of vTPM we may not just have kernel parameters but also emulator parameters that need to be passed. Another possible parameter to the emulator would be whether the emulator is supposed to encrypt the files the vTPM writes out. On input this could be a boolean and runc creates a random key or one could pass an (AES) key directly.
I suppose passing emulator parameters via the JSON is needed and allowed in this case, even if these parameters are not directly kernel parameters.

stefanberger · 2017-09-11T21:21:15Z

@wking There's no tpm namespace.

wking · 2017-09-11T21:30:55Z

On Mon, Sep 11, 2017 at 09:20:57PM +0000, Stefan Berger wrote: So in the case of vTPM we may not just have kernel parameters but also emulator parameters that need to be passed.

What do we gain by making the runtime a middleman between the emulator and the runtime-caller? It seems simpler to have the caller: 1. Setup whichever emulator they want however they want. 2. Put the resulting character device under the container's root.path (or add it to linux.devices). 3. Call ‘create’ to launch the container. Then there's no need to tell the runtime about vTPM at all, it's just passing a device through to the container like all the other devices it passes through.

wking · 2017-09-11T21:48:09Z

What do we gain by making the runtime a middleman between the emulator and the runtime-caller?

As an example of this in another context, making the runtime a middleman for pseudoterminal creation allows you to create the pseudoterminal pair from a newinstancedevpts mount. If you're an unprivileged user, you need the container mount namespace to be created before you have permission for the devpts mount, and while you could create those namespaces outside of the runtime and pass them in via file descriptor paths, that duplicates a lot of the work that the runtime is supposed to be helping you do. So having the runtime handle the newinstancedevpts mount and then create a new pseudoterminal pair from that devpts makes sense. It's not clear to me (yet?) what efficiencies we get by similarly pushing vTPM into the runtime.

stefanberger · 2017-09-11T22:37:08Z

@wking @crosbymichael dumb question: if we take the runtime, which I suppose you are referring to is represented by this code base here, out, does that mean we wouldn't have anything vTPM related in this repository? Maybe I am not following the discussion correctly, but I suppose that at least the config.json would have to contain the vtpms array and at least the version of TPM to emulate (and possibly the other parameters necessary for the emulator)?

stefanberger · 2017-09-12T12:32:28Z

@wking If we express a vTPM instance as shown above with a device on the level of the runtime-spec, then how do we represent it at the level of runc ? Can runc extend the JSON?

wking · 2017-09-12T14:05:05Z

If we express a vTPM instance as shown above with a device on the level of the runtime-spec, then how do we represent it at the level of runc ? Can runc extend the JSON?

Runc can store additional information in its libcontainer config. But why would it need to? If the caller is managing swtpm and the devices outside the runtime, there's nothing for the runtime to do.

If you want a tighter binding, you could reroll opencontainers/runc#1591 into a third-party hook tool, with a setup command for a pre-start hook and a teardown command for a post-stop hook.

But I don't see anything in a quick skim of the runc PR, except maybe checkpoint blocking, that cannot be handled by hooks or other external-to-the-runtime code. Am I missing something? Can you provide more details on the checkpoint breakage?

stefanberger · 2017-09-13T06:55:49Z

@wking Who is the 'caller' in this case? ('If the caller is managing swtpm and the device...'). Do you want to support vTPM on the runc level or push to even higher levels?

I assumed that checkpointing includes migration as well. With an attached vTPM its state would have to be migrated along with the state of the container, which isn't implemented.

stefanberger · 2017-09-13T07:52:41Z

@wking FYI: I am also working on namespacing of IMA. There I am hooking up an IMA namespace with a virtual TPM instance and the vTPM receives the TPM commands (PCR Extends) from IMA that would normally go to the hardware TPM. If we wanted to support that on the runc level some day, then there should not only be support for an IMA namespace on that level but also to hook up a vTPM to it.

wking · 2017-09-13T14:12:12Z

Who is the 'caller' in this case?

The runtime-caller (e.g., see the steps I floated here).

Do you want to support vTPM on the runc level or push to even higher levels?

I want to push it up to higher levels, unless we have a reason for embedding it in the runtime (like we already do for network setup).

With an attached vTPM its state would have to be migrated along with the state of the container, which isn't implemented.

That makes sense. But I don't see a reason why emulator checkpointing couldn't happen at higher levels too, so it's not a reason for putting this in the runtime vs. higher levels.

I am also working on namespacing of IMA.

Depending on the details, this could end up being like network-namespace setup (which we punt to higher levels) or mount-namespace setup (which we handle in the runtime via mount). Which do you think it seems closer to?

stefanberger · 2018-02-15T19:36:00Z

@wking To resume this discussion. I integrated vTPM (with IMA namespacing) into Docker-CE 17.12. As part of that I found it to be necessary to support the following runtime spec for supporting a vTPM:

+// VTPM definition
+type VTPM struct {
+	Statepath string `json:"statepath,omitempty"`
+	StatepathIsManaged bool `json:"statepathismanaged,omitempty"`
+	TPMVersion string `json:"vtpmversion,omitempty"`
+	CreateCertificates bool `json:"createcerts,omitempty"`
+}

I extended RunC with vTPM. When running RunC with vTPM support in Docker-CE one surprise was that when running 'docker restart' the vTPM state path was deleted and thus all the persistent state of the vTPM got lost. This for example required the addition of the 'StatePathIsManaged' field in this patch here, which basically helps us restart a container on the docker level without erasing the state like we do when we delete the container on the Docker level or using runc delete xyz on the RunC level.

Further, I find it necessary to integrate the vTPM into RunC also because of RunC spawning an IMA namespace (namespacing of IMA is not upstreamed) and to hook a vTPM to the IMA namespace I need to be able to transfer the vTPM device to the IMA namespace by calling an ioctl on the vTPM device's file descriptor. I doubt that this would be possible if vTPM was created in a hook like where we can setup networking using netns.

wking · 2018-02-16T19:55:19Z

I extended RunC with vTPM. When running RunC with vTPM support in Docker-CE one surprise was that when running 'docker restart' the vTPM state path was deleted and thus all the persistent state of the vTPM got lost. This for example required the addition of the 'StatePathIsManaged' field in this patch here, which basically helps us restart a container on the docker level without erasing the state like we do when we delete the container on the Docker level or using runc delete xyz on the RunC level.

That patch is making runc's removal of the vTPM state path conditional. But if all vTPM handling happens in higher layers, runc would never be deleting the vTPM state path, so I don't see an issue there.

Further, I find it necessary to integrate the vTPM into RunC also because of RunC spawning an IMA namespace (namespacing of IMA is not upstreamed)…

I can't find code for this. Can you link to it?

… and to hook a vTPM to the IMA namespace I need to be able to transfer the vTPM device to the IMA namespace by calling an ioctl on the vTPM device's file descriptor.

I see no reason why this couldn't happen between create and start (or in a prestart hook). You have the container PID from the state, so you can find the IMA namespace under /proc/{pid}/ns/. And you can configure your hook with the path to the vTPM device, so the hook can open that when it needs the file descriptor.

vbatts · 2019-12-17T18:35:48Z

house keeping:
TPMs seem like a fine thing to support, but this conversation died off. @stefanberger perhaps a fresh PR would be useful, and #920 (comment) asked if there had been any methods used to accomplish this functionality outside of the runtime-spec.
(i'm inclined to close this for now)

stefanberger · 2019-12-30T15:54:44Z

house keeping:
TPMs seem like a fine thing to support, but this conversation died off. @stefanberger perhaps a fresh PR would be useful, and #920 (comment) asked if there had been any methods used to accomplish this functionality outside of the runtime-spec.
(i'm inclined to close this for now)

@crosbymichael @vbatts

I think I had tried to answer some of the issues here..

I haven't looked at this in a while but my arguments for supporting vTPM on the runc level rather than higher layers doing it would be:

vTPM is abstracted to a JSON object which makes it easier for any higher layer wanting to use vTPM; one can start vTPM on the runc level as just another device
vTPM is stateful and for CRIU would either have to block CRIU or have its state blobs written
back then I was experimenting with IMA namespaces and attaching a vTPM to an IMA namespace required the file descriptor of the vTPM to be passed to the IMA namespace using an ioctl

AkihiroSuda · 2020-03-27T21:05:47Z

needs rebase

stefanberger · 2020-07-06T19:35:46Z

The update I pushed today adds all those fields that are needed to support the vTPM features implemented in runc PR opencontainers/runc#1591

lukehinds · 2020-07-06T19:56:55Z

@crosbymichael / @wking are you able to recommence / continue your reviews please. As @vbatts points out, vTPM support would be immensely useful for measuring trust of containers.

Add the vTPM specification to the documentation, config.go, and schema description. The following is an example of a vTPM description that is found under the path /linux/resources/vtpms: "vtpms": [ { "statePath": "/var/lib/runc/myvtpm1", "vtpmVersion": "2", "createCerts": false, "runAs": "tss", "pcrBanks": "sha1,sha512" } ] Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

This extension of LinuxDevice adds support for making a device available under a different path than it is seen on the host. The device path on the host may for example be /dev/foo and it may be made available under /dev/bar inside a container. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

mrunalp · 2020-07-22T18:35:51Z

 	// Path to the device.
 	Path string `json:"path"`
+	// Path of passed-through device on host
+	Devpath string `json:"devpath"`


What is this needed for?

So that /dev/tpm10 on the host can appear as /dev/tpm0 inside the container.

Do we need to know the host path or the entity creating the config.json can get the host device major / minor? For e.g. docker/podman have this feature:
sudo podman run --rm -it --device=/dev/null:/dev/mynull fedora:32 sh sh-5.0# ls -l /dev/ total 0 crw--w----. 1 root tty 136, 0 Aug 6 21:35 console lrwxrwxrwx. 1 root root 11 Aug 6 21:35 core -> /proc/kcore lrwxrwxrwx. 1 root root 13 Aug 6 21:35 fd -> /proc/self/fd crw-rw-rw-. 1 root root 1, 7 Aug 6 21:35 full drwxrwxrwt. 2 root root 40 Aug 6 21:35 mqueue crw-rw-rw-. 1 root root 1, 3 Aug 6 21:35 mynull crw-rw-rw-. 1 root root 1, 3 Aug 6 21:35 null lrwxrwxrwx. 1 root root 8 Aug 6 21:35 ptmx -> pts/ptmx drwxr-xr-x. 2 root root 0 Aug 6 21:35 pts crw-rw-rw-. 1 root root 1, 8 Aug 6 21:35 random drwxrwxrwt. 2 root root 40 Aug 6 21:35 shm lrwxrwxrwx. 1 root root 15 Aug 6 21:35 stderr -> /proc/self/fd/2 lrwxrwxrwx. 1 root root 15 Aug 6 21:35 stdin -> /proc/self/fd/0 lrwxrwxrwx. 1 root root 15 Aug 6 21:35 stdout -> /proc/self/fd/1 crw-rw-rw-. 1 root root 5, 0 Aug 6 21:35 tty crw-rw-rw-. 1 root root 1, 9 Aug 6 21:35 urandom crw-rw-rw-. 1 root root 1, 5 Aug 6 21:35 zero sh-5.0#
The config.json snippet for this:
"devices": [ { "path": "/dev/mynull", "type": "c", "major": 1, "minor": 3, "fileMode": 8630, "uid": 0, "gid": 0 } ],

mrunalp · 2020-08-06T21:23:06Z

+* **`statePathIsManaged`** *(string, OPTIONAL)* - Whether runc is allowed to delete the TPM's state path upon destroying the TPM, defaults to false.
+* **`vtpmVersion`** *(string, OPTIONAL)* - The version of TPM to emulate, either 1.2 or 2, defaults to 1.2.
+* **`createCerts`** *(boolean, OPTIONAL)* - If true then create certificates for the vTPM, defaults to false.
+* **`runAs`** *(string, OPTIONAL)* - Under which user to run the vTPM, e.g. 'tss'.


Does it make sense to run this as the container user or it is typically set to a separate tss user?

mfranczy · 2024-03-25T14:56:48Z

@stefanberger are you still working on this PR?

stefanberger · 2024-03-25T15:09:50Z

No, I am not working on it.

crosbymichael requested changes Sep 8, 2017
View reviewed changes

stefanberger force-pushed the vtpm branch 2 times, most recently from e8213b4 to 3f1ca50Compare September 8, 2017 19:31

stefanberger mentioned this pull request Sep 9, 2017
Add vTPM support for Linux opencontainers/runc#1591
Closed

wking reviewed Sep 11, 2017
View reviewed changes

stefanberger force-pushed the vtpm branch 2 times, most recently from 925a73d to 3bd6071Compare September 11, 2017 20:35

crosbymichael reviewed Sep 11, 2017
View reviewed changes

wking reviewed Sep 11, 2017
View reviewed changes

stefanberger force-pushed the vtpm branch 3 times, most recently from 4550cc0 to 04d0238Compare September 11, 2017 21:07

stefanberger force-pushed the vtpm branch from 04d0238 to b1d2cfaCompare September 11, 2017 21:35

h-vetinari mentioned this pull request Feb 3, 2020
Tag a new release #1010
Closed

stefanberger force-pushed the vtpm branch 6 times, most recently from e0768b8 to 1fe6983Compare July 6, 2020 19:33

stefanberger force-pushed the vtpm branch from 1fe6983 to 615f2f1Compare July 6, 2020 20:10

stefanberger added 2 commits July 6, 2020 16:39

stefanberger force-pushed the vtpm branch from 615f2f1 to 4621d9aCompare July 6, 2020 20:39

mrunalp reviewed Jul 22, 2020
View reviewed changes

mrunalp reviewed Aug 6, 2020
View reviewed changes

utam0k requested a review from a teamMarch 22, 2024 13:46

stefanberger closed this Mar 25, 2024

everzakov mentioned this pull request Aug 16, 2025
Add vTPM specification #1293
Open


		Each entry has the following structure:

		* `statePath` (string, REQUIRED) - full path to a directory where the vTPM is to write its persistent state into

Uh oh!

Conversation

stefanberger commented Sep 8, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crosbymichael commented Sep 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefanberger commented Sep 8, 2017

Uh oh!

stefanberger commented Sep 8, 2017

Uh oh!

crosbymichael commented Sep 11, 2017

Uh oh!

stefanberger commented Sep 11, 2017

Uh oh!

crosbymichael commented Sep 11, 2017

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crosbymichael commented Sep 11, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

stefanberger commented Sep 8, 2017•
edited
Loading

crosbymichael commented Sep 11, 2017•
edited
Loading

wking commented Sep 13, 2017•
edited
Loading