Migrated from https://bugzilla.mozilla.org/show_bug.cgi?id=1014735
Assigned to: Julien Vehent [:ulfr]
On 2014-05-22 11:03:10 -0700, Julien Vehent [:ulfr] wrote:
ACLs are currently hardcoded in the agent. Instead, ACLs should be signed by a master key and stored in the postgres database. The Scheduler could then distribute ACLs to agents via AMQP. Agents would verify the master signature and update their ACLs accordingly.
On 2015-05-18 12:55:42 -0700, Julien Vehent [:ulfr] wrote:
The end goal is to remove the AGENTACL and PUBLICPGPKEYS configuration variables from the agent's static conf, and instead distribute them via AMQP or API calls.
The tricky part is distributing the AGENTACL in a secure manner. We don't want to just trust whatever is in the database because a server compromise could rewrite these values and grant access to a malicious investigator. The AGENTACL list provided to an agent MUST be signed by an entity the agent trusts, and the signing key MUST be kept offline.
The proposal is to add two new configuration variables to the agent:
- MEMBER_OF: an array of strings containing groups the agent is a member of. If empty, or does not contain any group known to the database, a membership to a group named "default" is assumed.
- MASTERKEYS: an array of public pgp keys used to verify ACLs
- REQUIRED_ACL_SIGNATURES: an integer that indicates the number of valid signature an ACL must provide to be considered valid
On startup, the agent requests one ACL for each group listed in MEMBER_OF, and receives a JSON document containing a list of permissions to modules, and a list of signatures. The agent verifies each signature, and if enough are valid to satisfy the value of REQUIRED_ACL_SIGNATURES, loads the ACL.
The verification process MUST require that at least two signatures from two different MASTERKEYS are provided for an ACL to be valid, thus preventing a single MASTERKEY compromise from putting the infrastructure at risk. In production, the MASTERKEYS array will contain 5 to 10 keys from trusted individuals, and we will probably require three valid signatures to update an ACL. But to facilitate development and adoption in small environments, REQUIRED_ACL_SIGNATURES is made configurable and set to 1 by default.
The format of the JSON document is described below:
{
"modules": {
"default": {
"minimumweight": 2,
"investigators": {
"Bob Kelso": {
"fingerprint": "E60892BB....",
"weight": 2
},
"Morpheus": {
"fingerprint": "AD595634....",
"weight": 3
}
}
},
"mymodule1": {
"minimumweight": 1,
"investigators": {
"Jean-Kevin": {
"fingerprint": "E60892BB...",
"weight": 1
}
}
}
},
"pgpsignatures": [
"wsBcBAABCAAQBQJVWjgMCRC0wd1JzRH...",
"SQNRn2dG7abwbID7bV7+F+TdtIzOhfq..."
]
}
In environment where builtin configuration is disabled, the variables MEMBER_OF and MASTERKEY will be read from the configuration file in <config_dir>/mig-agent.cfg. In the future, removing these variables from the built-in configuration will also allow us to distribute compiled binaries of the agent publicly.
RISKS
Building ACLs into the agent binary currently provides a good level of security, which we are removing. Providing a mechanism to override ACLs opens the door to privilege escalation attacks. Requiring multiple signatures on ACLs mitigates that risks if the keys listed in the MASTERKEYS are securely kept, and if enough signatures are required.
I would like to hear what others think of this plan. Implementation will probably wait until Q3 anyway, so we have time to discuss it.
On 2015-07-07 15:31:12 -0700, Guillaume Destuynder [:kang] wrote:
Doc
doc nit: According to this, the MASTERKEYS should also be configurable to a minimum of 2 so that it works with the defaults
doc nit: Should specify that by ACLs we mean the investigators, their weight, fingerprint, and minimum required weight per MIG module.
on topic
I think the signature vs builtin trade off is good.
on format
It doesn't explain how the pgp signatures are generated (which data can you exactly verify, json doc without the pgpsignatures entry?)
on logic
How does it work exactly for the MEMBER_OF selection?
If my understanding is correct:
With the scenario of the attacker having control of the server queue - I could see an attack where he would change the MEMBER_OF to a group of keys he has control of (even if those are signed by masterkeys), weakening the system.
this means MEMBER_OF needs to also be strongly verified, otherwise we have no other security than checking the keys have been signed by a MASTERKEY.
On 2015-07-10 06:57:27 -0700, Aaron Meihm [:alm] wrote:
Some questions I have on this:
- How will the new ACLs be published to the database? Is this anticipated to happen through the API, or would we have some out-of-band tool to sign an ACL and install it in the database? The latter is probably the best way to ensure compromise of the API/scheduler can't result in escalation of privilege in the agent. In the past the scheduler/API has always been considered "less trusted" then the agent, so that's one thing to consider for this is we are now relying more on the security of the API/scheduler for the agent itself. Granted, the ACL still needs a valid signature, but that wouldn't stop somebody from owning the API and pushing a intentionally bad ACL into the system to stop the agents from operating correctly.
- Should we include a "lifetime" for the ACL, so if new ACLs are not received after a given period of time the agent reverts to some default ACL configuration?
- Should we include a global group, that always passes for a given agent to avoid needing to duplicate users into multiple groups in the ACL?
On 2015-08-02 07:30:29 -0700, Julien Vehent [:ulfr] wrote:
With the scenario of the attacker having control of the server queue - I could see an attack where he would change the MEMBER_OF to a group of keys he has control of (even if those are signed by masterkeys), weakening the system.
I don't see this as a concern, because for that attack to happen, the attacker would need permission to change the configuration of the agent. That means either commit access to the configuration management system (puppet) or root on the target host. In both cases, that level of access give the attacker full root, and compromising MIG is probably the least of our worries.
How will the new ACLs be published to the database?
We will use code in the MIG console to build, sign and post ACLs to the API. The API endpoint will simply store the ACL in the database. If someone owns the API or database, they won't be able to sign fraudulent ACLs.
Should we include a "lifetime" for the ACL?
I can see pros and cons with this. It's definitely a good way to prevent an ACL from going stale and grant access to users who have left the company. It might also add some amount of work on our side to resign the ACL on a regular basis. I think we can decide on this later, and for now make sure we store the necessary timestamps.
Should we include a global group?
Yes. It's the group named default in comment 1.
On 2015-08-02 09:29:17 -0700, Julien Vehent [:ulfr] wrote:
I started implementation. We have two options for managing multiple ACLs: merging them, or treating them independently.
=== Merging ACLs ===
Given an agent that is member of two groups: base and mygroup. The agent retrieves two signed ACLs from the API, one for each group.Each ACL contains a list of modules, each containing a list of investigator's keyid and weight.
The agent loads the first ACL base (or whatever is first in the MEMBEROF array). Then loads the second ACL mygroup, and for each module that is present in both ACLs, merges the content.
Meaning that base->file->investigators and mygroup->file->investigators will be merged together. Any investigator present in both will be kept only ones.
It is possible that both ACLs will have different values for minimumweight, in which case only the highest value should be kept.
Similarly, it is possible that a given investigator present in both lists would have a different weight, in which case only the highest value is kept.
=== Treating ACLs independently ===
We could also treat ACLs independently, and thus avoid the complexity of merging their content. It would make the code slightly simpler, and probably remove confusion when defining new ACLs that do not behave as expected because another ACL is being merged on top.
The downside is dealing with minimumweight across ACLs. A given action could have two signatures from investigators in two different ACLs.
- The first signature could belong to investigator X, have a weight of 2 and a minimum weight of 3.
- The second signature could belong to investigator Y, have a weight of 2 and minimum weight of 5.
In this situation, the total weight would be 4, which satisfies the minimumweight of the first ACL, but not the second one. What should MIG do in this case is an open question...
I haven't decided which approach to take yet. Any thought?
On 2015-08-03 04:30:39 -0700, Aaron Meihm [:alm] wrote:
It seems the simplest approach would be to treat it like an access list. Merging the ACLs sounds useful but would the behavior outlined always hold as being desired? For example, would we always merge in the higher weight ACL, or would there be a scenario where if somebody has assigned a lower weight to the ACL, that would win in the merge logic as we'd default to the lowest agreed upon privilege?
In the second example, would it make things simpler if we just have a minimumweight value for modules that is the same across ACLs to avoid the confusion?
On 2015-08-03 07:45:28 -0700, Julien Vehent [:ulfr] wrote:
The problem with detecting conflicts server-side is that the server doesn't know which ACLs an agent will request, so it cannot detect in advance that ACLs A and B have conflicting values.
I like the idea of merging the ACLs better than treating them separately, and I agree with you that we should default to better security, with the following rules:
- When merging ACLs that have different
minimumweight values for a given module, the highest minimumweight value wins.
- When merging investigators that have different weight on a given module, the lowest weight (least privilege) value wins.
Sounds good?
On 2015-08-03 09:34:20 -0700, Julien Vehent [:ulfr] wrote:
Here's an example of ACL:
{
"name": "testacl",
"validfrom": "2015-08-01T12:05:23.008403417-04:00",
"expireafter": "2015-08-31T12:05:23.008403445-04:00",
"permissions": [
{
"module": "testmodule",
"minimumweight": 1,
"investigators": [
{
"id": 1,
"name": "Test Investigator",
"pgpfingerprint": "abcdef0123456789",
"weight": 1
}
]
}
],
"pgpsignatures": [
"wpwEAAEIABAFAlW/k2QJEF4AASq0LBwdAADY3gQACOKNzFlw1mQNB92vyeUUnCeD6CeGI+qlACrIgOIr09FJQOip7X4rRqrr4oi3e0oj9upqH0TiWacJBVK3VubD5J7NcGNlSjgcwe6oVtt2ieMci8z2ZiXt4+eJ/bUfRsAKw+c9qV0oi0Sxm4cdxsEBwratQrMFUv3qIcoPjSkLx4==Sw35"
]
}
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent