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

Use GSSAPI for IntegratedSecurity on Unix #77

Merged
merged 7 commits into from
Aug 29, 2020

Conversation

dwink
Copy link
Contributor

@dwink dwink commented Aug 15, 2020

This PR leverages the libgssapi crate to provide IntegratedSecurity support via Kerberos on Unix.

At the TDS protocol level, all the SSPI tokens/packets/etc are used for this, too, so I turned off the cfg(windows) for all of that machinery, and added the libgssapi dependency as cfg(unix). From a dependency perspective, though, could be nice to make it a dependency folks can turn off. Happy to adjust.

Anyhow, have a look and let me know what else this needs, if anything -- tested in a ActiveDirectory-based Kerberos environment with SQL Server.

@pimeys
Copy link
Contributor

pimeys commented Aug 17, 2020

You might want to change the ADO.net connection string handling and documentation to support this new method. I think we can live with the IntegratedSecurity param, but change the docs so it supports Kerberos too on Unix and the behavior of the param handling...

https://docs.rs/tiberius/0.4.8/tiberius/struct.Config.html#method.from_ado_string

@pimeys
Copy link
Contributor

pimeys commented Aug 17, 2020

I think code-wise we could keep WindowsIntegrated for windows, but maybe consider renaming the new variant in the AuthMethod.

@dwink
Copy link
Contributor Author

dwink commented Aug 17, 2020

You might want to change the ADO.net connection string handling and documentation to support this new method. I think we can live with the IntegratedSecurity param, but change the docs so it supports Kerberos too on Unix and the behavior of the param handling...

https://docs.rs/tiberius/0.4.8/tiberius/struct.Config.html#method.from_ado_string

Ah, yes, totally missed that. Will do.

I think code-wise we could keep WindowsIntegrated for windows, but maybe consider renaming the new variant in the AuthMethod.

Not quite sure I follow; what do you think the variant should be? I think I could make WindowsIntegrated work on either cfg(windows) or cfg(unix) with flags, or keep it separate (as Integrated, or Gssapi or even UnixIntegrated)...

Copy link
Contributor

@esheppa esheppa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good and consistent with the rest of the codebase, appreciate you efforts @dwink. The implementation looks good as is, the only thing I'd like to see changed is the feature as a whole made optional so that only users who need Kerberos auth need to include the libgssapi dependency.

src/client/connection.rs Show resolved Hide resolved
src/client/connection.rs Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
dwink added 2 commits August 22, 2020 10:54
* Add integrated-auth feature
* Add trace!s where indicated
@pimeys
Copy link
Contributor

pimeys commented Aug 27, 2020

Back to work. Doing my review round now, and when happy, merging and releasing the next Tiberius.

Copy link
Contributor

@pimeys pimeys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to require a system library (on Linux at least) to compile the gssapi crate. I'm getting an error:

src/wrapper_mit.h:1:10: fatal error: 'gssapi.h' file not found
src/wrapper_mit.h:1:10: fatal error: 'gssapi.h' file not found, err: true
thread 'main' panicked at 'failed to generate gssapi bindings: ()', /home/pimeys/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/libgssapi-sys-0.2.1/build.rs:54:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I couldn't find documentation on this PR how to install the correct packages, so would be nice to have a bit more info for our users what is required to get a successful compilation if using the feature.

I'm thinking of what support for Kerberos would mean in Prisma. How I understood this, for the client and server versions to work properly together the crate needs to be compiled against the correct version of libgssapi C library. This would mean providing a pre-compiled Prisma library from a CI system including the right headers would not work correctly if the user would have a different version in their system?

To make this PR right, we should be explicitly documenting the whole procedure to make it as easy as possible for our users to understand how the feature works.

@dwink
Copy link
Contributor Author

dwink commented Aug 27, 2020

This seems to require a system library (on Linux at least) to compile the gssapi crate. I'm getting an error:

src/wrapper_mit.h:1:10: fatal error: 'gssapi.h' file not found
src/wrapper_mit.h:1:10: fatal error: 'gssapi.h' file not found, err: true
thread 'main' panicked at 'failed to generate gssapi bindings: ()', /home/pimeys/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/libgssapi-sys-0.2.1/build.rs:54:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I couldn't find documentation on this PR how to install the correct packages, so would be nice to have a bit more info for our users what is required to get a successful compilation if using the feature.

Sure, I'll update the README with some more details.

I'm thinking of what support for Kerberos would mean in Prisma. How I understood this, for the client and server versions to work properly together the crate needs to be compiled against the correct version of libgssapi C library. This would mean providing a pre-compiled Prisma library from a CI system including the right headers would not work correctly if the user would have a different version in their system?

To make this PR right, we should be explicitly documenting the whole procedure to make it as easy as possible for our users to understand how the feature works.

The GSSAPI interface has been stable for more than a decade, so I wouldn't expect there to be too many breaking changes in the interface. Also, this feature does require some setup in the environment:

In practice, organizations that would want to enable this feature for a system like Prisma would probably already have done this (as my organization has; I didn't think to document this because it Just Worked), as this is all the fundamental steps you need in order to join a Unix host to an Active Directory domain.

dwink and others added 2 commits August 27, 2020 10:13
- Do not enable `integrated-auth-gssapi` for docs.rs (they probably miss
the krb headers).
- Combine `AuthMethod::Integrated` and `AuthMethod::WindowsIntegrated`
to `AuthMethod::Integrated` variant for clarity.
- Make the docs a bit more clear (hopefully)
@pimeys
Copy link
Contributor

pimeys commented Aug 28, 2020

Pushed a few changes as a separate PR that would fix a few things. After these (or at least after a discussion about the changes) we're ready to go!

dwink#1

@pimeys pimeys merged commit 8e5f44f into prisma:master Aug 29, 2020
@pimeys
Copy link
Contributor

pimeys commented Aug 29, 2020

Thank you for this! I'll cut a new release today.

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.

3 participants