I've removed the --fpu switch and have so far not seen any issues (building).
I then encountered an issue of duplicate symbols between BaseCryptLib and
OpensslLib:
1>armlink : error L6200: Symbol TS_ACCURACY_it multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol TS_TST_INFO_it multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol TS_ACCURACY_free multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol TS_ACCURACY_new multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol TS_TST_INFO_free multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol TS_TST_INFO_new multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol d2i_TS_ACCURACY multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol d2i_TS_TST_INFO multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol i2d_TS_ACCURACY multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
1>armlink : error L6200: Symbol i2d_TS_TST_INFO multiply defined (by
BaseCryptLib.lib and OpensslLib.lib).
digging deeper I see magic macros that define these functions:
DECLARE_ASN1_FUNCTIONS (TS_ACCURACY)
which generates the _it, _new, _free, d2i_ and i2d_ functions. Ok, fine.
But then I see this happens in two places, once in openssl proper:
CryptoPkg\Library\OpensslLib\openssl-1.0.2d\crypto\ts\ts_asn1.c(135):IMPLEMENT_ASN1_FUNCTIONS_const(TS_ACCURACY)
and again in BaseCryptLib:
CryptoPkg\Library\BaseCryptLib\Pk\CryptTs.c(83):IMPLEMENT_ASN1_FUNCTIONS
(TS_ACCURACY)
and the TS_ACCURACY and TS_TS_INFO structures are duplicated, with the only
difference being the case of the structure member names - maybe an attempt to
fit the edk2 coding convetion? Since BaseCryptLib depends on OpensslLib, how
does this ever work? Do other linkers tolerate the duplication?
Is there a reason BaseCryptLib needs to redefine them instead of using using
the versions in OpensslLib?
As an experiment, I tried to just remove the duplicate function definitions and
structures and after fixing up the structure member case issues I got it all to
build. I have no idea if it works but hopefully Long can tell me if the
approach of just deleting code I don't understand has any pitfalls. :)
I've included a patch with the fixes to get openssl / CryptoPkg building again.
Note that there is a tweak to openssl itself which will need to get reflected
in the patch and upstreamed (still waiting to hear who is willing and able to
do this).
CryptoPkg: Fix ARM RVCT 4 build
Change struct timeval to add typedef prefix in OpenSslSupport.h to prevent
duplicate symbol definition error
Remove duplicated TS_ACCURACY and TS_TST_INFO IN CryptTs.c to prevent duplicate
symbols definition error
Add printf stub to CrtWrapper.c to prevent missing symbol error
Update openssl library build to remove the FPU switch and to suppress the
following warnings on RVCT:
128: loop is not reachable from preceding code
546: transfer of control bypasses initialization of variable
Change openssl-1.0.2d to not assume the GCC path when building on RVCT causing
a call to a missing __builtin_alloca function
---
edk2/CryptoPkg/Include/OpenSslSupport.h | 2 +-
edk2/CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c | 101 +++------------------
.../Library/BaseCryptLib/SysCall/CrtWrapper.c | 7 +-
edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf | 4 +-
.../OpensslLib/openssl-1.0.2d/crypto/bn/bn_exp.c | 2 +-
5 files changed, 22 insertions(+), 94 deletions(-)
diff --git a/edk2/CryptoPkg/Include/OpenSslSupport.h
b/edk2/CryptoPkg/Include/OpenSslSupport.h
index 64bef42..3a25750 100644
--- a/edk2/CryptoPkg/Include/OpenSslSupport.h
+++ b/edk2/CryptoPkg/Include/OpenSslSupport.h
@@ -136,7 +136,7 @@ struct tm {
char *tm_zone; /* timezone abbreviation */
};
-struct timeval {
+typedef struct timeval {
long tv_sec; /* time value, in seconds */
long tv_usec; /* time value, in microseconds */
} timeval;
diff --git a/edk2/CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c
b/edk2/CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c
index d495812..fe658a3 100644
--- a/edk2/CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c
+++ b/edk2/CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c
@@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include <openssl/x509.h>
#include <openssl/x509v3.h>
#include <openssl/pkcs7.h>
+#include <openssl/ts.h>
//
// OID ASN.1 Value for SPC_RFC3161_OBJID ("1.3.6.1.4.1.311.3.3.1")
@@ -56,86 +57,6 @@ ASN1_SEQUENCE (TS_MESSAGE_IMPRINT) = {
} ASN1_SEQUENCE_END (TS_MESSAGE_IMPRINT)
IMPLEMENT_ASN1_FUNCTIONS (TS_MESSAGE_IMPRINT)
-///
-/// Accuracy represents the time deviation around the UTC time contained
-/// in GeneralizedTime of time-stamp token.
-///
-/// Accuracy ::= SEQUENCE {
-/// seconds INTEGER OPTIONAL,
-/// millis [0] INTEGER (1..999) OPTIONAL,
-/// micros [1] INTEGER (1..999) OPTIONAL }
-///
-typedef struct {
- ASN1_INTEGER *Seconds;
- ASN1_INTEGER *Millis;
- ASN1_INTEGER *Micros;
-} TS_ACCURACY;
-
-//
-// ASN.1 Functions for TS_ACCURACY
-//
-DECLARE_ASN1_FUNCTIONS (TS_ACCURACY)
-ASN1_SEQUENCE (TS_ACCURACY) = {
- ASN1_OPT (TS_ACCURACY, Seconds, ASN1_INTEGER),
- ASN1_IMP_OPT (TS_ACCURACY, Millis, ASN1_INTEGER, 0),
- ASN1_IMP_OPT (TS_ACCURACY, Micros, ASN1_INTEGER, 1)
-} ASN1_SEQUENCE_END (TS_ACCURACY)
-IMPLEMENT_ASN1_FUNCTIONS (TS_ACCURACY)
-
-///
-/// The timestamp token info resulting from a successful timestamp request,
-/// as defined in RFC 3161.
-///
-/// TSTInfo ::= SEQUENCE {
-/// version INTEGER { v1(1) },
-/// policy TSAPolicyId,
-/// messageImprint MessageImprint,
-/// -- MUST have the same value as the similar field in
-/// -- TimeStampReq
-/// serialNumber INTEGER,
-/// -- Time-Stamping users MUST be ready to accommodate integers
-/// -- up to 160 bits.
-/// genTime GeneralizedTime,
-/// accuracy Accuracy OPTIONAL,
-/// ordering BOOLEAN DEFAULT FALSE,
-/// nonce INTEGER OPTIONAL,
-/// -- MUST be present if the similar field was present
-/// -- in TimeStampReq. In that case it MUST have the same value.
-/// tsa [0] GeneralName OPTIONAL,
-/// extensions [1] IMPLICIT Extensions OPTIONAL }
-///
-typedef struct {
- ASN1_INTEGER *Version;
- ASN1_OBJECT *Policy;
- TS_MESSAGE_IMPRINT *MessageImprint;
- ASN1_INTEGER *SerialNumber;
- ASN1_GENERALIZEDTIME *GenTime;
- TS_ACCURACY *Accuracy;
- ASN1_BOOLEAN Ordering;
- ASN1_INTEGER *Nonce;
- GENERAL_NAME *Tsa;
- STACK_OF(X509_EXTENSION) *Extensions;
-} TS_TST_INFO;
-
-//
-// ASN.1 Functions for TS_TST_INFO
-//
-DECLARE_ASN1_FUNCTIONS (TS_TST_INFO)
-ASN1_SEQUENCE (TS_TST_INFO) = {
- ASN1_SIMPLE (TS_TST_INFO, Version, ASN1_INTEGER),
- ASN1_SIMPLE (TS_TST_INFO, Policy, ASN1_OBJECT),
- ASN1_SIMPLE (TS_TST_INFO, MessageImprint, TS_MESSAGE_IMPRINT),
- ASN1_SIMPLE (TS_TST_INFO, SerialNumber, ASN1_INTEGER),
- ASN1_SIMPLE (TS_TST_INFO, GenTime, ASN1_GENERALIZEDTIME),
- ASN1_OPT (TS_TST_INFO, Accuracy, TS_ACCURACY),
- ASN1_OPT (TS_TST_INFO, Ordering, ASN1_FBOOLEAN),
- ASN1_OPT (TS_TST_INFO, Nonce, ASN1_INTEGER),
- ASN1_EXP_OPT(TS_TST_INFO, Tsa, GENERAL_NAME, 0),
- ASN1_IMP_SEQUENCE_OF_OPT (TS_TST_INFO, Extensions, X509_EXTENSION, 1)
-} ASN1_SEQUENCE_END (TS_TST_INFO)
-IMPLEMENT_ASN1_FUNCTIONS (TS_TST_INFO)
-
-
/**
Convert ASN.1 GeneralizedTime to EFI Time.
@@ -236,7 +157,7 @@ CheckTSTInfo (
)
{
BOOLEAN Status;
- TS_MESSAGE_IMPRINT *Imprint;
+ TS_MSG_IMPRINT *Imprint;
X509_ALGOR *HashAlgo;
CONST EVP_MD *Md;
EVP_MD_CTX MdCtx;
@@ -255,7 +176,7 @@ CheckTSTInfo (
// The version field (currently v1) describes the version of the
time-stamp token.
// Conforming time-stamping servers MUST be able to provide version 1
time-stamp tokens.
//
- if ((ASN1_INTEGER_get (TstInfo->Version)) != 1) {
+ if ((ASN1_INTEGER_get (TstInfo->version)) != 1) {
return FALSE;
}
@@ -263,7 +184,7 @@ CheckTSTInfo (
// -- Check Policies
// The policy field MUST indicate the TSA's policy under which the
response was produced.
//
- if (TstInfo->Policy == NULL) {
+ if (TstInfo->policy_id == NULL) {
/// NOTE: Need to check if the requested and returned policies.
/// We have no information about the Requested TSA Policy.
return FALSE;
@@ -272,8 +193,8 @@ CheckTSTInfo (
//
// -- Compute & Check Message Imprint
//
- Imprint = TstInfo->MessageImprint;
- HashAlgo = X509_ALGOR_dup (Imprint->HashAlgorithm);
+ Imprint = TstInfo->msg_imprint;
+ HashAlgo = X509_ALGOR_dup(Imprint->hash_algo);
Md = EVP_get_digestbyobj (HashAlgo->algorithm);
if (Md == NULL) {
@@ -288,15 +209,15 @@ CheckTSTInfo (
EVP_DigestInit (&MdCtx, Md);
EVP_DigestUpdate (&MdCtx, TimestampedData, DataSize);
EVP_DigestFinal (&MdCtx, HashedMsg, NULL);
- if ((MdSize == (UINTN)ASN1_STRING_length (Imprint->HashedMessage)) &&
- (CompareMem (HashedMsg, ASN1_STRING_data (Imprint->HashedMessage),
MdSize) != 0)) {
+ if ((MdSize == (UINTN)ASN1_STRING_length(Imprint->hashed_msg)) &&
+ (CompareMem(HashedMsg, ASN1_STRING_data(Imprint->hashed_msg), MdSize) !=
0)) {
goto _Exit;
}
//
// -- Check Nonces
//
- if (TstInfo->Nonce != NULL) {
+ if (TstInfo->nonce != NULL) {
//
// Nonces is optional, No error if no nonce is returned;
//
@@ -305,7 +226,7 @@ CheckTSTInfo (
//
// -- Check if the TSA name and signer certificate is matched.
//
- if (TstInfo->Tsa != NULL) {
+ if (TstInfo->tsa != NULL) {
//
// Ignored the optional Tsa field checking.
//
@@ -476,7 +397,7 @@ TimestampTokenVerify (
//
if (SigningTime != NULL) {
SetMem (SigningTime, sizeof (EFI_TIME), 0);
- Status = ConvertAsn1TimeToEfiTime (TstInfo->GenTime, SigningTime);
+ Status = ConvertAsn1TimeToEfiTime(TstInfo->time, SigningTime);
}
_Exit:
diff --git a/edk2/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
b/edk2/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
index fb446b6..dda3c01 100644
--- a/edk2/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
+++ b/edk2/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
@@ -384,7 +384,12 @@ size_t fread (void *b, size_t c, size_t i, FILE *f)
return 0;
}
-int fprintf (FILE *f, const char *s, ...)
+int fprintf(FILE *f, const char *s, ...)
+{
+ return 0;
+}
+
+int printf(const char *s, ...)
{
return 0;
}
diff --git a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index 261861b..e9ce8c1 100644
--- a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -889,6 +889,8 @@
# 513: a value of type <type> cannot be assigned to an entity of type <type>
# 188: enumerated type mixed with another type (i.e. passing an integer as
an enum without a cast)
# 1296: Extended constant initialiser used
- RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS)
--library_interface=aeabi_clib99 --fpu=vfpv3
--diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546
+ # 128: loop is not reachable from preceding code
+ # 546: transfer of control bypasses initialization of variable
+ RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS)
--library_interface=aeabi_clib99
--diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546
XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w
XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w
diff --git
a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/bn/bn_exp.c
b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/bn/bn_exp.c
index 24afdd6..b6edd94 100644
--- a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/bn/bn_exp.c
+++ b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/bn/bn_exp.c
@@ -118,7 +118,7 @@
# ifndef alloca
# define alloca _alloca
# endif
-#elif defined(__GNUC__)
+#elif defined(__GNUC__) && !defined(__CC_ARM)
# ifndef alloca
# define alloca(s) __builtin_alloca((s))
# endif
--
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eugene Cohen < [email protected]>
Eugene
-----Original Message-----
From: Ard Biesheuvel [mailto:[email protected]]
Sent: Tuesday, November 24, 2015 9:56 AM
To: Cohen, Eugene <[email protected]>; Leif Lindholm <[email protected]>; Leif
Lindholm <[email protected]>
Cc: David Woodhouse <[email protected]>; Long, Qin <[email protected]>;
[email protected] <[email protected]>
Subject: Re: [edk2] CryptoPkg: OpenSSL build issue with RVCT
On 24 November 2015 at 16:53, Cohen, Eugene <[email protected]> wrote:
> Ard, thanks - as you can tell this is an issue in the original flags and not
> with the changes I've added. I agree that we should not redefine the fpu in
> this file but rather inherit from the build.
>
Looking at the repo history, it turns out that this has been present
since the original RVCT support was added to OpenSslLib in 2011. That
does not make it right, though, and using this library to implement
runtime services for authenticated variables is going to cause trouble
regardless. So I am inclined to suggest it be removed, considering
that EDK2 is a reference implementation, and we should not be setting
bad examples.
> David, good comments - I'm working on an updated patch. I'm running into
> some issues related to printf and alloca that I need to address as well.
>
> I don't think we want each edk2 developer independently opening openssl
> tickets as we encounter issues. It would probably make more sense for Long
> to do this or, if we think this is an RVCT-unique issue, for Ard or Leif to
> do this. Do we already have these responsibilities identified?
>
I'd like to see RVCT support in upstream OpenSSL first. Otherwise, it
will be a moving target, and we can never upgrade our OpenSSL version
without the risk of breaking the build for RVCT in a way that requires
new OpenSSL tickets to be created. Unfortunately, I don't have the
bandwidth to get involved in that, since RVCT is not a priority for
Linaro. (Note that RVCT is 32-bit only. The proprietary 64-bit ARM
compiler is based on Clang. Also, the RVCT asm dialect is completely
different, so none of the ALU and NEON accelerated ARM implementation
for AES and SHA can be built with it without major surgery on the
perlasm files)
Then, we can see what issues remain in the EDK2 integration, the most
important of which is the lack of softfloat support (but perhaps we
can port the code from StdLibPkg so that it can be used in DXE?)
Regards,
Ard.
> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Tuesday, November 24, 2015 8:49 AM
> To: David Woodhouse <[email protected]>
> Cc: Cohen, Eugene <[email protected]>; Long, Qin <[email protected]>;
> [email protected] <[email protected]>; Leif Lindholm
> <[email protected]>
> Subject: Re: [edk2] CryptoPkg: OpenSSL build issue with RVCT
>
> On 24 November 2015 at 15:47, David Woodhouse <[email protected]> wrote:
>> On Tue, 2015-11-24 at 14:19 +0000, Cohen, Eugene wrote:
>>>
>>> Here's a patch with this changes:
>>>
>>> ---
>>> edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf |
>>> 2 +-
>>> edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c |
>>> 1 +
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> index 2e74f6c..261861b 100644
>>> --- a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> +++ b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>> @@ -889,6 +889,6 @@
>>> # 513: a value of type cannot be assigned to an entity of type
>>> # 188: enumerated type mixed with another type (i.e. passing an integer
>>> as an enum without a cast)
>>> # 1296: Extended constant initialiser used
>>> - RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS)
>>> --library_interface=aeabi_clib99 --fpu=vfpv3
>>> --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188
>>> + RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS)
>>> --library_interface=aeabi_clib99 --fpu=vfpv3
>>> --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546
>
> This looks wrong. The UEFI spec disallows the use of hardware FP on
> 32-bit ARM, and yet we are passing --fpu=vfpv3. I actually tried
> building OpenSslLib for ARM using GCC, and got in trouble with the RNG
> routines that rely on FP, so my conclusion was that it requires some
> software FP routines (or a build time switch to omit those files and
> lose some functionality)
>
> Note that this is not a theoretical problem. Under Linux, you can only
> use FP in kernel mode if you explicitly wrap it with fp_begin/fp_end
> routines, and if you don't, anything that touches the FP register file
> will trap. This includes the UEFI runtime services invocations, since
> their entry/exit routines don't call those wrappers. And even if it
> wouldn't trap, you would still have to preserve the contents of the FP
> registers, since an ordinary context switch or syscall does not take
> care of that.
>
> --
> Ard.
>
>
>>
>>
>> You neglected to add to the comments — they're there in the context of
>> your patch, explaining at least #513, #188 and #1296. You should be
>> adding the same for the newly-added #128 and #546.
>>
>> Additionally *all* of those comments should have a reference to the
>> specific OpenSSL RT ticket which covers the problem. And then we can
>> properly track them and remove them when they're obsolete, and don't
>> just accumulate them for ever.
>>
>>> XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64
>>> $(OPENSSL_FLAGS) -w
>>> XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64
>>> $(OPENSSL_FLAGS) -w
>>> diff --git
>>> a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c
>>> b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c
>>> index 35dde28..35e3b39 100644
>>> ---
>>> a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c
>>> +++
>>> b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c
>>> @@ -859,6 +859,7 @@ static int check_cert(X509_STORE_CTX *ctx)
>>> X509 *x;
>>> int ok, cnum;
>>> unsigned int last_reasons;
>>> + ok = 0;
>>> cnum = ctx->error_depth;
>>> x = sk_X509_value(ctx->chain, cnum);
>>> ctx->current_cert = x;
>>
>> Again, don't add this here without an upstream RT ticket being filed.
>> The aim is to get to *zero* delta between us and upstream OpenSSL, and
>> we won't get there if we do this kind of thing.
>>
>> --
>> dwmw2
>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
