Skip to content
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.

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

Add the cert.create event #9822

Merged
merged 5 commits into from Feb 8, 2022
Merged

Conversation

espadolini
Copy link
Contributor

@espadolini espadolini commented Jan 18, 2022

This adds an event that's emitted whenever an auth server issues a certificate (or a set of certificates, like the SSH+TLS pair that is issued to users). The event is only emitted on user cert generation for now.

Sample events:

Local login
{
  "ei": 0,
  "event": "cert.create",
  "uid": "f41a1615-5147-4a1f-8a29-4f62ab0285d3",
  "code": "TC000I",
  "time": "2022-01-21T15:41:59.299Z",
  "cluster_name": "localtele.example.com",
  "cert_type": "user",
  "identity": {
    "user": "espadolini",
    "roles": [
      "access",
      "auditor",
      "editor"
    ],
    "logins": [
      "espadolini"
    ],
    "expires": "2022-01-22T03:41:59.298282Z",
    "route_to_cluster": "localtele.example.com",
    "traits": {
      "logins": [
        "espadolini"
      ]
    },
    "teleport_cluster": "localtele.example.com"
  }
}
OIDC login
{
  "ei": 0,
  "event": "cert.create",
  "uid": "8aa10300-4130-41ba-823f-508af9098688",
  "code": "TC000I",
  "time": "2022-01-21T15:32:43.448Z",
  "cluster_name": "localtele.example.com",
  "cert_type": "user",
  "identity": {
    "user": "edoardo.spadolini.2@teleportinfra.dev",
    "roles": [
      "access"
    ],
    "logins": [
      "-teleport-nologin-c5f6c25c-cb65-456c-bcca-d38c3f6b2cfa"
    ],
    "expires": "2022-01-22T21:32:43.439044Z",
    "route_to_cluster": "localtele.example.com",
    "traits": {
      "at_hash": [
        "uO_uGk7zRUoNWDZUnWi3gg"
      ],
      "aud": [
        "289270662827-193i30qgssofegigee0bad0gv6mr26pl.apps.googleusercontent.com"
      ],
      "azp": [
        "289270662827-193i30qgssofegigee0bad0gv6mr26pl.apps.googleusercontent.com"
      ],
      "email": [
        "edoardo.spadolini.2@teleportinfra.dev"
      ],
      "groups": [
        "this-gets-access-role@teleportinfra.dev",
        "this-gets-no-roles@teleportinfra.dev"
      ],
      "hd": [
        "teleportinfra.dev"
      ],
      "iss": [
        "https://accounts.google.com"
      ],
      "picture": [
        "https://lh3.googleusercontent.com/a/default-user=s96-c"
      ],
      "sub": [
        "101835025516069956467"
      ]
    },
    "teleport_cluster": "localtele.example.com"
  }
}
OIDC login after assuming a role through an access request
{
  "ei": 0,
  "event": "cert.create",
  "uid": "5867c39c-cbd9-4216-9c74-1e66f2800cf7",
  "code": "TC000I",
  "time": "2022-01-21T15:38:06.838Z",
  "cluster_name": "localtele.example.com",
  "cert_type": "user",
  "identity": {
    "user": "edoardo.spadolini.2@teleportinfra.dev",
    "roles": [
      "access",
      "auditor"
    ],
    "logins": [
      "-teleport-nologin-ecdf1efb-59c4-4abf-934f-3e1e085a53b9"
    ],
    "expires": "2022-01-22T21:37:32.018833Z",
    "route_to_cluster": "localtele.example.com",
    "traits": {
      "at_hash": [
        "JuNmTsSRPXl14Jj5Qt7M2A"
      ],
      "aud": [
        "289270662827-193i30qgssofegigee0bad0gv6mr26pl.apps.googleusercontent.com"
      ],
      "azp": [
        "289270662827-193i30qgssofegigee0bad0gv6mr26pl.apps.googleusercontent.com"
      ],
      "email": [
        "edoardo.spadolini.2@teleportinfra.dev"
      ],
      "groups": [
        "this-gets-access-role@teleportinfra.dev",
        "this-gets-no-roles@teleportinfra.dev"
      ],
      "hd": [
        "teleportinfra.dev"
      ],
      "iss": [
        "https://accounts.google.com"
      ],
      "picture": [
        "https://lh3.googleusercontent.com/a/default-user=s96-c"
      ],
      "sub": [
        "101835025516069956467"
      ]
    },
    "teleport_cluster": "localtele.example.com",
    "access_requests": [
      "44aa82d6-d8d3-4b11-85e8-6f548058b86d"
    ]
  }
}

Fixes half of TEL-Q321-7.

Closes #9591.

@espadolini espadolini added audit-log Issues related to Teleports Audit Log needs-product-decision labels Jan 18, 2022
api/types/events/events.proto Outdated Show resolved Hide resolved
api/types/events/events.proto Outdated Show resolved Hide resolved
lib/events/api.go Outdated Show resolved Hide resolved
@russjones
Copy link
Contributor

russjones commented Jan 19, 2022

@espadolini Can you provide an example of what the event looks like, in particular the subject field?

@espadolini
Copy link
Contributor Author

@espadolini Can you provide an example of what the event looks like, in particular the subject field?

Updated the PR description.

@espadolini espadolini force-pushed the espadolini/event-certsuseremit branch from 4db171d to ac80782 Compare January 19, 2022 17:05
@codingllama codingllama changed the title Add the certs.user.emit event Add the user.cert.create event Jan 19, 2022
@russjones
Copy link
Contributor

@espadolini I think we should break up the Subject into fields on the event. Users have to parse it right now and it's going to just get harder as we add support for Windows. Plus, even if we say something like it's comma separated, that will be what users expect forever and we'll be stuck with it even if it doesn't work for us in the future.

Also, what do you think about making it cert.create and having a field with the type of certificate. That way it's easier to extend this in the future to support host certs.

@espadolini
Copy link
Contributor Author

@espadolini I think we should break up the Subject into fields on the event. Users have to parse it right now and it's going to just get harder as we add support for Windows. Plus, even if we say something like it's comma separated, that will be what users expect forever and we'll be stuck with it even if it doesn't work for us in the future.

I did it this way because I intended it to be more for security audit/postmortem situations rather than general event consumption. Should we replicate every field that goes into a tlsca.Identity instead? We'd have to keep the event in sync with every future addition to the Identity type.

Also, what do you think about making it cert.create and having a field with the type of certificate. That way it's easier to extend this in the future to support host certs.

Yeah, that's probably better.

@espadolini espadolini force-pushed the espadolini/event-certsuseremit branch from ac80782 to ff1c1e1 Compare January 21, 2022 15:28
@espadolini espadolini changed the title Add the user.cert.create event Add the cert.create event Jan 21, 2022
@russjones
Copy link
Contributor

@espadolini Let's change cert_type to be user or host, other than that looks good to me.

Copy link
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

@espadolini, please push changes after reviews to new commits, so it's easier to what changed in relation to the last round.

api/types/events/events.proto Outdated Show resolved Hide resolved
api/types/events/events.proto Outdated Show resolved Hide resolved
api/types/events/events.proto Outdated Show resolved Hide resolved
api/types/events/events.proto Outdated Show resolved Hide resolved
@espadolini
Copy link
Contributor Author

@espadolini, please push changes after reviews to new commits, so it's easier to what changed in relation to the last round.

Sorry about that, I tend to just amend commits if they're almost completely rewritten - I'll be more mindful about it in the future.

@codingllama
Copy link
Contributor

Sorry about that, I tend to just amend commits if they're almost completely rewritten - I'll be more mindful about it in the future.

No worries. In an ideal world GitHub would be more helpful in showing diffs. Or maybe I'm just bad at GitHub? 🤷

@russjones
Copy link
Contributor

russjones commented Feb 4, 2022

@espadolini Looks good to me, but make sure you make the webapps changes so this event shows up in the UI.

Feel free to merge this into master, but don't backport until you have the webapps changes for release branches.

@espadolini espadolini force-pushed the espadolini/event-certsuseremit branch from c669167 to bd7f327 Compare February 8, 2022 10:45
@espadolini espadolini enabled auto-merge (squash) February 8, 2022 10:45
@espadolini espadolini merged commit 154d6fb into master Feb 8, 2022
@espadolini espadolini deleted the espadolini/event-certsuseremit branch February 8, 2022 11:13
@espadolini espadolini restored the espadolini/event-certsuseremit branch February 8, 2022 17:58
@espadolini espadolini deleted the espadolini/event-certsuseremit branch February 8, 2022 18:18
espadolini added a commit that referenced this pull request Feb 8, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 8, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 8, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 8, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 14, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 14, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
espadolini added a commit that referenced this pull request Feb 14, 2022
* Add the `cert.create` event

For now, this is only emitted for user certificate issuance.

* Make `cert_type` a string rather than an enum

* Match field names and json tags in events.Identity

* Change events.Identity.Traits to be a wrappers.LabelValues/wrappers.Traits

* Event code shouldn't be under T10xx anymore
@webvictim webvictim mentioned this pull request Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-log Issues related to Teleports Audit Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certificate issuance should emit an audit event
3 participants