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

Expect keyid/scope as part of the token in Bundle signing #4462

Closed
deepapt opened this issue Mar 23, 2022 · 11 comments
Closed

Expect keyid/scope as part of the token in Bundle signing #4462

deepapt opened this issue Mar 23, 2022 · 11 comments

Comments

@deepapt
Copy link

deepapt commented Mar 23, 2022

I used the below command to generate the signature.json for bundle signing.

$ ./opa_windows_amd64.exe sign --signing-key private-key.pem --bundle vdna/

After Decoding the signature JWT, it looks like this:

{
  "files": [
    {
      "name": "vdna/data.json",
      "hash": "9c202b67235b5be212dbcc9ff8754fd933745cb072caa8c1ad3e4cc5729383a4",
      "algorithm": "SHA-256"
    },
    {
      "name": "vdna/policy.rego",
      "hash": "ff9e76e77ed2db90766af06a3f288f1fdfdcda8c7a88dd3727f02d0e65b2fa99",
      "algorithm": "SHA-256"
    }
  ]
}

I expect iat/iss/keyid/scope as part of token.

@deepapt deepapt added the bug label Mar 23, 2022
@srenatus
Copy link
Contributor

I'm not sure this would classify as a bug. We're not claiming to be standards-compliant, I believe -- is there some standard that makes you expect those infos in the token?

@srenatus
Copy link
Contributor

💡 So our docs mention those fields, https://www.openpolicyagent.org/docs/latest/management-bundles/#signature-format

...but it's unclear to me (at least) where anything but scope could come from 🤔

@srenatus
Copy link
Contributor

Reading the docs again, I believe those fields do not matter at all: https://www.openpolicyagent.org/docs/latest/management-bundles/#signature-verification

kid: If supplied in the header, dictates which key (and algorithm) to use for verification. The actual key is supplied via OPA out-of-band

☝️ That means it's not part of the signature payload, it's just in the ordinary JWT header.

iat: unused for verification even if present in payload
iss: unused for verification even if present in payload

@deepapt What is your use case? What do you need these headers for?

Anyways, I've tried singing a bundle and the header I get is

{
  "alg": "RS256"
}

so that doesn't explain where iat/iss/id could come from...

@srenatus srenatus added the bug label Mar 23, 2022
@anderseknert
Copy link
Member

anderseknert commented Mar 23, 2022

Thanks for reporting this @deepapt 👍 Looks like a mixed bag of issues, really..

  1. The docs make it seem like iat, iss and scope are added to the claims when building a .signatures.json file, but this is demonstrably not so.
  2. The docs mention the above claims as not required, but makes no attempt to explain how to include them if desired.
  3. opa build provides a --scope option for verifying the scope claim, but there's no equivalent flag for opa sign.
  4. The above claims can be added to the bundle signature file using a custom --claims-file, but it's certainly not clear that's the intended way to get them included (as any arbitrary claim can be added that way) and one would think that at least iat would be something calculated by the bundle signer and not through a manual process.

Might have missed something, but that's at least a starting point :)

@anderseknert
Copy link
Member

OPA build has this:
--verification-key-id string name assigned to the verification key used for bundle verification (default "default")

But opa sign doesn't. Not really clear to me how those two commands are meant to play together (if at all).

@deepapt
Copy link
Author

deepapt commented Mar 24, 2022

The link shows the additional keys so that they are able to understand the context as well.

https://www.openpolicyagent.org/docs/latest/management-bundles/#signature-verification

In the example the provided keyid for bundle signing. That keyid is getting extracted from the signature.

services:

bundles:
authz:
service: acmecorp
resource: somedir/bundle.tar.gz
persist: true
polling:
long_polling_timeout_seconds: 10
signing:
keyid: my_global_key
scope: read

Signature:

{
"signatures": [ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmaWxlcyI6W3sibmFtZSI6Ii5tYW5pZmVzdCIsImhhc2giOiJjMjEzMTU0NGM3MTZhMjVhNWUzMWY1MDQzMDBmNTI0MGU4MjM1Y2FkYjlhNTdmMGJkMWI2ZjRiZDc0YjI2NjEyIiwiYWxnb3JpdGhtIjoiU0hBMjU2In0seyJuYW1lIjoicm9sZXMvYmluZGluZ3MvZGF0YS5qc29uIiwiaGFzaCI6IjQyY2ZlNjc2OGI1N2JiNWY3NTAzYzE2NWMyOGRkMDdhYzViODEzNTU0ZWJjODUwZjJjYzM1ODQzZTcxMzdiMWQifV0sImlhdCI6MTU5MjI0ODAyNywiaXNzIjoiSldUU2VydmljZSIsImtleWlkIjoibXlQdWJsaWNLZXkiLCJzY29wZSI6IndyaXRlIn0.ZjtUgXC6USwmhv4XP9gFH6MzZwpZrGpAL_2sTK1P-mg"]
}

After Decode:

{
"name": ".manifest",
"hash": "c2131544c716a25a5e31f504300f5240e8235cadb9a57f0bd1b6f4bd74b26612",
"algorithm": "SHA256"
},
{
"name": "roles/bindings/data.json",
"hash": "42cfe6768b57bb5f7503c165c28dd07ac5b813554ebc850f2cc35843e7137b1d"
}
],
"iat": 1592248027,
"iss": "JWTService",
"keyid": "myPublicKey",
"scope": "write"
}

@ashutosh-narkar
Copy link
Member

Just to clarify about opa sign, if you run the command w/o providing any additional claims (ie. no --claims-file), your JWT payload will only represent the files in your bundle under the files key. If you want additional claims in your JWT payload, then you will need to provide a claims file using the --claims-file flag. The format of such as file:

{
    "scope": "read"
}

With the above the JWT payload will have keys files and scope. opa sign --help will have more information about the command usage.

@ashutosh-narkar
Copy link
Member

If you need a single command to build and sign your bundle, opa build would be the way to go. If you run opa build --help, you will more detailed information on this.

@ashutosh-narkar
Copy link
Member

We should clarify the points raised in this comment and make it part of the docs for the feature. I'll work on getting those in.

@ashutosh-narkar
Copy link
Member

FYI docs updated with more info about signing in this PR.

@anderseknert
Copy link
Member

Closing as doc fixes got merged.

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

No branches or pull requests

4 participants