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/add OIDC #31

Closed

Conversation

potto007
Copy link
Contributor

@potto007 potto007 commented Apr 24, 2023

This is still a work in progress....

@potto007
Copy link
Contributor Author

This contribution adds the ability to perform OIDC auth with ssh and scp and integrates with the external JWT capabilities now present in OpenZiti.

It adds several flags to the CLI, most notably, --oidc . There is also a new subcommand, auth which just tests the OIDC auth flow and then exits.

@dovholuknf
Copy link
Member

gimme a while to work through this. I want to dig in and understand what you did and how but i have a few other things on my plate right now. hoping to get to this this week, just wanted to give you a heads up. thanks for the PR!

@potto007 potto007 force-pushed the feature/add-oidc branch from 8aaef9b to 37c5561 Compare May 4, 2023 18:19
@dovholuknf
Copy link
Member

I'm watching the sdk-golang pr and I'll come back to this once that PR is sorted. Just letting you know, I'm still here and watching :)

@potto007 potto007 force-pushed the feature/add-oidc branch from d642556 to 47c7514 Compare May 10, 2023 16:03
@potto007
Copy link
Contributor Author

@dovholuknf The sdk-golang PR has been merged and tagged, along edge. The only thing still in a temporary state is the need for "replace" to be used for ziti until the next release of ziti is cut.

@dovholuknf
Copy link
Member

Very, very cool stuff @potto007 !!! I put out a fun teaser video over at https://www.youtube.com/watch?v=NZJtzSoS_g0

I think I'll have a few updates/changes to make but I was successful!

@dovholuknf
Copy link
Member

I'm still keeping an eye on this. Still have other things in front of this though. just keeping you informed. ;)

@dovholuknf
Copy link
Member

Just the "every now and again reminder" that I've not forgotten about this, just haven't had the time to get to it. in the current form it's not generic enough (unless it's changed?) It was requiring oidc for secondary auth. I'd like to see it allowed for primary auth as well. i'll get back to this eventually, just have other things in front of it as usual. if you wanted to make it more generic, allow for primary or secondary auth etc, great. otherwise, it'll be here until one of us can pick it up. regardless, thanks again for the PR. :) Still looking forward to showing something on a ziti tv when you're ready

@lshahar
Copy link

lshahar commented Apr 26, 2024

Any estimation for merging this function? even to get this as alpha function and improve it later sounds great.

@dovholuknf
Copy link
Member

I'm ashamed to admit that I haven't been able to get back to this... Community demand, like your upvote, definitely helps drive priorities!

@TetrusP
Copy link

TetrusP commented Jun 5, 2024

Hi, any update on having this merged? CC @dovholuknf @potto007

What zssh config file are you supposed to use for un-enrolled identities. I managed to get the main zssh to work with a basic identity but when I configured the ext-jwt-signer to use the modified zssh client written by potto007 I get a SEG error

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x20 pc=0x1053f28e8]

P.S. The OIDC flow works, I get a JWT from a Keycloak, the panic seems to happen after this function is called

NFO username set to: ubuntu
INFO targetIdentity set to: zssh-client-oidc-2
INFO OIDC auth flow succeeded

func EstablishClient(f SshFlags, userName, targetIdentity, token string) *ssh.Client {
	conf := getConfig(f.ZConfig)
	print(token)
	ctx, err := ziti.NewContext(conf)
	conf.Credentials.AddJWT(token) <-- Debugger goes to panic after this line
	if err != nil {
		logrus.Fatalf("error creating ziti context: %v", err)
	}

@TetrusP
Copy link

TetrusP commented Jun 5, 2024

So I found what's causing the SIGSEV, it was the AddJWT function. I am not sure why but I reverted the code to manually create the authorization header in the EstabClient function and it doesn't crash anymore so that's a promising sign

image

@dovholuknf
Copy link
Member

I had started a branch based on this work that was a bit more generic. another person found the youtube video and reported that it wasn't working for them too. I will revisit this, sometime this week, since there's a bit of demand for it.

@potto007
Copy link
Contributor Author

potto007 commented Jun 6, 2024 via email

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

Thanks guys! im looking to get this working with my Keycloak instance for a POC. Now that the client is not crashing I just need to finalize the flow to Keycloak. Currently getting a failure to create Dial Session to my ziti service. I think all my issues are server side now.

If you have time to answer one more question, what needs to be in the ~/.ziti/zssh.json file on the client? Is it the server jwt? When I created the server/client identities I made sure my client used the --external-id flag with my ext-jwt-signer but im not sure if that was correct.

In any case this is really cool tech and im eager to learn more lol

@potto007
Copy link
Contributor Author

potto007 commented Jun 6, 2024

Hey @TetrusP - it should write the server's CA in the zssh.json file upon successful initial setup of the OIDC flow when you use the OIDCFlag.

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

Interesting, I did not know that!

Following the guide with setting up the dialer and binder I am now receiving a new error -

error dialing SSH Conn: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain

I believe I am close!

@dovholuknf
Copy link
Member

I believe I am close!

That actually looks like you fully succeeded and contacted ssh. Did you provide a path to the key with -i? -i, --SshKeyPath string Path to ssh key. default: $HOME/.ssh/id_rsa

that looks to me like you made it to the ssh server and it rejected your auth request. maybe try adding -d, --debug pass to enable additional debug information if needed?

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

I believe I am close!

That actually looks like you fully succeeded and contacted ssh. Did you provide a path to the key with -i? -i, --SshKeyPath string Path to ssh key. default: $HOME/.ssh/id_rsa

that looks to me like you made it to the ssh server and it rejected your auth request. maybe try adding -d, --debug pass to enable additional debug information if needed?

Wow that was it, completely forgot to feed it the key. I had been staring at my screen for so long i didn’t notice.

The full flow is working! Im wondering now if i can get a PAM keycloak module hooked into the server so i dont need to provide an ssh key

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

Thanks for everyones help!!!

@dovholuknf
Copy link
Member

now if i can get a PAM keycloak module hooked into the server so i dont need to provide an ssh key

If you figure this out -- let's do a Ziti TV? That sounds super neat! :) (are you familiar with Ziti TV?)

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

Sure if I figure that out I'll circle back haha

@dovholuknf
Copy link
Member

I've been able to resurrect my branch with minor changes here/there. i'll try to fix the panic you saw too, a better error should be presented insteead.

@TetrusP did you happen to create an ext-jwt-signer, auth-policy and assign those to your identity? I tried that tonight but ended up with a different panic I'll try to diagnose at some point this week with help from Andrew.

Assuming I get through it, I think I'll revisit this on Ziti TV and end up merging this soon.

@TetrusP
Copy link

TetrusP commented Jun 6, 2024

I've been able to resurrect my branch with minor changes here/there. i'll try to fix the panic you saw too, a better error should be presented insteead.

@TetrusP did you happen to create an ext-jwt-signer, auth-policy and assign those to your identity? I tried that tonight but ended up with a different panic I'll try to diagnose at some point this week with help from Andrew.

Assuming I get through it, I think I'll revisit this on Ziti TV and end up merging this soon.

Hey, yea i did use the ext-jwt-signer and updated the default auth policy to use the id of the ext-jwt-signer, (i set my dialer/binder zssh identities to use the default auth policy)

one thing i noticed the ziti controller was throwing some errors on journald regarding the KID in the JWT not being able to get validated or something. The ssh still went through though with the keyclock oidc login flow

@dovholuknf
Copy link
Member

FYI, I've not forgotten this PR even though it's been open forever (i know)... Sorry. It's still on my list to get to, and I will eventually.

@TetrusP
Copy link

TetrusP commented Jul 9, 2024

FYI, I've not forgotten this PR even though it's been open forever (i know)... Sorry. It's still on my list to get to, and I will eventually.

No worries! I’ve been busy with other things as well. I’ll be revisiting this soon though! Cheers

@dovholuknf
Copy link
Member

i have branched from the original potto007 branch and refined this further. new PR will be here. should be merged soon. we had some recent updates that have caused some bugs that needed to be sorted.

@dovholuknf dovholuknf closed this Jul 25, 2024
@dovholuknf
Copy link
Member

new PR here: #35

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 this pull request may close these issues.

4 participants