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

Feature request: using OPA Agent instead of assertions in EAS #33

Open
JrCs opened this issue Oct 13, 2019 · 22 comments
Open

Feature request: using OPA Agent instead of assertions in EAS #33

JrCs opened this issue Oct 13, 2019 · 22 comments

Comments

@JrCs
Copy link

JrCs commented Oct 13, 2019

Hi,

This is a request feature to use the OPA Agent instead of assertion parts of EAS.
With this we can use a common DSL language (rego) to create policies for authz.

@travisghansen
Copy link
Owner

Very cool, I'll do some research and see where/how it might fit. I'm not familiar with OPA so if you have any insights to share on that and how it would integrate please share.

@JrCs
Copy link
Author

JrCs commented Oct 13, 2019

I don't have so much information for OPA.
The basics is you send a JSON struct to the OPA agent (via a REST API) and it validate the json using the rego language.
OPA agent returns an HTTP 200 response code if the policy was evaluated successfully. Non-HTTP 200 response codes indicate configuration or runtime errors. The policy decision is contained in the "result" key of the response message body.
For example ISTIO can use OPA Agent to evaluate authz: https://istio.io/docs/reference/config/policy-and-telemetry/adapters/opa/

@travisghansen
Copy link
Owner

Does the client specify which policy/rule to apply (already defined on the opa server) when making the request? This seems like a pretty cool idea and fits nicely with some longer term changes I'd like to make to assertions. I'd like to combine all assertion data into a single struct and make the rule definitions operate on that instead of the individual datasets (ie: cross check something in userinfo against some portion of the request data, or cross check userinfo against access token etc).

@JrCs
Copy link
Author

JrCs commented Oct 13, 2019

Yes the client can specify the policy an optionally the rules (you can have multiple rules by policy) to use on the opa server. See:

@travisghansen
Copy link
Owner

Very cool. I'll start digging around at it and see how it might fit into the picture.

@mlushpenko
Copy link
Contributor

Haha, great more people like OPA. We were thinking to use it after EAS instead of Istio authorization. Istio supports OPA as well, but we didn't see an exact benefit versus Istio native functionality. But could be great to separate core EAS installation from the assertion logic, although another dependency would be introduced in this case. Perhaps it could be like some opt-in feature of EAS.

@mlushpenko
Copy link
Contributor

@travisghansen I think an interesting part for EAS could be this https://www.openpolicyagent.org/docs/latest/http-api-authorization/. It didn't work for us because we would have to rewrite many apps to call OPA agent from inside the app, but for EAS as a central piece of authz workflow, it would be awesome I think.

@travisghansen
Copy link
Owner

Yeah it would simply be specific type of assertion I think. If nothing is defined in the opa assertion block then it would never get invoked.

I think this is pretty trivial to implement honestly and seems fairly powerful. I would have performance concerns about a sub request within a sub request but the added latency becomes a concern to the operator I suppose and a decision for them to make. At least they'll have the option.

@mlushpenko
Copy link
Contributor

Thinking through your reply again, maybe you don't need to provide any OPA block, just allow possibility to call OPA for request validation like in the link I shared, like specifying OPA endpoint and a flag to use it or not. So, OPA installation etc is left to the user

@travisghansen
Copy link
Owner

Yeah, I would leave the installation to the user for sure, I think I would handle this similar the to the header injection stuff that can be applied at the plugin and/or the config_token level. In the config we'd have something like:

opa: [
 {
  url: <blah>
  rules: <blah>
  ...<other appropriate configuration options>
 }
 ...2nd/3rd/etc endpoints to allow for multiple assertions policies
]

We'd create a normalized data block containing information about the request to eas, parent request, specific plugin being analyzed, whole config_token, authorization data (ie: tokens for oauth/oidc, whatever else for other plugins), etc and pass that along. If the result is pass then additionally add the opa data to the same block of info that is/can be used for header injection with the opa data.

Happy to hear other thoughts. Seems like a pretty powerful framework..wished I'd bumped into it sooner.

Above and beyond the idea of replacing the assertions, it feels like it could be a plugin all it's own depending on how much code can be imported to the endpoints/policies (ie: ldap clients and the like). We'd simply need to define a proper result/response structure that effectively would expose the whole functionality of the internal plugins (ie: set response code, headers, and cookies in essence) and just let eas pass the response on (this is effectively what the forward plugin is already).

@mlushpenko
Copy link
Contributor

Sounds good. I was thinking something in the lines of gatekeeper that rules themselves could be kubernetes objects but then realised that your project is not only kubernetes-specific, so having rules defined inside config_token set up may be better. Also, things like gatekeeper deal with crud operations on kubernetes, while for the proxy - api autorization I posted above is needed and other things don't matter

@travisghansen
Copy link
Owner

@mlushpenko yeah. One of the items on the todo is to add features that determine if/when specific routes are authenticated or not (similar to items described here: https://www.keycloak.org/docs/latest/securing_apps/index.html#_keycloak_generic_adapter). I wasn't sure how badly I wanted to get into that but this seems like it could be a happy medium (if replacing assertions) or full blown replacement (if a full plugin was created).

In any case, lots of good stuff to review here and design.

@mlushpenko
Copy link
Contributor

Indeed, lots of stuff =D One of the items on the todo is to add features that determine if/when specific routes are authenticated or not I'd think that this is usually done by the proxy itself like nginx/envoy but perhaps could be useful for eas as well. Actually, eas right now protects all https traffic for us and to make some endpoint public, we need to have a separate entry point not protected by eas, so this could be very useful if I think about it twice :)

@travisghansen
Copy link
Owner

@mlushpenko yeah, I'd agree about the handling at the proxy level (that was my original thought as well). I can however see folks wanting a unified (and proxy agnostic) solution. We'll see where it all goes.

@travisghansen
Copy link
Owner

On a related note, I may create a specific plugin to handle endpoint auth/no-auth logic. I've realized that with clever combination of the existing noop plugin with pcb it possible to achieve this for the most part but doesn't allow for multiple criteria (ie: path and host and verb). I'm still thinking through the opa stuff as well. I really need to unify some of the configuration options/data across plugins a little better which will make this easier to implement and maintain going forward.

@mlushpenko
Copy link
Contributor

That would be great, as I mentioned before we do have use-cases for such functionality and the workaround for now is to use separate endpoint not protected by EAS for public endpoints

@travisghansen
Copy link
Owner

Just getting around to revisiting this feature. Do you see more value in integrating with an existing instance of opa using the http-api-authorization feature, or running the binary locally with eas and doing an opa eval?

I've mostly got concerns about adding another http request from a performance perspective. We could also run opa locally over a unix socket which could help I suppose.

In any case, this integration opens up some really cool stuff, just want to make sure it gets done 'right'.

@travisghansen
Copy link
Owner

travisghansen commented Mar 12, 2020

@mlushpenko any thoughts from you on this one? I've been doing my research and feel I have a decent enough grasp on opa at this point to start firing up some code..

@mlushpenko
Copy link
Contributor

hey, @travisghansen, it's been some time since our last discussion (not taking into account cookie settings). I've reviewed this thread a bit to refresh the memory and I can say that with the newer version of istio, we do handle public/private endpoints on the level of ingress. But still thinking that doing it on EAS side could have been a bit easier, envoy filters are not easy to work with.

Apart from that, just sharing info :) we are using header auth plugin so that our monitoring system can bypass oidc and monitor our endpoints. Testing full authz/authn flow would be awesome, but much more work and we don't have time right now.

Next one, regarding performance, I agree it is important. We have some problems with networking and wanted to increase the timeout for outgoing requests, it is hardcoded here for 10 seconds. So, adding extra request could lead to more problems and then doing it locally seems very interesting.

But the bad news from my side, I am leaving the company at the end of March and my colleague still doesn't have github account even though he is the one usually diving into eas code to try some new settings :) What I am saying is I will likely not be able to test your changes, so if not many other people are interested, perhaps it's not worth to spend your time. I will check at a new place if there is a space to use EAS there :)

And I can still review/discuss changes if that's good enough, just won't have a production system to see real usage of the new feature.

@travisghansen
Copy link
Owner

@mlushpenko no problem! best wishes at the new place :)

@mlushpenko
Copy link
Contributor

One more cool thing, that colleague wrote some integration so that all tokens are configured in a json file like this (base shared config + overrides of clients):
image

and then this config is saved as eas-config-tokens kubernetes secret which is used by an initContainer to generate actual tokens and then via shared volume mount tokens are propagated to the EAS container:

apiVersion: v1
kind: Deployment
metadata:
  name: eas-external-auth-server
spec:
  template:
    spec:
      containers:
      - name: external-auth-server
        volumeMounts:
        - name: config-shared
          mountPath: /config
      initContainers:
      - name: generate-config-tokens
        imagePullPolicy: Always
        image: docker-registry.dimension.ws/hal24k/eas-generate-config-tokens:latest
        command:
        - node
        - /src/generate-config-tokens.js
        - /eas-config-tokens.json
        - /config/config-tokens.json
        env:
        - name: EAS_CONFIG_TOKEN_SIGN_SECRET
          valueFrom:
            secretKeyRef:
              name: eas-external-auth-server
              key: config-token-sign-secret
        - name: EAS_CONFIG_TOKEN_ENCRYPT_SECRET
          valueFrom:
            secretKeyRef:
              name: eas-external-auth-server
              key: config-token-encrypt-secret
        envFrom:
        - secretRef:
            name: eas-external-auth-server
        volumeMounts:
        - name: eas-config-tokens
          mountPath: /eas-config-tokens.json
          subPath: eas-config-tokens.json
        - name: config-shared
          mountPath: /config
      volumes:
      - name: config-shared
        emptyDir: {}
      - name: eas-config
        secret:
          secretName: eas-external-auth-server
      - name: eas-config-tokens
        secret:
          secretName: eas-config-tokens

And in helm values file, this snippet is used to use thhe file with tokens:

configTokenStores:
  primary:
    adapter: file
    options:
      cache_ttl: 1
      path: /config/config-tokens.json

And generateTokens script:

const fs = require('fs');
const jwt = require("jsonwebtoken");
const crypto = require("crypto");
const merge = require('deepmerge')

const algorithm = "aes-256-cbc";

function exit_failure(msg) {
  console.log(msg)
  process.exit(1);
}

function encrypt(salt, text, encoding = "base64") {
    try {
      var cipher = crypto.createCipher(algorithm, salt);
      var encrypted = Buffer.concat([
        cipher.update(Buffer.from(text, "utf8")),
        cipher.final()
      ]);
  
      return encrypted.toString(encoding);
    } catch (exception) {
      throw new Error(exception.message);
    }
  }

const strategicMerge = (targetArray, sourceArray, options) => {
  sourceArray.forEach(source => {
    const mergeKey = source[options.arrayMergeKey]
    if (!mergeKey) {
      targetArray.push(source)
    } else {
      targetArray.forEach((target, idx) => {
        if (target[mergeKey] === source[mergeKey]) {
          let clone = options.cloneUnlessOtherwiseSpecified(source, options)
          delete clone[options.arrayMergeKey]
          targetArray[idx] = merge(target, clone, options)
        }
      })
    }
  })
  return targetArray
}


var args = process.argv.slice(2);
if (args.length != 2) exit_failure("Invalid arguments")

var requiredEnvs = [
  "EAS_CONFIG_TOKEN_SIGN_SECRET",
  "EAS_CONFIG_TOKEN_ENCRYPT_SECRET"
]
requiredEnvs.forEach(varName => process.env[varName] || exit_failure(`Missing required environment variable "${varName}"`))

console.log(`Reading config from ${args[0]}`)
sourceFile = JSON.parse(fs.readFileSync(args[0], "utf-8"))

baseConfig = sourceFile["baseConfig"] || exit_failure("Missing baseConfig in source file")
tokens = sourceFile["tokens"] || exit_failure("Missing tokens in source file")

generatedTokens = {}
for (let [tokenId, tokenConfig] of Object.entries(tokens)) {
  let generatedToken = {
    eas: {
      plugins: merge(baseConfig, tokenConfig, { arrayMerge: strategicMerge, arrayMergeKey: "mergeKey" })
    }
  }
  console.log(`Generated token ${tokenId}: ${JSON.stringify(generatedToken)}`)
  let encryptedToken = encrypt(process.env["EAS_CONFIG_TOKEN_ENCRYPT_SECRET"], jwt.sign(generatedToken, process.env["EAS_CONFIG_TOKEN_SIGN_SECRET"]))
  console.log(`Encrypted token ${tokenId}: ${encryptedToken}`)
  generatedTokens[tokenId] = encryptedToken
}

console.log(`Writing generatedtokens to ${args[1]}`)
fs.writeFileSync(args[1], JSON.stringify(generatedTokens))

I asked him to create a PR or at least open an issue to discuss it, but don't think he'll do it and it's something I had in mind long time ago but was lazy to do it, but wanted to share it with you, so perhaps you will actually incorporate it into EAS.

My colleague's name is Samuel Hessel, just to give credit to him in case he ever creates his GitHub account :)

@travisghansen
Copy link
Owner

Ah, good to know, that looks like some pretty cool work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants