pan-globalprotect-okta icon indicating copy to clipboard operation
pan-globalprotect-okta copied to clipboard

Update to work with new fido2 version

Open keis opened this issue 2 years ago • 15 comments

Not working yet and I don't know enough about okta to figure it out easily

keis avatar Jan 27 '22 09:01 keis

Seems to be the interface changes to get_assertion in upstream python-fido2 version 0.8.0 and 0.9.0 https://github.com/Yubico/python-fido2/blob/58471d4af1c09cc9fe316771d6203b4844cbc668/NEWS

Do you want to continue to work on a PR? I can carry on as well, if you like, but I am also happy to just peer-review it and leave that PR to you.

coldcoff avatar Jan 27 '22 09:01 coldcoff

With these changes it should be up to date with that api change I think but something is still not working.

I'll probably poke at it some more between meetings and what not, but feel free to have a go at it.

keis avatar Jan 27 '22 09:01 keis

Thanks, I'll take my go in the evening. Not good to play with the VPN in the middle of my working day :-)

coldcoff avatar Jan 27 '22 10:01 coldcoff

I currently don't have my hands on Fido2 to test and help, but I could review the PR, when it's done. If I get my hands on one, does it need some additional config on OKTA side?

arthepsy avatar Jan 27 '22 10:01 arthepsy

I played without success several hours now :-(

I think your change is good and does the right thing to upgrade python-fido2 from 0.7.0 to 0.9.3

But: it doesn't work for me finally, either.

2 minor remarks:

  • From reading the examples in https://github.com/Yubico/python-fido2 I think it is "better" to use camelCase here:

    result = client.get_assertion({
    'challenge': challenge.encode('utf-8'),
    'rpId': purl[1],
    'allowCredentials': allow_list
    })
    

    ... but that should not matter as there seems to be code to accept both spellings.

  • Additionally, also from reading the examples, I think the result handling looks better as:

    esponse = result.get_response(0) # only one cred in allowList, so only one response.
    ata = {
    'stateToken': state_token,
    'clientData': to_n((base64.b64encode(response.client_data)).decode('ascii')),
    'signatureData': to_n((base64.b64encode(response.signature)).decode('ascii')),
    'authenticatorData': to_n((base64.b64encode(response.authenticator_data)).decode('ascii'))
    
    

    (this saves accessing the "private" assertion._client_data in your code and uses the new result object)

... but that's all only cosmetics it seems. Main issue is that it does not work anymore. Those fido2 steps all seem to succeed. We get POST data for Okta without issues and they look at least sensible. But when I post the result to Okta API they respond with a 503 and an error with something like:

{"errorCode":"E0000068","errorSummary":"Invalid Passcode/Answer","errorLink":"E0000068","errorId":"oaesNxKuS33RFeyOwaJXNyJ1w","errorCauses":[{"errorSummary":"Invalid nonce."}]}

I assume it's the same picture on your side when you say "not working yet"?

Probably we need to cut the fido code out to some dedicated test program and try to get some stable input (might be easy) and output (might be hard due to the use case...) to see what's the difference in detail when changing 0.7.0 <-> 0.9.3.

I'm reverting to 0.7.0 for the time being, need to show up at work :-)

Let's see I'll find some time to continue playing over the weekend. At least the fix it not urgent, we have some time. But we should make it work again, with recent python-fido2.

coldcoff avatar Jan 28 '22 09:01 coldcoff

yes, that's the same thing I'm seeing there's some activity and the fido2 calls as far as I can tell are working but then I get that "Invalid Passcode/Answer" from OKTA.

keis avatar Jan 28 '22 09:01 keis

Got it!!!!! (I peeked a bit at https://github.com/Nike-Inc/gimme-aws-creds)

Here is my diff against master:

diff --git a/gp-okta.py b/gp-okta.py
index 24d4662..3907884 100755
--- a/gp-okta.py
+++ b/gp-okta.py
@@ -8,7 +8,8 @@
    Copyright (C) 2019 Aaron Lindsay ([email protected])
    Copyright (C) 2019 Taylor Dean ([email protected])
    Copyright (C) 2020 Max Lanin ([email protected])
-   Copyright (C) 2019-2020 Tino Lange ([email protected])
+   Copyright (C) 2019-2022 Tino Lange ([email protected])
+   Copyright (C) 2022 David Keijser ([email protected])
 
    Permission is hereby granted, free of charge, to any person obtaining a copy
    of this software and associated documentation files (the "Software"), to deal
@@ -56,6 +57,7 @@ try:
 	from fido2.utils import websafe_decode
 	from fido2.hid import CtapHidDevice
 	from fido2.client import Fido2Client
+	from fido2.webauthn import PublicKeyCredentialRequestOptions, PublicKeyCredentialDescriptor, PublicKeyCredentialType
 	have_fido = True
 except ImportError:
 	pass
@@ -120,13 +122,13 @@ def warn(s):
 		print(u'[WARN] {0}'.format(s))
 
 def dbg(d, h, *xs):
-	# type: (Any, str, Union[str, List[str], Dict[str, Any]]) -> None
+	# type: (Any, str, Union[str, List[str], Tuple[str], Dict[str, Any]]) -> None
 	if quiet:
 		return
 	if not d:
 		return
 	for x in xs:
-		if not isinstance(x, dict) and not isinstance(x, list):
+		if not isinstance(x, dict, list, tuple):
 			for line in x.split('\n'):
 				print(u'[DEBUG] {0}: {1}'.format(h, line))
 		else:
@@ -649,27 +651,30 @@ def okta_mfa_webauthn(conf, factor, state_token):
 	profile = rfactor['profile']
 	purl = parse_url(conf.okta_url)
 	origin = '{0}://{1}'.format(purl[0], purl[1])
-	challenge = rfactor['_embedded']['challenge']['challenge']
-	credentialId = websafe_decode(profile['credentialId'])
-	allow_list = [{'type': 'public-key', 'id': credentialId}]
+	request_options = PublicKeyCredentialRequestOptions(
+		challenge = websafe_decode(rfactor['_embedded']['challenge']['challenge']),
+		rp_id = purl[1],
+		allow_credentials = [PublicKeyCredentialDescriptor(PublicKeyCredentialType.PUBLIC_KEY,
+			websafe_decode(profile['credentialId']))]
+	)
 	for dev in devices:
 		client = Fido2Client(dev, origin)
 		print('!!! Touch the flashing U2F device to authenticate... !!!')
 		try:
-			result = client.get_assertion(purl[1], challenge, allow_list)
-			dbg(conf.debug, 'assertion.result', result)
+			result = client.get_assertion(request_options)
+			dbg(conf.debug, 'assertion.result', vars(result))
 			break
 		except Exception:
 			traceback.print_exc(file=sys.stderr)
 			result = None
 	if not result:
 		return None
-	assertion, client_data = result[0][0], result[1] # only one cred in allowList, so only one response.
+	response = result.get_response(0) # only one cred in allow_credentials, so only one response.
 	data = {
 		'stateToken': state_token,
-		'clientData': to_n((base64.b64encode(client_data)).decode('ascii')),
-		'signatureData': to_n((base64.b64encode(assertion.signature)).decode('ascii')),
-		'authenticatorData': to_n((base64.b64encode(assertion.auth_data)).decode('ascii'))
+		'clientData': to_n((base64.b64encode(response.client_data)).decode('ascii')),
+		'signatureData': to_n((base64.b64encode(response.signature)).decode('ascii')),
+		'authenticatorData': to_n((base64.b64encode(response.authenticator_data)).decode('ascii'))
 	}
 	log('mfa {0} signature request [okta_url]'.format(provider))
 	_, _h, j = send_json_req(conf, 'okta', 'uf2 mfa signature', j['_links']['next']['href'], data, expected_url=conf.okta_url)

coldcoff avatar Jan 28 '22 10:01 coldcoff

w00t that's awesome!

Want me to update this PR with that, or perhaps it's easier for you to open a new one instead?

keis avatar Jan 28 '22 11:01 keis

Let's not waste PRs and forks ...

Please integrate in your branch, test & adapt the PR yourself (maybe you have something more to add/change?) [also I did not test if it still works on py2, btw] -- and get @arthepsy merge that ;-)

Besides: LGTM from my side. Thanks for bringing that up!

coldcoff avatar Jan 28 '22 11:01 coldcoff

Tested with both python2.7 and 3.9

keis avatar Jan 28 '22 13:01 keis

@arthepsy -- IMHO this can be merged. I'm using that new code every day now, works!

coldcoff avatar Feb 01 '22 09:02 coldcoff

bump :)

keis avatar Feb 10 '22 11:02 keis

@arthepsy -- this can be merged. works like a charm. any remaining objections?

coldcoff avatar Mar 14 '22 08:03 coldcoff

Ping?

coldcoff avatar Mar 30 '22 07:03 coldcoff

@keis perhaps you could consider adding some of the changes from https://github.com/arthepsy/pan-globalprotect-okta/pull/35 in to this pr as well.. pin support, automatically selecting the right key if you have multiple key in the computer etc..

eedgar avatar Apr 05 '22 19:04 eedgar