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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
libstore: Enable kerberos negotiation for http binary caches #10568
base: master
Are you sure you want to change the base?
Conversation
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.
Could you add a test for this?
I think this needs to be a NixOS test, because that makes it easy to set up the necessary services, and doesn't add dependencies to the nix package.
We don't have a NixOS test for binary caches, so you could add a new one under tests/nixos
.
const Setting<bool> negotiate{this, false, "negotiate", | ||
"Whether to do kerberos negotiate when talking to the http binary cache."}; |
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.
Does this need to be a configuration item, or could it be autodetected?
I think negotiate
is a bit vague. Would kerberos
be a better name?
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 could add CURLAUTH_ANY
instead of CURLAUTH_NEGOITATE
, which would do the detection for you. We could add that flag at all times, and libcurl will use the best available authentication it finds. This is what git does, and that works pretty well. The downside is that if the HTTP server will accept negotiation OR another protocol, libcurl will always choose negotiation, which has a very slim chance of breaking use cases. This may be a very acceptable risk as git has been doing this for a decade already.
negotiate
is the best name here, it matches the curl command line argument --negotiate
, and is widely known as that.
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.
I'm not too familiar with that, but it's generally good to mirror existing interfaces.
What about CURLAUTH_ANYSAFE
?
Maybe we should similarly expose a single auth
setting, instead of specific ones for each possible solution.
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.
A decent idea, I've created #10584 as an alternate proposal to this one.
Testing kerberos/negotiation auth is notoriously hard. It requires setting up a KDC, generating service keytabs, users keytabs and configuring servers to do it. In the end we are testing a single option to libcurl, of which libcurl is very well tested. Creating a nixos test for this would take more time than I can devote to this feature. |
NixOS does come with |
Motivation
It would be nice to be able to do kerberos negotiation when fetching or pushing stuff to an HTTP binary store. This PR makes it possible to add
?negotiate=true
to have libcurl do krb negotiation.I debated on just adding
CURLAUTH_ANY
unconditionally here, which is what git does and would automatically do krb negotiation if the server supports it, but figured that might be a too dramatic change as that could change existing behavior.Priorities and Process
Add 馃憤 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.