Refactor Packages for Auth Logic for Transparent Release

[project-oak]

Refactor Packages for Auth Logic for Transparent Release

[tiziano88]

Thanks, this is useful context. I think modelling the association to a principal via an interface with the extra Identify() method is quite dangerous in Go though, since anything that implements that method implicitly implements the interface. So, for instance, a "malicious" wrapper may not only implement EmitStatement, but also Identify so that it returns some other principal, which would allow it to impersonate something else. The top-level can still override that (and hopefully it does), but it's risky because if it forgets, then the impersonated principal will be used.

I guess another option is to instead use a method that accepts an object that implements EmitStatement, and then an explicit principal as another parameter. Then this method will only be usable by the top-level, so there is no risk of impersonation.

But let me know if there is something else I'm missing.

[aferr]

I don't actually think any of these choices defend against a malicious wrapper. Even if you just pass a principal as another argument, the text that a malicious wrapper emits in the body could be wrong. The point of separating it this way is to just let the consumer of the wrapper set the names of the principals separately from the bodies for convenience, not to defend against a malicious wrapper (I don't really think that's possible). So since none of these choices are indistinguishable in terms of defending against malicious wrappers, I am inclined to leave it as-is.

[tiziano88]

Even if you just pass a principal as another argument, the text that a malicious wrapper emits in the body could be wrong.

Yes, but at least (thanks to authorization logic) it would not affect others that don't already trust that principal, right? That to me seems quite desirable. Then again, of course all of this ends up compiled into a single binary, so it's hard to really say that someone trusts one wrapper and not another, but at least conceptually I think it makes sense.

[aferr]

Closing because we're not doing this