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

OpenSSL Fips broken with 1.0.2n #4864

Closed
Jan-E opened this Issue Dec 7, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@Jan-E
Copy link

Jan-E commented Dec 7, 2017

With OpenSSL 1.0.2n and OpenSSL Fips 2.0.16 I am running into this compile error:

        cl /Fotmp32dll\o_init.obj  -Iinc32 -Itmp32dll /MD /Ox /O2 /Ob2 -DOPENSSL_THREADS  -
DDSO_WIN32 -W3 -WX -Gs0 -GF -Gy -nologo -DOPENSSL_SYSNAME_WIN32 -DWIN32_LEAN_AND_MEAN -DL_E
NDIAN -D_CRT_SECURE_NO_DEPRECATE -D_WINSOCK_DEPRECATED_NO_WARNINGS -DOPENSSL_BN_ASM_PART_WO
RDS -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -I\usr\local\ssl\fips-2
.0/include -DRC4_ASM -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM
-DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM -DOPENSSL_USE_APPLINK -I. -DOPENSSL_NO_RC5 -DOPENSS
L_NO_MD2 -DOPENSSL_NO_SSL2 -DOPENSSL_NO_KRB5 -DOPENSSL_FIPS -DOPENSSL_NO_JPAKE -DOPENSSL_NO
_WEAK_SSL_CIPHERS -DOPENSSL_NO_STATIC_ENGINE /Fdtmp32dll/lib -D_WINDLL  -DOPENSSL_BUILD_SHL
IBCRYPTO -c .\crypto\o_init.c
o_init.c
.\crypto\o_init.c(77): error C2220: warning treated as error - no 'object' file generated
.\crypto\o_init.c(77): warning C4013: 'FIPS_crypto_set_id_callback' undefined; assuming extern returning int
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.EXE"' : return code '0x2'
Stop.

Did something change in the FIPS compilation?

@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

Related commit:
a43cfd7

@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

@jfigus @richsalz If I revert the patch in a43cfd7 OpenSSL 1.0.2n with Fips 2.0.16 compiles fine and passes all tests. However, I do not know if it is safe to revert it.

@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Dec 7, 2017

This seems to be an old issue previously discovered some while ago, see #2533. So this doesn't seem to be any change specific to the 1.0.2n release. That issue doesn't seem to have gone anywhere though, so I wonder why more people haven't run into this? Perhaps because that issue says its a warning only on most platforms. @mspncp may know more.

Anyway, what options are you using to Configure OpenSSL? The "warning treated as error" message says that you have asked for warning to be treated as errors - which suggests a possible work around for this is to not do that!

@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

Basically, I am just using
perl Configure VC-WIN32 fips --with-fipsdir=\usr\local\ssl\fips-2.0

After searching what added the -WX flag to treat warnings as error I ran into
b2921cf

Configure: add back /WX to VC-WIN32.
We had /WX (treat warnings as errors) in VC-WIN32 for long time. At
some point it was somehow omitted. It's argued that it allows to
keep better focus on new code, which motivates the comeback...

Reviewed-by: Rich Salz rsalz@openssl.org

dot-asm committed 26 days ago

At least, this explains why I am running into this now. The question is what is the preferred way:

  1. Revert the patch that added the callback
  2. Remove the WX flag once again
@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Dec 7, 2017

  1. Await the patch that @mspncp just mentioned in #2533 that he is in the middle of writing.
@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

Fine with me

@mattcaswell

This comment has been minimized.

Copy link
Member

mattcaswell commented Dec 7, 2017

See #4870

@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

I already found it.

pending 2nd review

@Jan-E

This comment has been minimized.

Copy link
Author

Jan-E commented Dec 7, 2017

Closing. Fixed with #4870

@Jan-E Jan-E closed this Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.