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

Add GitLab CI signing example #26

Closed
aarongoldenthal-work opened this issue Oct 6, 2023 · 7 comments · Fixed by #143
Closed

Add GitLab CI signing example #26

aarongoldenthal-work opened this issue Oct 6, 2023 · 7 comments · Fixed by #143

Comments

@aarongoldenthal-work
Copy link

Given the broad use of GitLab in environments where an OIDC provider as a CA has significant benefits, it would be helpful to have a reference example.

@EthanHeilman
Copy link
Member

@aarongoldenthal-work Thanks, it would be great to support gitlab. I am looking into it, but will probably be a little while before this is added.

We are accepting PRs if you wanted to write a proof-of-concept I'd be happy to work with you to get merged.

@kipz
Copy link
Contributor

kipz commented Nov 2, 2023

It looks as if GitLab OIDC provider for workloads/automation doesn't support setting any claims on-the-fly, so we may not be able to bind the ephemeral keys to the identity.

@EthanHeilman @jonnystoten should we consider a mode for workload/code-signing whereby we use GQ to sign directly and don't generate and use the ephemeral keys at all?

IMHO, having a story for GitLab is super important.

@kipz
Copy link
Contributor

kipz commented Nov 2, 2023

There's a GitLab issue where this capability is discussed and one of the maintainers states that supporting this would weaken the security of GitLab overall, and should not be implemented:

https://gitlab.com/gitlab-org/gitlab/-/issues/388514

I'm not sure I fully agree, but that's where we are I think.

@jonnystoten
Copy link
Member

I'm interested in the idea of using GQ to sign the CIC. I think this means we don't need to bind the key using the nonce/audience claim. This could be useful for GitHub as well, because it would allow us to create multiple PK tokens with different CICs from the same ID token. The disadvantage is that the original OIDC token is more valuable if compromised. Personally I think that's a reasonable tradeoff, keen to hear other opinions though.

I can create another issue for GQ signing the CIC so we can keep this one focused on GitLab integration.

@EthanHeilman
Copy link
Member

EthanHeilman commented Nov 2, 2023

The disadvantage is that the original OIDC token is more valuable if compromised. Personally I think that's a reasonable tradeoff, keen to hear other opinions though.

@jonnystoten I think it is a reasonable trade off as well. However in this setting you need to be very careful that you don't use the same audience claim for non-GQ ID Token because then anyone can use the RSA signature to generate a GQ signature on that ID Token. We need to very explicitly flag it as GQ only.

I was playing around with the right way to represent this last night:

Set the audience claim to aud=GQ-Only-<Intended Audience>. If we see a token with this audience claim prefix we should ensure it is from the list of GQ only issuers such as gitlab and reject otherwise. I believe this can be fit in with the proposed GQ JWS representation mechanism from: #15 (comment)

ID Token you get from Gitlab

{
    "payload":  BASE64URL({
      "aud": GQ-Only-<Intended Audience>,
      ...
    }), // ID Token
    "signatures": [
        {
            "protected": BASE64URL({"alg":"RSA256","kid":"6","typ":"JWT"}),
            "signature": BASE64URL(RSAsig) // OpenID Provider Signature
        },

aud=GQ-Only-
ID Token that had signature and protected header transformed by a GQ signature

{
    "payload": BASE64URL({
      "aud": GQ-Only-<Intended Audience>,
       ...
    }), // ID Token
    "signatures": [
        { // OP (OpenID Provider) 
            "protected": BASE64URL({
                "alg":"GQ256",
                "kid":BASE64URL(originalPh={"alg":"RSA256","kid":"6","typ":"JWT"}),
                "typ":"JWT",
                }),
            "signature": BASE64URL(
                 GQSign(
                   RSAsig, 
                   RSAmsg=(originalPh,payload),
                   GQmsg=SHA3(cic)
                 )
           )
        },

This way we can commit to the cic via a cic hash in the ID Token, but rather than do it in the nonce or aud we do it in the new protected header used by the GQ signature.

@EthanHeilman
Copy link
Member

There's a GitLab issue where this capability is discussed and one of the maintainers states that supporting this would weaken the security of GitLab overall, and should not be implemented:

My understanding of the security issue is that:

  • Gitlab has a permission system that can control what YAML can be set,
  • Gitlab does not have a permission system that can control what happens outside of the YAML,
    This means that just replicating getIDToken(audience) function like in gitlab would be very dangerous because the repo maintains would not be able to set permissions on which runners in their repo could call this. I believe this is ok in github because github has permissions for what a runner can do outside of just runner YAML configuation. I could be wrong about this I've only started to the read the gitlab source code.

What I don't understand is why the following approach was not taken. Create a YAML keyword id_token_req_key that grants a key for requesting an ID Token and have the code be getIDToken($id_token_req_key, audience). I think this pattern would let you enforce all sorts of permissions on runners while providing more flexibility.

Ideally what I would want would be:

sign:
  id_tokens:
    : id_token_req_key
      aud_prefix: "openpubkey"

getIDToken($id_token_req_key, aud_postfix)

That way you could enforce aud_prefixes at the yaml level.

@EthanHeilman
Copy link
Member

@aarongoldenthal-work I wanted to let you know I just created a PR that adds gitlab. I'd be interested in your feedback/review

#143

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

Successfully merging a pull request may close this issue.

4 participants