New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 754473 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 649672



Sign in to add a comment

Fix dev-libs/opencryptoki for OpenSSL 1.1

Project Member Reported by djkurtz@chromium.org, Aug 10 2017

Issue description

dev-libs/opencryptoki-2.2.8-r24 fails to build with dev-libs/openssl-1.1.0f.

opencryptoki is a fork of an upstream package [0] that is now a cros_workon package.
[0] https://github.com/opencryptoki/opencryptoki

2.2.8-r24 corresponds to commit:
b67690aeeb4174b2253db18a9c1b19eeb219a4ef
rename configure.in->ac

It looks like the fork happened some time after upstream version v2.4, at commit:
d340b631a5d76c12dfea20aafc9431b611f66a20
Accept CKA_VALUE_LEN when unwrapping an aes key since

Upstream added OpenSSL 1.1 support in v3.6.1 (and fixed backwards compatibility in v3.6.2).

Upstream gentoo now uses v3.6.1, which includes the updates to OpenSSL 1.1, but not the backwards-compatible fix.

For the record, the build failures are:

soft_specific.c:429:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key2;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:430:5: error: unknown type name 'const_des_cblock'; did you mean 'const_DES_cblock'?
        const_des_cblock key_val_SSL, in_key_data;
        ^~~~~~~~~~~~~~~~
        const_DES_cblock
/usr/include/openssl/des.h:29:35: note: 'const_DES_cblock' declared here
typedef /* const */ unsigned char const_DES_cblock[8];
                                  ^
soft_specific.c:431:2: error: unknown type name 'des_cblock'; did you mean 'DES_cblock'?
        des_cblock out_key_data;
        ^~~~~~~~~~
        DES_cblock
/usr/include/openssl/des.h:28:23: note: 'DES_cblock' declared here
typedef unsigned char DES_cblock[8];
                      ^
soft_specific.c:436:2: warning: implicit declaration of function 'des_set_key_unchecked' is invalid in C99 [-Wimplicit-function-declaration]
        des_set_key_unchecked(&key_val_SSL, des_key2);
        ^
soft_specific.c:449:4: warning: implicit declaration of function 'des_ecb_encrypt' is invalid in C99 [-Wimplicit-function-declaration]
                        des_ecb_encrypt(&in_key_data, &out_key_data, des_key2, DES_ENCRYPT);
                        ^
soft_specific.c:481:2: error: unknown type name 'des_cblock'; did you mean 'DES_cblock'?
        des_cblock ivec;
        ^~~~~~~~~~
        DES_cblock
/usr/include/openssl/des.h:28:23: note: 'DES_cblock' declared here
typedef unsigned char DES_cblock[8];
                      ^
soft_specific.c:483:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key2;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:484:5: error: unknown type name 'const_des_cblock'; did you mean 'const_DES_cblock'?
        const_des_cblock key_val_SSL;
        ^~~~~~~~~~~~~~~~
        const_DES_cblock
/usr/include/openssl/des.h:29:35: note: 'const_DES_cblock' declared here
typedef /* const */ unsigned char const_DES_cblock[8];
                                  ^
soft_specific.c:500:3: warning: implicit declaration of function 'des_ncbc_encrypt' is invalid in C99 [-Wimplicit-function-declaration]
                des_ncbc_encrypt(in_data, out_data, in_data_len, des_key2, &ivec, DES_ENCRYPT);
                ^
soft_specific.c:522:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key1;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:523:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key2;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:524:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key3;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:526:5: error: unknown type name 'const_des_cblock'; did you mean 'const_DES_cblock'?
        const_des_cblock key_SSL1, key_SSL2, key_SSL3, in_key_data;
        ^~~~~~~~~~~~~~~~
        const_DES_cblock
/usr/include/openssl/des.h:29:35: note: 'const_DES_cblock' declared here
typedef /* const */ unsigned char const_DES_cblock[8];
                                  ^
soft_specific.c:527:2: error: unknown type name 'des_cblock'; did you mean 'DES_cblock'?
        des_cblock out_key_data;
        ^~~~~~~~~~
        DES_cblock
/usr/include/openssl/des.h:28:23: note: 'DES_cblock' declared here
typedef unsigned char DES_cblock[8];
                      ^
soft_specific.c:549:3: warning: implicit declaration of function 'des_ecb3_encrypt' is invalid in C99 [-Wimplicit-function-declaration]
                des_ecb3_encrypt((const_DES_cblock *)&in_key_data, 
                ^
soft_specific.c:587:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key1;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:588:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key2;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:589:2: error: unknown type name 'des_key_schedule'; did you mean 'DES_key_schedule'?
        des_key_schedule des_key3;
        ^~~~~~~~~~~~~~~~
        DES_key_schedule
/usr/include/openssl/des.h:43:3: note: 'DES_key_schedule' declared here
} DES_key_schedule;
  ^
soft_specific.c:591:5: error: unknown type name 'const_des_cblock'; did you mean 'const_DES_cblock'?
        const_des_cblock key_SSL1, key_SSL2, key_SSL3;
        ^~~~~~~~~~~~~~~~
        const_DES_cblock
/usr/include/openssl/des.h:29:35: note: 'const_DES_cblock' declared here
typedef /* const */ unsigned char const_DES_cblock[8];
                                  ^
soft_specific.c:592:2: error: unknown type name 'des_cblock'; did you mean 'DES_cblock'?
        des_cblock ivec;
        ^~~~~~~~~~
        DES_cblock
/usr/include/openssl/des.h:28:23: note: 'DES_cblock' declared here
typedef unsigned char DES_cblock[8];
                      ^
soft_specific.c:614:3: warning: implicit declaration of function 'des_ede3_cbc_encrypt' is invalid in C99 [-Wimplicit-function-declaration]
                des_ede3_cbc_encrypt(in_data,
                ^
soft_specific.c:683:5: error: incomplete definition of type 'struct rsa_st'
        rsa->n = bn_mod;
        ~~~^
/usr/include/openssl/ossl_typ.h:110:16: note: forward declaration of 'struct rsa_st'
typedef struct rsa_st RSA;
               ^
soft_specific.c:685:5: error: incomplete definition of type 'struct rsa_st'
        rsa->e = bn_exp;
        ~~~^
/usr/include/openssl/ossl_typ.h:110:16: note: forward declaration of 'struct rsa_st'
typedef struct rsa_st RSA;
               ^
soft_specific.c:757:6: error: incomplete definition of type 'struct rsa_st'
                rsa->n = bn_mod;
                ~~~^
/usr/include/openssl/ossl_typ.h:110:16: note: forward declaration of 'struct rsa_st'
typedef struct rsa_st RSA;
               ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
5 warnings and 20 errors generated.

 
Hmm. The chromium local fork of opencryptoki is really far behind.  It would probably be a good idea to unfork this package and resolve any local carried patches (there are about 20 of them).

However, for now a less disruptive way to add OpenSSL 1.1 support is to:
 (a) cherry-pick the related upstream fixes
 (b) cherry-pick any required preliminary backports to make (1) land cleanly
 (c) apply required fixes to any code added by local chromium commits
CLs:
   https://chromium-review.googlesource.com/612521 UPSTREAM: Fix public exponent attribute endianness bug        
   https://chromium-review.googlesource.com/612522 UPSTREAM: Check the return values of RSA functions        
   https://chromium-review.googlesource.com/612523 UPSTREAM:  Bug 67985  - correctly derivate CKA_MODULUS_BITS in RSA C_ObjectCreate        
   https://chromium-review.googlesource.com/612524 UPSTREAM: Fix rsa_keygen testcase        
   https://chromium-review.googlesource.com/612525 Revert "opencryptoki: Fix to opencryptoki PKCS#11 TPM generated key handling"        
   https://chromium-review.googlesource.com/612526 UPSTREAM: SF3131950: The public exponent needs to be added to the private ...        
   https://chromium-review.googlesource.com/612527 UPSTREAM: Fixes issue where public exponent passed to software token became ...        
   https://chromium-review.googlesource.com/612528 BACKPORT: Deprecated includes tss/tcpa_* removed, some casts to silence the ...        
   https://chromium-review.googlesource.com/612529 BACKPORT: Replace deprecated des_ methods and types        
   https://chromium-review.googlesource.com/612530 UPSTREAM: SOFT: Add support to openssl-1.1 RSA functions        
   https://chromium-review.googlesource.com/612531 UPSTREAM: TPM: Replace openssl-1.1 APIs that were removed        
   https://chromium-review.googlesource.com/612532 UPSTREAM: Add backward compatibility to openssl older versions        
   https://chromium-review.googlesource.com/612533 UPSTREAM: Fix the retrieval of p from a generated rsa key        
   https://chromium-review.googlesource.com/612534 BACKPORT: Fix warning regarding deprecated __BSD_SOURCE        
   https://chromium-review.googlesource.com/612535 BACKPORT: Replace deprecated RSA_generate_key        
   https://chromium-review.googlesource.com/612536 CHROMIUM: Fix tpm_specific.c for OpenSSL 1.1        
   https://chromium-review.googlesource.com/612537 CHROMIUM: tpm_specific: fix -Wpointer-sign warning        

Comment 3 by vapier@chromium.org, Aug 15 2017

fwiw, i don't feel confident in reviewing these as i've never worked on this project before
Cc: kmixter@chromium.org drinkcat@chromium.org mnissler@chromium.org gauravsh@chromium.org gspencer@chromium.org benchan@chromium.org
@#3 Well, technically you made the last two commits to this repo - and the only two meaningful commits since 01/2012.  Back then it looked like gauravsh, gspencer, kmixter and benchan were working on opencryptoki.

These patches are mostly upstream commits that make the OpenSSL 1.1 stuff apply cleanly.
Cc: -gspencer@chromium.org -gauravsh@chromium.org -kmixter@chromium.org jorgelo@chromium.org dkrahn@chromium.org
+jorgelo, +dkrahn, -a few others who no longer work on Chrome OS
What are we using opencryptoki again? What packages depend on it?
$ equery-samus depends opencryptoki
 * These packages depend on opencryptoki:
app-crypt/tpm-tools-1.3.9-r1 (pkcs11 ? dev-libs/opencryptoki)

$ equery-samus depends tpm-tools
 * These packages depend on tpm-tools:
virtual/target-chromium-os-dev-1-r23 (tpm ? app-crypt/tpm-tools)
virtual/target-chromium-os-test-1-r51 (tpm ? app-crypt/tpm-tools)

Comment 8 by dkrahn@chromium.org, Aug 17 2017

I think this is just an unfortunate dependency. I don't think we use anything in tpm-tools that uses opencryptoki. If we do, we probably shouldn't. IMO, we should also purge all memory of opencryptoki from Chrome OS.

I've forgotten most of what I once knew about ebuilds (and that wasn't much) so I don't know if we can drop 'pkcs11' here but keep it for other packages (I think this same flag is used to pull in chaps).
Mike, any Portage wizardry we could use here?
tpm-tools was enabled here:
   https://crbug.com/196743 
and was done simply because tpm-tools was failing to build w/out it.

lets turn it off and see what happens:
  https://chromium-review.googlesource.com/618468
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/fe17a4c277a3d6847ef5fcf73e3bcd4aeb5aefa5

commit fe17a4c277a3d6847ef5fcf73e3bcd4aeb5aefa5
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Aug 17 18:31:49 2017

tpm-tools: disable USE=pkcs11

This was turned on a while back ( https://crbug.com/196743 ) to fix a
build failure, not because anyone seems to need this functionality.
Turn it back off.

BUG= chromium:754473 
TEST=precq passes

Change-Id: I8e331a0c0e2fe37897ffaa51acf4ffa0d8a9d2bf
Reviewed-on: https://chromium-review.googlesource.com/618468
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/fe17a4c277a3d6847ef5fcf73e3bcd4aeb5aefa5/profiles/targets/chromeos/package.use

I'm not an expert, but "pkcs11" can still be set in USE flags, right?
Shouldn't we instead have a line "app-crypt/tpm-tools -pkcs11" in package.use?

Or even modify tpm-tools ebuild to never depend on opencryptoki and always have "--disable-pkcs11-support" in our case, regardless of USE=pkcs11?



yes, boards can set USE=pkcs11, but no one does that today.  but if a board decided to do that and ran into the opencryptoki dep, it'd be their problem to sort out.  there's nothing in CrOS that says "we never want opencryptoki".

if opencryptoki is no longer being pulled into our builds, and no one cares about it, we simply drop it from the tree entirely.  that makes it even more clear that, if someone tries to bring it back, it's their problem to make sure it works.

imo it doesn't make sense to change tpm-tools to hard disable a dep like this.
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/977226036910d12daeb6c7bcfa2b3adca14d14f4

commit 977226036910d12daeb6c7bcfa2b3adca14d14f4
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 18 01:42:32 2017

opencryptoki: punt it

No one is using this anymore, and it causes troubles with OpenSSL
upgrades.  Punt it from our build.

BUG= chromium:754473 
TEST=precq passes

Change-Id: Ia358a7feb17e727ff22bdf114de8c2c26c9806b3
Reviewed-on: https://chromium-review.googlesource.com/619016
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[delete] https://crrev.com/6c415f1ec71668ce98617926387050ae22f6ecc5/dev-libs/opencryptoki/opencryptoki-2.2.8-r24.ebuild
[delete] https://crrev.com/6c415f1ec71668ce98617926387050ae22f6ecc5/dev-libs/opencryptoki/metadata.xml
[delete] https://crrev.com/6c415f1ec71668ce98617926387050ae22f6ecc5/dev-libs/opencryptoki/opencryptoki-9999.ebuild

Cc: djkurtz@chromium.org
Owner: vapier@chromium.org
sorry Dan you did all that work for us to just delete it :)
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chromeos/manifest-internal/+/ec2ace572d1aadc3a26fd3fb9325ffb6c1151a04

commit ec2ace572d1aadc3a26fd3fb9325ffb6c1151a04
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 18 07:57:27 2017

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/manifest/+/a70354694d2a784aac3413506311480c18ec3b38

commit a70354694d2a784aac3413506311480c18ec3b38
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 18 07:57:29 2017

opencryptoki: punt it

No one is using this anymore, and it causes troubles with OpenSSL
upgrades.  Punt it from our build.

BUG= chromium:754473 
TEST=precq passes

Change-Id: I2b8f7b4526515243fc59eeac84415ad2e700c108
Reviewed-on: https://chromium-review.googlesource.com/619300
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/a70354694d2a784aac3413506311480c18ec3b38/full.xml

Status: Fixed (was: Started)
@#15 - I like your solution better!

Comment 20 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
Status: Fixed (was: Archived)

Sign in to add a comment