-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Extract Kerberos functionality from SQL Server engine #51352
base: master
Are you sure you want to change the base?
Conversation
|
||
type clientProvider struct { | ||
AuthClient windows.AuthInterface | ||
DataDir string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had some offline discussion on the cache dir. we can look into that afterwards. Here are just some notes but they should not block this PR:
kinit
does refresh the output every connection, but there could be a race that two connections trying to output to the same file then read it.
On the other hand, it seems credentials.LoadCCache
finishes reading that file immediately and has no use of it afterwards. We could experiment if we could just use tmp dir instead of data-dir.
Another consideration is if we want to add real caching to avoid kinit on every connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another possibility is to reimplement the the protocol internally, without shelling out to kinit
. Not only we would drop the external dependency and the need to juggle various files. As @gabrielcorado points out, we would also benefit from much better control over the whole process and much better diagnostic information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference here is the outstanding issue in gokrb5
:
The RFC in question is not super long either: https://datatracker.ietf.org/doc/html/rfc4556.html
Gentle ping @gabrielcorado @probakowski |
Changes:
kinit
intodb/common
kerberos.ClientProvider
.kerberos.ClientProvider
I've tried very hard to keep this change minimal. There is a number of things I'd like to change in the current code, but those changes can happen in future PRs while keeping this one as small as possible.
This is a prerequisite to using Kerberos in other db engines, in particular Oracle.