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

Support DCE IOV functions on macOS #258

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

jborean93
Copy link
Contributor

@jborean93 jborean93 commented Aug 3, 2021

The DCE IOV functions on macOS are not exported by any public header on
the GSS.Framework. This PR defines some macros in the C file that when
compiled against the GSS.Framework will alias the private symbols to the
ones expected by GSSAPI. While the symbols are considered to be private
they haven't changed across any macOS version since the introduction of
GSS.Framework and the inclusion of ext_dce is still dependent on whether
the symbol is present at compile time.

This allows users of this library on macOS to better interop with
Windows SSPI message encryption which typically require the IOV wrapping
functions that were previously unavailable.

This is round 2 from #210 but I'm a lot more happier with this approach as the macros are defined in the .c file for the sdist allowing this to work on macOS without requiring a wheel. It is also not relying on dlopen/dlsym at runtime to get the path to the GSS.Framework library which is a big plus.

@jborean93
Copy link
Contributor Author

jborean93 commented Aug 3, 2021

Oh and the change https://github.com/pythongssapi/python-gssapi/pull/258/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7R125 is unrelated but clang will emit warning saying this -framework ... is unused as compiler args. You only need this during the linking phase.

Copy link
Member

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

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

In general I don't see any problems with this approach. Some commentary inline.

gssapi/raw/ext_dce.pyx Outdated Show resolved Hide resolved
gssapi/raw/python_gssapi_ext.h Outdated Show resolved Hide resolved
gssapi/raw/python_gssapi_ext.h Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
gssapi/raw/ext_dce.pyx Show resolved Hide resolved
@frozencemetery
Copy link
Member

clang will emit warning saying this -framework ... is unused as compiler args. You only need this during the linking phase.

Makes sense. Should be mentioned in the commit description (unless you think it warrants its own commit).

@jborean93
Copy link
Contributor Author

Should be mentioned in the commit description

I'll amend the commit to include this.

@jborean93
Copy link
Contributor Author

Thanks @frozencemetery for the review, I've incorporated the changes and have manually tested again and things work just fine.

@jborean93 jborean93 force-pushed the macOS-IOV branch 2 times, most recently from 16411b9 to 7bdffb2 Compare August 5, 2021 21:02
The DCE IOV functions on macOS are not exported by any public header on
the GSS.Framework.  While the symbols are considered to be private they
haven't changed across any macOS version since the introduction of
GSS.Framework.  Since the inclusion of ext_dce is still dependent on
whether the symbol is present at compile time, their usage in
python-gssapi is relatively safe.  Provide our own copy of the
definitions accordingly.

The `-framework GSS.framework` has also been removed from the compiler
args as clang emits a warning with these present, and it is only needed
for linking.

Signed-off-by: Jordan Borean <jborean93@gmail.com>
[rharwood@redhat.com: C style, slight comment/commit message tweaks]
@frozencemetery frozencemetery merged commit 2bccab4 into pythongssapi:main Aug 6, 2021
@jborean93 jborean93 deleted the macOS-IOV branch August 6, 2021 18: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.

None yet

2 participants