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

Obtaining public key is enough to authenticate with SSO #12992

Open
3 of 4 tasks
kvedes opened this issue Apr 29, 2024 · 9 comments
Open
3 of 4 tasks

Obtaining public key is enough to authenticate with SSO #12992

kvedes opened this issue Apr 29, 2024 · 9 comments
Labels
area/sso-rbac P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important type/bug type/security Security related

Comments

@kvedes
Copy link

kvedes commented Apr 29, 2024

Pre-requisites

  • I have double-checked my configuration
  • I have tested with the :latest image tag (i.e. quay.io/argoproj/workflow-controller:latest) and can confirm the issue still exists on :latest. If not, I have explained why, in detail, in my description below.
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

I have SSO configured with Azure Entra ID. I have experimented a bit with the authentication, and it seems to me, that something is wrong in the SSO flow. Argo seems to discard all information in the token received from Azure before it issues its' own token. I have created some simple Go code to illustrate it. If I extract the sso secret created by Arog Workflows and run the code below, I can create a token which will successfully authenticate. The code only uses the public part of the sso key. Note that I put in an object ID from an Entra ID group which is allowed access through the Azure Enterprise App which Argo uses for authentication (putting 000-.. as an example). Such an ID is easily obtained by people inside an organization, as Entra object ID's are not secret .

This means that obtaining the public key is enough to authenticate with Argo when SSO is enabled. I realize that the sso secret is not publicly accessible, however it seems to me that this should not be possible regardless. My knowledge of authentication is limited, but I would expect to see some of the properties from the Azure token contained in the Argo token. In that way Argo can verify that the client has actually authenticated with Azure beforehand.

Found this issue which proposes a solution I would solve my issue: #12049

TLDR; Argo SSO doesn't encode anything from the Azure token into its' own token, thus use of public key is enough to authenticate.

Version

v3.4.11

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

package main
 
import (
    //"crypto"
    "crypto/x509"
    "encoding/pem"
    "fmt"
    "time"
    "github.com/go-jose/go-jose/v3"
    "github.com/go-jose/go-jose/v3/jwt"
    "github.com/argoproj/argo-workflows/v3/server/auth/types"
)
 
// EncryptJWTToken encrypts a JWT token using the provided private key in PEM format.
func EncryptJWTToken(privateKeyPEM string) (string, error) {
    // Parse the private key from PEM format
    block, _ := pem.Decode([]byte(privateKeyPEM))
    if block == nil {
        return "", fmt.Errorf("failed to decode private key")
    }
    privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes)
    if err != nil {
        return "", fmt.Errorf("failed to parse private key: %v", err)
    }
 
    // Create a JWE encrypter with the **public** key
    encrypter, err := jose.NewEncrypter(jose.A256GCM, jose.Recipient{Algorithm: jose.RSA_OAEP_256, Key: privateKey.Public()}, &jose.EncrypterOptions{Compression: jose.DEFLATE})
    if err != nil {
        return "", fmt.Errorf("failed to create JWE encrypter: %v", err)
    }
   
    argoClaims := &types.Claims{
        Claims: jwt.Claims{
            Issuer:  "argo-server",
            Subject: "foobar",
            Expiry:  jwt.NewNumericDate(time.Unix(int64(1714140108), 0)),
        },
        Groups:                []string{"00000000-0000-0000-0000-000000000000"},
        Email:                   "foobar",
        EmailVerified:           true,
        Name:                    "foobar",
        ServiceAccountName:      "foobar",
        PreferredUsername:       "foobar",
        ServiceAccountNamespace: "foobar",
    }
 
    // Encrypt the token
    serializedToken, err := jwt.Encrypted(encrypter).Claims(argoClaims).CompactSerialize()
   
    if err != nil {
        return "", fmt.Errorf("serialization failed: %v", err)
    }
 
    return serializedToken, nil
}
 
 
func main() {
    // Replace with your actual private key in PEM format
    privateKey := `
-----BEGIN RSA PRIVATE KEY-----
...
-----END RSA PRIVATE KEY-----`
 
    out_token, _ := EncryptJWTToken(privateKey)
    fmt.Printf("(De)Encrypted token: %s\n", out_token)
}

Logs from the workflow controller

time="2024-04-29T08:53:17.759Z" level=info duration="81.94µs" method=GET path=index.html size=473 status=0
time="2024-04-29T08:53:37.260Z" level=info msg="selected SSO RBAC service account for user" email= loginServiceAccount=ACCOUNT-lab-argoworkflows-admin serviceAccount=ACCOUNT-lab-argoworkflows-admin ssoDelegated=false ssoDelegationAllowed=false subject=SUBJECT
W0429 08:53:37.274001       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.276846       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.279561       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.286895       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.291901       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.294897       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.297572       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.300226       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.305100       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
W0429 08:53:37.317733       1 warnings.go:70] Use tokens from the TokenRequest API or manually created secret-based tokens instead of auto-generated secret-based tokens.
time="2024-04-29T08:53:37.318Z" level=info msg="finished unary call with code OK" grpc.code=OK grpc.method=CreateWorkflow grpc.service=workflow.WorkflowService grpc.start_time="2024-04-29T08:53:37Z" grpc.time_ms=59.361 span.kind=server system=grpc
time="2024-04-29T08:53:37.319Z" level=info duration=60.522135ms method=POST path=/api/v1/workflows/argo-workflows size=8958 status=0
time="2024-04-29T08:53:37.424Z" level=info msg="finished unary call with code Unauthenticated" error="rpc error: code = Unauthenticated desc = token not valid for running mode" grpc.code=Unauthenticated grpc.method=GetWorkflow grpc.service=workflow.WorkflowService grpc.start_time="2024-04-29T08:53:37Z" grpc.time_ms=0.051 span.kind=server system=grpc
time="2024-04-29T08:53:37.425Z" level=info duration="511.841µs" method=GET path=/api/v1/workflows/argo-workflows/subscription-orchestration-ccpsr size=56 status=401
time="2024-04-29T08:53:37.759Z" level=info duration="90.936µs" method=GET path=index.html size=473 status=0

Logs from in your workflow's wait container

time="2024-04-29T08:53:37.325Z" level=info msg="Processing workflow" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.363Z" level=warning msg="Non-transient error: configmaps \"artifact-repositories\" not found"
time="2024-04-29T08:53:37.363Z" level=info msg="resolved artifact repository" artifactRepositoryRef=default-artifact-repository
time="2024-04-29T08:53:37.363Z" level=info msg="Updated phase  -> Running" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.363Z" level=warning msg="Node was nil, will be initialized as type Skipped" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.364Z" level=info msg="was unable to obtain node for , letting display name to be nodeName" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.364Z" level=info msg="Steps node subscription-orchestration-ccpsr initialized Running" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.364Z" level=info msg="StepGroup node subscription-orchestration-ccpsr-325367412 initialized Running" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.364Z" level=warning msg="Node was nil, will be initialized as type Skipped" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.364Z" level=info msg="Pod node subscription-orchestration-ccpsr-2515602399 initialized Pending" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
W0429 08:53:37.398660       1 warnings.go:70] metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must be no more than 63 characters]
time="2024-04-29T08:53:37.399Z" level=info msg="Created pod: subscription-orchestration-ccpsr[0].create-transient-topic (subscription-orchestration-ccpsr-create-transient-topic-2515602399)" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.399Z" level=info msg="Workflow step group node subscription-orchestration-ccpsr-325367412 not yet completed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.399Z" level=info msg="TaskSet Reconciliation" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.399Z" level=info msg=reconcileAgentPod namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:37.399Z" level=info msg="Workflow to be dehydrated" Workflow Size=18813
time="2024-04-29T08:53:37.415Z" level=info msg="Workflow update successful" namespace=argo-workflows phase=Running resourceVersion=48810716 workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.403Z" level=info msg="Processing workflow" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.403Z" level=info msg="Task-result reconciliation" namespace=argo-workflows numObjs=0 workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.403Z" level=info msg="Pod failed: Error (exit code 1)" displayName=create-transient-topic namespace=argo-workflows pod=subscription-orchestration-ccpsr-create-transient-topic-2515602399 templateName=create-transient-topic workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.403Z" level=info msg="node changed" namespace=argo-workflows new.message="Error (exit code 1)" new.phase=Failed new.progress=0/1 nodeID=subscription-orchestration-ccpsr-2515602399 old.message= old.phase=Pending old.progress=0/1 workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Step group node subscription-orchestration-ccpsr-325367412 deemed failed: child 'subscription-orchestration-ccpsr-2515602399' failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr-325367412 phase Running -> Failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr-325367412 message: child 'subscription-orchestration-ccpsr-2515602399' failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr-325367412 finished: 2024-04-29 08:53:47.405160151 +0000 UTC" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="step group subscription-orchestration-ccpsr-325367412 was unsuccessful: child 'subscription-orchestration-ccpsr-2515602399' failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Outbound nodes of subscription-orchestration-ccpsr-2515602399 is [subscription-orchestration-ccpsr-2515602399]" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Outbound nodes of subscription-orchestration-ccpsr is [subscription-orchestration-ccpsr-2515602399]" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr phase Running -> Failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr message: child 'subscription-orchestration-ccpsr-2515602399' failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="node subscription-orchestration-ccpsr finished: 2024-04-29 08:53:47.405367337 +0000 UTC" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="TaskSet Reconciliation" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg=reconcileAgentPod namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Updated phase Running -> Failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Updated message  -> child 'subscription-orchestration-ccpsr-2515602399' failed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Marking workflow completed" namespace=argo-workflows workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.405Z" level=info msg="Workflow to be dehydrated" Workflow Size=19527
time="2024-04-29T08:53:47.411Z" level=info msg="cleaning up pod" action=deletePod key=argo-workflows/subscription-orchestration-ccpsr-1340600742-agent/deletePod
time="2024-04-29T08:53:47.420Z" level=info msg="Workflow update successful" namespace=argo-workflows phase=Failed resourceVersion=48810805 workflow=subscription-orchestration-ccpsr
time="2024-04-29T08:53:47.432Z" level=info msg="cleaning up pod" action=labelPodCompleted key=argo-workflows/subscription-orchestration-ccpsr-create-transient-topic-2515602399/labelPodCompleted
@agilgur5
Copy link
Member

agilgur5 commented Apr 29, 2024

Found this issue which proposes a solution I would solve my issue: #12049

Yes that solves a number of issues. I'd like to implement that too, but need a solid chunk of time to understand it in-depth, implement, and test. Argo also might still need to issue its own tokens as a fallback in some cases (I'm not sure yet).

TLDR; Argo SSO doesn't encode anything from the Azure token into its' own token, thus use of public key is enough to authenticate.

I need to take a closer look at the rest. If accurate, only needing the public key would certainly be problematic in a few ways. Although currently, as you stated, since Argo generates both in a k8s Secret there would be no new exploit route (would still be a vuln, but 0 impact vuln).

Argo does validate the OAuth2 token before exchanging for its own token. Bypassing that and crafting your own tokens with the private key would be fair game for a token exchange (although now I'm wondering if capability/authZ-wise there's potentially a MITM possibility in and of itself there? 🤔).

Btw, Argo does have a private security disclosure process. A bit late at this point though 😕

@kvedes
Copy link
Author

kvedes commented Apr 30, 2024

Sorry this is my first time reporting a security issue, so I hadn't thought about people misusing it. Feel free to take down the issue and keep it internally.

@agilgur5
Copy link
Member

agilgur5 commented May 4, 2024

Sorry this is my first time reporting a security issue, so I hadn't thought about people misusing it

For future reference, please use projects' SECURITY.md or other publicly posted security contacts. On GitHub, the UI links to it in a few places and it's also present in the /security route of any repo, for instance https://github.com/argoproj/argo-workflows/security.
For websites there's also security.txt, typically under /.well-known/security.txt, for instance https://www.google.com/.well-known/security.txt.

Feel free to take down the issue and keep it internally.

A little bit easier said than done, I believe this requires a GH support request 😕.
As this is already public, it's likely already been scraped by crawlers too, so taking it down is not quite so useful.


With regard to the actual contents of the issue:

The code only uses the public part of the sso key.

So your code uses the private key, not the public key. Could you elaborate why you think only the public key is necessary?

Bypassing that and crafting your own tokens with the private key would be fair game for a token exchange (although now I'm wondering if capability/authZ-wise there's potentially a MITM possibility in and of itself there? 🤔).

This token exchange does feel a little weak, but the only MITM is if someone has access to your Argo Server's private key, they can then potentially access everything that all users can access. That would normally require the same permissions as to modify the Argo Server's Deployment though, if not even more elevated (as it would require permissions to Secrets in Argo's namespace), so there's technically no privilege escalation route there. If you can modify the Server, you can turn off auth entirely

@kvedes
Copy link
Author

kvedes commented May 7, 2024

So the key stored in the Kubernetes secret sso contains both a private and a public key. My point is, that if you extract the public key, that is sufficient to create a token. This means that if at some point the key is split into a private and a public key, and the public part is exposed there will be a security hole.

My code example might be little misleading, due to my lack of Go skills. I use the "private key" in the example, which is the full key from Kubernetes which contains both the public and private key as mentioned above. The code then extracts the public part of the key:

encrypter, err := jose.NewEncrypter(jose.A256GCM, jose.Recipient{Algorithm: jose.RSA_OAEP_256, Key: privateKey.Public()}, &jose.EncrypterOptions{Compression: jose.DEFLATE})

Note the part that extracts the public key: Key: privateKey.Public()

I did manage to manually extract the public part of the key, but as mentioned my Go skills are limited so I'm not sure how to pass that in directly. The above code is based on the code from the sso module in Argo Workflows.

@agilgur5
Copy link
Member

agilgur5 commented May 8, 2024

Note the part that extracts the public key: Key: privateKey.Public()

Oh I missed that detail (it requires scrolling to the right also). I've edited your initial sample to clarify that in the comments.

I'll have to look into this more why it's accepting that 😕

@agilgur5 agilgur5 added the P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important label May 8, 2024
@agilgur5
Copy link
Member

I'll have to look into this more why it's accepting that 😕

Oh... the code straight up uses the public key to encrypt:

encrypter, err := jose.NewEncrypter(jose.A256GCM, jose.Recipient{Algorithm: jose.RSA_OAEP_256, Key: privateKey.Public()}, &jose.EncrypterOptions{Compression: jose.DEFLATE})

That seems to date back to #4095 and seems to have been on purpose, as the tokens are opaque. I'm not sure if that's necessary, but that would explain usage of the public key; only the private key can decrypt it.

Looking further, Argo having its own JWE apparently stems from #4035 / #3873 et al. Instead of implementing refresh tokens in that PR, it was decided to use a new JWE effectively as a token exchange. Although it's not entirely clear why that was decided.

I might be missing something, but that would suggest that #12049 was the original approach and then was changed?

@agilgur5
Copy link
Member

agilgur5 commented May 14, 2024

Although currently, as you stated, since Argo generates both in a k8s Secret there would be no new exploit route

Oh it doesn't generate both, it only generates the private key in the Secret. The public key is then derived from the private key if I'm understanding the code correctly.

That's a bit better, technically the public key is not stored anywhere.

This is effectively doing symmetric encryption but using asymmetric keys, which is confusing and odd. It would probably be more correct to just use symmetric encryption

@kvedes
Copy link
Author

kvedes commented May 15, 2024

It seems that crypto/rsa generates a private key that also contains a public key: rsa source code. See line 113-127.

As to your considerations about encryption options I'm not sure what the best option is. Just seems weird to me, that Argo WF issues its' own JWE tokens, instead of validating the SSO tokens from the provider.

@agilgur5
Copy link
Member

agilgur5 commented May 15, 2024

Just seems weird to me, that Argo WF issues its' own JWE tokens, instead of validating the SSO tokens from the provider.

Just to be 100% explicit, as I wrote before, Argo does validate the provider tokens and then exchanges those for its own longer-lived encrypted/opaque tokens. (confusingly asymmetric as symmetric)

Otherwise I agree that directly using the provider tokens would seem to make more sense (and have less indirection and therefore less room for error), and some SIG Security folks agreed (albeit only based on a short summary) during a meeting today as well.

I'm not sure if you saw my comment from earlier today -- apparently Argo did that in the past and there was support & progress to improve that, but then it got replaced with the current JWE approach without much documented explanation.

I'd like to make sure we have full context on that decision before changing that. Off-hand, I can imagine the JWE could be useful for some providers that don't support refresh tokens or have other limitations, but I would imagine that the vast majority are capable, and that Dex as an intermediary could mitigate various limitations too (if needed, similar to its current optional usage in Workflows). May check with some Intuit folks if they can find records of it etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sso-rbac P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important type/bug type/security Security related
Projects
None yet
Development

No branches or pull requests

2 participants