-
Notifications
You must be signed in to change notification settings - Fork 519
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
Introduce apm-server.auth.anonymous
config
#5532
Conversation
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errorsExpand to view the tests failures> Show only the first 10 test failures
|
Also, remove AnonymousAuthorizationMiddleware. We now use AuthorizationMiddleware with a single auth builder that handles anonymous auth.
We also add a method to authorization.Handler to disable anonymous auth for just that handler. This is used to provide a handler that does not allow anonymous access for a specific routes: sourcemap upload).
Use a single Authorization middleware for both backend and RUM agents. For the RUM path, we rely on the authorization.Builder's new support for anonymous auth.
Also, use the new apm-server.auth config.
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 wanted to test this through manually to get a better understanding, but always receive
{
"accepted": 0,
"errors": [
{
"message": "unauthorized: agent \"js-base\" not allowed"
}
]
}
# When anonymous auth is enabled, only agents matching allow_agent and services | ||
# matching allow_service are allowed. See below for details on default values for | ||
# allow_agent. | ||
#enabled: |
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.
In a follow up - I think we should also deprecate apm-server.rum.enabled
and derive it from anonymous.enabled
in combination with anonymous.allow_agent
.
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'd probably:
- deprecate
apm-server.rum.enabled
, and have it always enabled by default in 8.0 - default
apm-server.auth.anonymous.enabled: true
Which would be effectively the same I think
result.Reason = fmt.Sprintf("agent %q not allowed", resource.AgentName) | ||
case resource.ServiceName != "" && len(a.allowedServices) > 0 && !a.allowedServices[resource.ServiceName]: | ||
result.Authorized = false | ||
result.Reason = fmt.Sprintf("service %q not allowed", resource.ServiceName) |
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.
Wouldn't this mean that empty agentName
and empty serviceName
is authorized? How about:
result := Result{Authorized: true, Anonymous: true}
case len(a.allowedAgents) > 0 && (resource.AgentName == "" || !a.allowedAgents[resource.AgentName]):
result.Authorized = false
result.Reason = fmt.Sprintf("agent %q not allowed", resource.AgentName)
case len(a.allowedServices) > 0 && (resource.ServiceName == "" || !a.allowedServices[resource.ServiceName]):
result.Authorized = false
result.Reason = fmt.Sprintf("service %q not allowed", resource.ServiceName)
}
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.
Indeed, it's by design:
- the initial auth check for requests uses
Resource{}
, to check if the client has some access before agent/service is known - for some endpoints (one endpoint -- agent config) the agent name isn't known, only service name is known
To prevent agents from bypassing the allowed agent/service checks we rely on agent and service name always being set where it matters. In agent config the service name is always set, and in intake both the agent name and service name are always set.
TBH I don't really feel 100% comfortable with this, so I'm open to alternatives.
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.
That's indeed a tricky one!
How about we set a any
(or *
or something else) as agent name for the initial check, and give it a special meaning in the auth check, but ensure that this value is removed in case it is set by the requester?
for some endpoints (one endpoint -- agent config) the agent name isn't known, only service name is known
this means we would need to have dedicated auth logic per handler, defining which information needs to be passed via the request. Also not certain how we could integrate that well; will think a bit more about it.
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.
Another more radical option: introduce two different methods, Authenticate and Authorize.
Authenticate checks that a provided secret token or API Key is valid, or (if no auth token is provided) that anonymous access is allowed. If API Key auth is used, the API Key ID will be obtained so we can associate it with events.
If an agent authenticates, we assume it is allowed to send events and query agent config and we'll proceed to decoding events and parsing agent config queries. This is effectively like AuthorizedFor(Resource{}) succeeding. Once authenticated, Authorize will be used to check that that the agent is allowed to perform an action for a given agent name and/or service name.
The challenge with this is that we currently have the ability to create API Keys for sending events, querying agent config, and indexing source maps. Since source map uploading is moving to Kibana, I think we could simplify things by saying that if an agent has an API Key with some apm application privileges, then it's good enough for agent authentication.
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 that. It would generally be cleaner to distinguish between authentication and authorization.
}, | ||
"allow_agent_unspecified": { | ||
allowAgent: []string{"iOS/swift"}, | ||
resource: authorization.Resource{ServiceName: "opbeans-ios"}, // AgentName not specified |
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.
Why is this not denied? There is a set of allowed agents specified, which can be worked around by simply not sending an agent name.
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.
It's not possible to not send an agent name.
}, | ||
"allow_service_unspecified": { | ||
allowService: []string{"opbeans-ios"}, | ||
resource: authorization.Resource{AgentName: "iOS/swift"}, // ServiceName not specified |
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.
same as above - why is an empty service name allowed when a list of allowlisted services is specified?
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.
same as above :)
I'll rebase and reopen once #5545 is done. |
Motivation/summary
Generalise the ability for agents to send events unauthenticated (anonymous) but rate-limited. Until now this has been a RUM-only feature, but we now find ourselves needing it also for the iOS agent.
Anonymous auth is disabled by default, but is automatically enabled when RUM is enabled as long as
apm-server.auth.anonymous
hasn't been explicitly configured. The existing RUM config for allowed services and rate limiting are deprecated and replaced with equivalent config underapm-server.auth.anonymous.*
.Instead of restricting anonymous auth to requests going by endpoint (i.e. RUM intake and agent config), we now restrict based on the provided agent and service names. There was previously nothing stopping clients from spoofing RUM agents, e.g. sending events to the RUM intake with a non-RUM agent name, so this is not any less secure.
Checklist
How to test these changes
apm-server.auth.anonymous.allow_agent: [iOS/swift]
, check that the iOS/swift agent can send data without an auth token.apm-server.auth.anonymous.allow_service: [opbeans-rum]
, check that opbeans-rum can send data. Change it to something else and check that opbeans-rum cannot send data.Related issues
Closes #5347