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

Windows vcbuild is missing FIPS support, like ./configure --openssl-fips #14115

Closed
sam-github opened this issue Jul 6, 2017 · 24 comments
Closed
Labels
build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.

Comments

@sam-github
Copy link
Contributor

  • Version: all
  • Platform: Windows
  • Subsystem: crypto/tls

node can be linked against a FIPS cannister with ./configure --openssl-fips=..., but vcbuild.bat has no equivalent option.

@sam-github
Copy link
Contributor Author

@refack I've started digging into this, trying to figure out how FIPS builds work on any platform.

Is the vcbuild.bat even worth modifying? Weren't you rewriting it into PowerShell, or something?

@refack
Copy link
Contributor

refack commented Jul 6, 2017

@refack I've started digging into this, trying to figure out how FIPS builds work on any platform.

Is the vcbuild.bat even worth modifying? Weren't you rewriting it into PowerShell, or something?

It's a really boring process, but surreptitiously we've declared an impromptu "Windows ❤️ " hackathon for this weekend #4603 (comment), hopefully I'll wrap up the powershell goop.
Ref: #12310

tl;dr find out what comes after the = and I'll follow up.

@mscdex mscdex added build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform. labels Jul 6, 2017
@sam-github
Copy link
Contributor Author

Does anybody know (@refack?) whether the LINK properety set at https://github.com/nodejs/node/blob/master/configure#L989 and eventually showing up in config_fips.gypi as something like

# Do not edit. Generated by the configure script.                                
  { 'make_global_settings': [ [ 'LINK',                                            
~ '/home/sam/w/core/tls-node/deps/openssl/fips/fipsld <(openssl_fips)/bin/fipsld']]}

will also have an effect on Windows .sln files? Its not clear to me whether its possible to replace the Windows linker with a different command using gyp.

@sam-github
Copy link
Contributor Author

@joaocgreis maybe you know something of this? I'm trying to convince the sln files (via gyp) to do what is described in section 5.3.2 of https://www.openssl.org/docs/fips/UserGuide-2.0.pdf, which seems to be covered by defining the LINK property for Makefile based builds, but I'm having trouble seeing if it does anying on sln based builds.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jul 10, 2017

@sam-github I don't think it does. Linking is done by msbuild. Maybe you can persuade GYP to invoke it with msbuild /t:Link=/path/to/fipsld but caveat emptor, I'm not even sure that incantation works.

The ninja generator lets you override it through { 'make_global_settings': { 'LD': 'fipsld' } } (edit: because it calls link.exe directly) and works on Windows. Maybe that's a good alternative?

(LD is not a typo, the ninja generator doesn't call it LINK for some reason.)

@sam-github
Copy link
Contributor Author

@bnoordhuis thanks! I'll try ninja, I'd just convinced myself that LINK that was ignored by msbuild, and am glad to have some confirmation I'm not doing something wrong.

@sam-github
Copy link
Contributor Author

Notes for those coming after...

[2017-07-10 14:43] <refack> https://github.com/nodejs/node/pull/12632
[2017-07-10 14:44] <refack> and you need to run `python configure  --dest-cpu=x64 --tag= --ninja`

@refack
Copy link
Contributor

refack commented Jul 11, 2017

I'm trying to convince the sln files (via gyp) to do what is described in section 5.3.2 of https://www.openssl.org/docs/fips/UserGuide-2.0.pdf,

"Holy ___________, Batman!"
Well your best course of action will be to "bypass" GYP's regular "rules", and use custom actions:

@sam-github
Copy link
Contributor Author

sam-github commented Jul 13, 2017

The ninja builds fail for linux with FIPS, they need to define ld = ...fipsld... in build.ninja (easy, see below), but they also need to define ldxx = ... the same thing. How do I convince gyp to do this?

diff --git a/configure b/configure
index e0369f3..be9166d 100755
--- a/configure
+++ b/configure
@@ -987,6 +987,8 @@ def configure_openssl(o):
     fips_ld = os.path.abspath(os.path.join(fips_dir, 'fipsld'))
     o['make_fips_settings'] = [
       ['LINK', fips_ld + ' <(openssl_fips)/bin/fipsld'],
+      ['LD', fips_ld + ' <(openssl_fips)/bin/fipsld'],
+      ['LDXX', fips_ld + ' <(openssl_fips)/bin/fipsld'],
     ]
   else:
     o['variables']['openssl_fips'] = ''

I tested by manually hacking up the build.ninja to do ldxx = $ld and I can do fips builds.

@bnoordhuis
Copy link
Member

Doesn't LINK work? I thought it let you override the C++ linker (on Linux, not on Windows.)

Does this patch make a difference?

diff --git a/tools/gyp/pylib/gyp/generator/ninja.py b/tools/gyp/pylib/gyp/generator/ninja.py
index 1f67c94..91082c9 100644
--- a/tools/gyp/pylib/gyp/generator/ninja.py
+++ b/tools/gyp/pylib/gyp/generator/ninja.py
@@ -1931,6 +1931,10 @@ def GenerateOutputForConfig(target_list, target_dicts, data, params,
       ld = os.path.join(build_to_root, value)
     if key == 'LD.host':
       ld_host = os.path.join(build_to_root, value)
+    if key == 'LINK':
+      ldxx = os.path.join(build_to_root, value)
+    if key == 'LINK.host':
+      ldxx_host = os.path.join(build_to_root, value)
     if key == 'NM':
       nm = os.path.join(build_to_root, value)
     if key == 'NM.host':

@sam-github
Copy link
Contributor Author

@refack any idea why the configure script would be dying with KeyError: 'Debug_x64' from msvs_emultation.py, line 422? (sorry, I'm trying to figure out how to get text of my remote windows VM, its not going well, so no stack trace yet). Generating ninja build files worked for a while, but no longer. I reopened the cmd shell, trying both the "msbuild command prompt for vs2015" and the "developer command prompt for vs2015" (what's the difference?), but neither worked.

@refack
Copy link
Contributor

refack commented Jul 17, 2017

It's a GYP bug, when doing cross-compilation (x64 on a x32 host or the reverse). Make sure you have this patch in https://chromium-review.googlesource.com/c/482580/13/pylib/gyp/msvs_emulation.py#315

@refack
Copy link
Contributor

refack commented Jul 17, 2017

P.S.

  • "msbuild command prompt for vs2015" makes sure MSBuild is setup for .NET compilation (AFAICT cl.exe is not in the PATH)
  • "developer command prompt for vs2015" Makes sure all VS tools are set-up (cl.exe is in the PATH, so I think you need this one for ninja)

@sam-github
Copy link
Contributor Author

@refack thanks a lot! I'm not trying to cross compile, maybe I should be using --dest-cpu=x32 instead of x64? Its a remote VM, I'll try to figure out its architecture.

@gibfahn
Copy link
Member

gibfahn commented Jul 18, 2017

thanks a lot! I'm not trying to cross compile, maybe I should be using --dest-cpu=x32 instead of x64? Its a remote VM, I'll try to figure out its architecture.

Assuming you're using the machine you were using before (mania) then it's 64-bit.

@refack
Copy link
Contributor

refack commented Jul 18, 2017

@refack thanks a lot! I'm not trying to cross compile, maybe I should be using --dest-cpu=x32 instead of x64? Its a remote VM, I'll try to figure out its architecture.

Anyway it's a GYP bug, so make sure you didn't lose the floating #12632

@bnoordhuis
Copy link
Member

I believe @sam-github's conclusion was that Windows FIPS support is basically unattainable. I'll close this out, reopen if I'm mistaken.

@refack
Copy link
Contributor

refack commented Aug 8, 2017

@sam-github do post a post-mortem please...

@bnoordhuis
Copy link
Member

Sam is on holiday but the big blocker was the CRT to link against - /MT vs. /MD - which you are not allowed to change if you want to be FIPS-compliant (and compliance is the point of this exercise.)

https://mta.openssl.org/pipermail/openssl-users/2017-July/006097.html for the interested.

@untra
Copy link

untra commented Nov 6, 2017

@sam-github Is there any followup to this? I am about to embark on a similar journey, trying to compile FIPS openssl into a windows build of node. Compiling FIPS+openssl is an endeavor into itself, and you seem to have taken a deep dive into this effort before. If there is any wisdom you can share (including whether "Windows FIPS support is basically unattainable") it would be greatly appreciated 🙏

@sam-github
Copy link
Contributor Author

I hit a dead end. AFAICT, the fips stub is compiled against the single-threaded runtime, which cannot be changed without invalidating the FIPS cert, and node is compiled against multi-threaded, and must be. This seemed irreconcileable to me, though maybe those are all just warnings, and can be ignored.

Also, gyp is horrible, and nmake is bad. Trying to get gyp to output the correct nmake files using the perl link script seems impossible, so I used the ninja output. I didn't try to get gyp to output correct ninja build rules for FIPS, I just directly hacked the .ninja files to try to build, figuring if I could get success at that (I didn't), I could then backtrack and try to figure out how to get gyp to output the correct .ninja files.

@untra
Copy link

untra commented Nov 7, 2017

@sam-github Thank you so much for sharing these insights 😲
I'm very intimidated by the process at this point. We have a windows node service within the logical crypto boundary, but at this point it looks easier to just rewrite the service in something other than javascript if it means avoiding this cross-compilation headache.

AFAICT, the fips stub is compiled against the single-threaded runtime, which cannot be changed without invalidating the FIPS cert, and node is compiled against multi-threaded, and must be. This seemed irreconcileable to me

That makes sense to me, but then how does the 'nix built FIPS+node reconcile those differences? Is that because 'nix executables can interact between singlethreaded/multithreaded runtimes while windows executables cannot? (I also recognize that both node and openssl were originally linux made with their windows counterparts developed as second-class citizens, so it's no surprise that the build process here is hellacious).

Edit: I just found https://mta.openssl.org/pipermail/openssl-users/2017-July/006126.html
okay, this explains this comment. Oh jeez...

I'm still going to take my own stab at this endeavor over this next week, so I'll report back my own attempts. I have never heard of the ninja build system. Is it worth it to try that approach?

@refack
Copy link
Contributor

refack commented Nov 7, 2017

I'm still going to take my own stab at this endeavor over this next week, so I'll report back my own attempts. I have never heard of the ninja build system. Is it worth it to try that approach?

Yes. Immensely simpler than MSBuild, and even simpler from make (also more performant as it uses hashes and not timestamps for build cache invalidation).

@sam-github
Copy link
Contributor Author

how does the 'nix built FIPS+node reconcile those differences?

They don't exist on unixen, there is no equivalent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

6 participants