Replies: 10 comments 8 replies
-
|
I managed to trace this first issue back from 'stat.h' to 'types.h' to reveal something missing from 'ngx_win32_config.h': In 'types.h', the _OFF_T_DEFINED section is not defining the '_off_t' type... ...because the 'ngx_win32_config.h' header was only including 'off_t' and not '_off_t' used by the 'stat.h' to support 32-bit and 64-bit together: This leaves me with the next issue: |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
After reviewing this area of code in the module: The article link on Variadic Macros explains it well. The solution is to modify the code with: |
Beta Was this translation helpful? Give feedback.
-
|
Now the third issue after fixing the earlier two issues in the module build: The easiest way to solve this is to change the variable name passed to add an '_t' to it so it don't conflict with the static global variable with the same name. Change the variables in scope to only change the parameter and not conflict as hiding the static global. I don't know the impact of this on the code since most other methods are updating the static only. |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
Now on to the fourth error in the build: Here's the lines of code: It appears to be coming from PCRE2: My guess is to define an #ifdef and then use /tmp or the Windows Temp Folder as needed. |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
I fixed the 4th issue by defining retrieval of the Windows Temp Path to use and only allocate again if not already defined the first time. |
Beta Was this translation helpful? Give feedback.
-
|
Now the fifth build issue found: I added the following code snippet to cover for Windows builds: |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
Now for the 6th issue with the build: |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
I added a guard check on 'kcontext' in case it is NULL... However, it seems I may need to rewrite the entire code to make this work with Windows. |
Beta Was this translation helpful? Give feedback.
-
|
The 7th issue...(sigh, I am trying). Similar issue to the 6th one. Just needs a guard check around kcontext. |
Beta Was this translation helpful? Give feedback.
3 replies
-
|
Modified if condition to add 'kcontext' check: |
Beta Was this translation helpful? Give feedback.
-
|
8th Build Issue (similar to 6th and 7th with kcontext possibly undefined code path): |
Beta Was this translation helpful? Give feedback.
-
|
Line 776: kcontext wasn't initialized to NULL or to anything. If not NULL, it should be at least initialized to some default. |
Beta Was this translation helpful? Give feedback.
-
|
You are compiling with /w4, which has absurd semantics for warnings on
Windows and ignores standard clang/gcc suppression using additional parens.
Don’t do that and you will have fewer issues.
As written, your patches won’t be accepted.
…On Thu 15 Dec 2022 at 00:01, justdan23 ***@***.***> wrote:
I added a guard check on 'kcontext' in case it is NULL...
if( kcontext != NULL )
{
if ((kerr = krb5_cc_initialize(kcontext, ccache, principal))) {
spnego_log_error("Kerberos error: Cannot initialize ccache");
spnego_log_krb5_error(kcontext, kerr);
return kerr;
}
if ((kerr = krb5_cc_store_cred(kcontext, ccache, &creds))) {
spnego_log_error("Kerberos error: Cannot store credentials");
spnego_log_krb5_error(kcontext, kerr);
return kerr;
}
}
else
{
spnego_log_error("Kerberos error: kcontext is NULL!")
}
However, it seems I may need to rewrite the entire code to make this work
with Windows.
—
Reply to this email directly, view it on GitHub
<#135 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADGOATSAEY7734EDGKKUDLWNJNV7ANCNFSM6AAAAAAS67MGKA>
.
You are receiving this because you are subscribed to this thread.Message
ID:
<stnoonan/spnego-http-auth-nginx-module/repo-discussions/135/comments/4405000
@github.com>
|
Beta Was this translation helpful? Give feedback.
1 reply
-
|
These are the flags which NGINX is passing through it's configuration. I'm trying to help make the source resolve protections since this is a very sensitive area of code targeted for zero day attacks. These turn out to be legitimate bugs in the code. The Author of NGINX told me this past week to only use NMAKE and MSVC and not GCC at all as they do not support GCC for any WINDOWS builds. Since we have to build it without GCC specifics working, are you saying you can't patch the code to support WIN32 or _WIN64 builds? |
Beta Was this translation helpful? Give feedback.
-
|
Discovered another bug (Number 9) in the code which would never allow the execution of the following method because 'cccache' should be 'ccache': Propose fixing the mispelled variable to 'ccache' so this will properly close the connection and cleanup memory allocated. |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
I'm getting closer to resolving the inline if condition statements which initialize variables. This is a security risk to leave this defined this way as hackers can override the if condition at the assembly level. From a security perspective, I propose updating the code to remove the inline variable assigning within if conditionals. You did this in most of the code, so it is just some cleanup which is needed to fix this. |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
I fixed all compiler issues and now all code compiles cleanly using Visual Studio 2022 with Windows 11 SDK as x64 (64-bit only). Since the author doesn't want any of these fixes, I will be planning to publish a formal article on these issues to help the next person who wants to use this module in some capacity. Our security staff will be running scans against this code to also find any vulnerabilities to follow up on reporting. Next issues are the linking of the krb5 libraries which are not mentioned in the documentation. Is there updated docs somewhere which help the developer with the NGINX build? |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
Hello, did you manage to build nginx.exe , I'm having the same problems right now |
Beta Was this translation helpful? Give feedback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
I have successfully built NGINX using Visual Studio 2022 NMAKE/CL and MSYS2 (for Configure to generate x64 Makefiles for Windows). I'm able to build all of the stock NGINX with OpenSSL, PCRE2, and Zip entirely with x64 for Windows. I'm using the following to setup the dev environment for MSVC:
Note: The windows readme docs for 'krb5' only indicate x86 or amd64. Has anyone tried with x64 for Intel CPUs?
However, after I built the 'krb5' SDK for Windows to create the SDK folder containing the gssapi/gssapi.h header file, when compiling this SPNEGO NGINX Module from 'master', I am running into an issue with the includes not properly picking up the definitions which should appear if the DEFINEs are in place for MINGW. It's as if it is thinking it is GCC when reaching this module's source code for inclusion in NGINX.exe?
Here's the GitHub repo I used for building the 'krb5' SDK:
https://github.com/krb5/krb5I used the following steps to build the SDK after cloning it:
Thoughts?
Here's the output showing all compiler parameters passed by the NGINX nmake build of this module:
Beta Was this translation helpful? Give feedback.
All reactions