Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[RFC] Session RFC PR to move to Stage 2 (Draft) #935
base: main
Are you sure you want to change the base?
[RFC] Session RFC PR to move to Stage 2 (Draft) #935
Changes from 2 commits
d6714a0
4c2a21d
9d1e57d
57fc786
0721f80
16d1b9d
c8cb020
983668c
71e62aa
c341ea3
848fffd
69c11c7
94aa976
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ECS team will update to the date when PR is merged as stage 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also link the Stage 2 PR at the bottom now that the PR's opened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO
kind
vstype
is really hard to reason about / remember.type
would be more clear if it was namedconnection_type
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea, tho my brain says "local isn't a connection!" but maybe thats ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've updated these a bit, was hoping to get some feedback before submitting updates. I'll try and get the revisions up in the near future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I would say that local is a connection, connecting to local host is a connection, just within a single kernel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This compact table is a great way to visualize all of these values together.
I have a few questions:
The value "access" appears to have been dropped out of the values mentioned in the YAML description. Was this intentional? If so, please remove it from here. If it was not intentional, please add it back in the new YAML file at
rfcs/text/0004/session.yml
I initially thought there was an overlap with values
session.kind: network
andsession.type: remote
. When adjusting the YAML file for readability it occurred to me thatsession.kind: network
was likely more to identify network level sessions, rather than anything that happens over a network. In other words, a web cookie-based session would not usesession.kind: network
, butsession.kind: application
. Am I understanding this correctly?However within the different values for
session.type
, I do wonder if there's overlap. It seems like "wired", "wireless" and "vpn" would always be accompanied by value "remote". Is this also how you see this? Note that this is not necessarily a problem. "local" vs "remote" could be useful when plotted against one another, whereas "wired", "wireless" & "vpn" could be useful to break down the kinds of "remote" session types.One final overlap concern: I do think
session.kind: system
andsession.authorization: service
are almost entirely overlapping, aren't they? Should we get rid of one of them, or am I misunderstanding a subtlety?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to find discrete combinations that would uniquely identify logging into a system, and a service running with authentication/authorization included in the "session". It may be unnecessary (I'll look thru auditbeat service logs and see what they look like... could just be that a system service is not something that would typically be defined as a session (e.g. starting up a vpn connection on a pfsense linux firewall - presumably the vpn application logs would be sufficient)
the revamp of the fields seems to lend itself more towards the entity being authorized - authentication would be a part of the overall session, but I don't think it makes sense as a specific session field. Basically the split becomes:
Not all of the type values will apply to every session.kind:
Session Kind:
-Network
I think this one could use some more input- I can see session.type having fixed single values (virtual implies remote), or using an array (vpn would be network & remote, published apps & citrix desktop would be virtual & remote, opening a virtualbox session locally could be local & virtual...)
Does that all make sense?
Wired & wireless however would typically not use remote as the session is for the local user or workstation directly connecting to the local network (as opposed to connecting to another entity over the network)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually wondering if virtual needs to be specifically identified... a "remote" "system" session will be telnet, ssh, rdp, or citrix... not sure the virtual type is necessary to split out rdp/citrix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And service as an authorization could also make sense for workstation based network access (802.1x cert auth), network to network vpn, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each of the log examples in the "Source data" section, could you include a breakdown of expected values for the session fields of each log event?
E.g. the ASA login could be something like
Not sure if one of the values in the ASA example is a session name or session id, but if so, list those as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - probably wouldn't be a bad idea to fill in everything in some additional example.json file(s)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For our RUM agent, running on client websites tracking browsing sessions, I can only see us setting
session.kind
andsession.id
. Would love feedback from the RFC authors on whether this makes sense.CC @jahtalab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea (only filling out relevant fields) makes total sense... tho in respect to this and the previous comment it makes me think I'm missing something that identifies a RUM type session (but then I also don't really know what other fields would be coming in that would make filtering results from a mixed index easy). Unauthenticated web sessions for instance would't need to use session.auth, but after a login could (along with user.name, etc.). I don't see that one as big a deal as e.g. network access logins (vpn, wireless, etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add some context, the RUM sessions are not necessarily authenticated, even until they are complete, e.g. a user session for going through checkout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jahtalab do we have any notion of auth no? AFAIK we don't, and it's unlikely given the extreme variety of auth methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewvc , We don't detect authentication per se, but there's a configuration for customers to add the user (username, id, ..) in the RUM agent. We can assume the user is authenticated (with some method) but it doesn't have to be.