Improved pkcs8 support

karel-m commented

2018-06-11 13:19:55 -05:00

(Migrated from github.com)

This is a re-created #403 which got closed and for some mysterious reasons cannot be re-opened. We should share the new ECC+pcks8 with RSA as the old rsa_import_pkcs8 does not support password protected keys.

ecc_import_pkcs8
rsa_import_pkcs8
documentation
further tests

This is a re-created #403 which got closed and for some mysterious reasons cannot be re-opened. We should share the new ECC+pcks8 with RSA as the old `rsa_import_pkcs8` does not support password protected keys. * [x] ecc_import_pkcs8 * [x] rsa_import_pkcs8 * [ ] documentation * [x] further tests

sjaeckel commented

2018-07-06 12:23:27 -05:00

(Migrated from github.com)

When factoring-out the relevant pkcs#8 parts into an own module, I'll put it in src/pk/asn1/pkcs8? or do you think it should go somewhere else?

When factoring-out the relevant pkcs#8 parts into an own module, I'll put it in `src/pk/asn1/pkcs8`? or do you think it should go somewhere else?

karel-m commented

2018-07-06 12:53:43 -05:00

(Migrated from github.com)

I am also not sure, perhaps

src/misc/pbes/pbes1_decrypt.c
src/misc/pbes/pbes2_pbkdf2_decrypt.c         (or just pbes2_decrypt.c)
src/pk/asn1/pkcs8/der_decode_pkcs8_flexi.c

I am also not sure, perhaps ``` src/misc/pbes/pbes1_decrypt.c src/misc/pbes/pbes2_pbkdf2_decrypt.c (or just pbes2_decrypt.c) src/pk/asn1/pkcs8/der_decode_pkcs8_flexi.c ```

karel-m commented

2018-09-30 09:47:19 -05:00

(Migrated from github.com)

@sjaeckel do you have any plans with your ideas from refactor/pr/pkcs8-improved (in libtomcrypt_p)?

@sjaeckel do you have any plans with your ideas from `refactor/pr/pkcs8-improved` (in libtomcrypt_p)?

sjaeckel commented

2018-10-04 02:32:37 -05:00

(Migrated from github.com)

I'm working on it ;)

sjaeckel commented

2018-10-04 08:48:55 -05:00

(Migrated from github.com)

I've force-pushed pr/pkcs8-improved and created pr/pkcs8-improved-original which points to your original latest version

I've force-pushed `pr/pkcs8-improved` and created `pr/pkcs8-improved-original` which points to your original latest version

👍 1

sjaeckel commented

2018-10-04 10:51:46 -05:00

(Migrated from github.com)

so either we disable the PKCS#8 API's in the LTC_EASY build or we'd need the entire chain PKCS#8+PBES+PKCS#5+PKCS#12+PADDING ... what do you think?

... I just realized that all the PKCS options are enabled anyways for LTC_EASY ... so it'd be only the PADDING

so either we disable the PKCS#8 API's in the `LTC_EASY` build or we'd need the entire chain PKCS#8+PBES+PKCS#5+PKCS#12+PADDING ... what do you think? ... I just realized that all the PKCS options are enabled anyways for `LTC_EASY` ... so it'd be only the PADDING

karel-m commented

2018-10-04 13:02:15 -05:00

(Migrated from github.com)

Adding PADDING to LTC_EASY is IMO ok.

Adding `PADDING` to `LTC_EASY` is IMO ok.

karel-m commented

2018-10-04 13:23:24 -05:00

(Migrated from github.com)

clang-tidy:

src/pk/ecc/ecc_import_pkcs8.c:29:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (flexi->type != LTC_ASN1_SEQUENCE)
                                        ^
                                         {
src/pk/ecc/ecc_import_pkcs8.c:33:44: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
      if (!LTC_ASN1_IS_TYPE(cur, check->t))
                                           ^
                                            {
clang-tidy FAILED!

clang-tidy: ``` src/pk/ecc/ecc_import_pkcs8.c:29:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors] if (flexi->type != LTC_ASN1_SEQUENCE) ^ { src/pk/ecc/ecc_import_pkcs8.c:33:44: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors] if (!LTC_ASN1_IS_TYPE(cur, check->t)) ^ { clang-tidy FAILED! ```

sjaeckel commented

2018-10-04 13:37:46 -05:00

(Migrated from github.com)

clang-tidy FAILED!

fixed locally, waiting for more review!?

> ``` > clang-tidy FAILED! > ``` fixed locally, waiting for more review!?

karel-m commented

2018-10-04 13:43:40 -05:00

(Migrated from github.com)

It looks good, but while testing with my perl test suit (which passed all pkcs8 EC/RSA tests) it turned out that the latest ECDSA changes broke some wycheproof tests - now I am trying to fix them.

sjaeckel commented

2018-10-04 13:48:48 -05:00

(Migrated from github.com)

Okay, then I'll force-push so you can base your changes on top of that (if there are some to come)

btw @rmw42 you're also welcome to review if you feel like it :)

Okay, then I'll force-push so you can base your changes on top of that (if there are some to come) btw @rmw42 you're also welcome to review if you feel like it :)

sjaeckel commented

2018-10-04 14:54:50 -05:00

(Migrated from github.com)

Do you probably have the time to create the missing test-vectors for pbes1 (sha1 with 3des), pbes2 (RC2 with weird key sizes) and encrypted cases for rsa?

rmw42 commented

2018-10-04 15:55:26 -05:00

(Migrated from github.com)

btw @rmw42 you're also welcome to review if you feel like it :)

Cheers! Not sure I'd even know where to start - haven't had any dealings with PKCS#8 ^_^;;

> btw @rmw42 you're also welcome to review if you feel like it :) Cheers! Not sure I'd even know where to start - haven't had any dealings with PKCS#8 ^_^;;

sjaeckel commented

2018-10-04 16:01:46 -05:00

(Migrated from github.com)

... haven't had any dealings with PKCS#8 ^_^;;

neither did I of your SSH stuff ;)

...also I meant more in regards to the code or obvious bugs, not functionality

[edit] and no worries if you don't review, your contributions are already more than enough!

> ... haven't had any dealings with PKCS#8 ^_^;; neither did I of your SSH stuff ;) ...also I meant more in regards to the code or obvious bugs, not functionality [edit] and no worries if you don't review, your contributions are already more than enough!

👍 1

karel-m commented

2018-10-04 16:55:20 -05:00

(Migrated from github.com)

Do you probably have the time to create the missing test-vectors for pbes1 (sha1 with 3des), pbes2 (RC2 with weird key sizes) and encrypted cases for rsa?

I'll do it over the weekend.

> Do you probably have the time to create the missing test-vectors for pbes1 (sha1 with 3des), pbes2 (RC2 with weird key sizes) and encrypted cases for rsa? I'll do it over the weekend.

🎉 1

karel-m commented

2018-10-05 15:59:50 -05:00

(Migrated from github.com)

RSA + encrypted pkcs8 tests added

sjaeckel commented

2018-10-06 05:56:24 -05:00

(Migrated from github.com)

RSA + encrypted pkcs8 tests added

awesome, thanks :)

also especially for the docs!

> RSA + encrypted pkcs8 tests added awesome, thanks :) also especially for the docs!

karel-m commented

2018-10-06 06:00:50 -05:00

(Migrated from github.com)

I think it is ready for rebase & merge (after merging #438)

sjaeckel commented

2018-10-06 06:37:37 -05:00

(Migrated from github.com)

Do we have to wait for #438 or can I also merge now?

sjaeckel commented

2018-10-06 07:39:27 -05:00

(Migrated from github.com)

I just created testvectors for PBES2 RC2-40 and RC2-64 and they can't be handled... dunno yet why :(

rc2_keys.zip

I just created testvectors for PBES2 RC2-40 and RC2-64 and they can't be handled... dunno yet why :( [rc2_keys.zip](https://github.com/libtom/libtomcrypt/files/2453033/rc2_keys.zip)

karel-m commented

2018-10-06 11:14:06 -05:00

(Migrated from github.com)

Looks like we can load a key created like this:

openssl pkcs8 -topk8 -inform PEM -outform DER -passout pass:secret -in ../test.key \
-v2 rc2 -v2prf hmacWithSHA1 -out key_pkcs8_pbkdf2_sha1_rc2_cbc.der

But cannot load a key created like this:

openssl pkcs8 -topk8 -inform PEM -outform DER -passout pass:secret -in ../test.key \
-v2 rc2 -v2prf hmacWithSHA256 -out key_pkcs8_pbkdf2_sha256_rc2_cbc.der

Looks like we **can** load a key created like this: ``` openssl pkcs8 -topk8 -inform PEM -outform DER -passout pass:secret -in ../test.key \ -v2 rc2 -v2prf hmacWithSHA1 -out key_pkcs8_pbkdf2_sha1_rc2_cbc.der ``` But **cannot** load a key created like this: ``` openssl pkcs8 -topk8 -inform PEM -outform DER -passout pass:secret -in ../test.key \ -v2 rc2 -v2prf hmacWithSHA256 -out key_pkcs8_pbkdf2_sha256_rc2_cbc.der ```

karel-m commented

2018-10-06 11:30:56 -05:00

(Migrated from github.com)

It looks link we incorrectly handle hmacWithSHA256 as in pbes_decrypt there is arg->type.h = sha1

It looks link we incorrectly handle `hmacWithSHA256 ` as in `pbes_decrypt` there is `arg->type.h = sha1`

sjaeckel commented

2018-10-06 16:05:44 -05:00

(Migrated from github.com)

got it! PBKDF2-params is specified in rfc2898 as follows and we didn't handle the optional keyLength...

   PBKDF2-params ::= SEQUENCE {
       salt CHOICE {
           specified OCTET STRING,
           otherSource AlgorithmIdentifier {{PBKDF2-SaltSources}}
       },
       iterationCount INTEGER (1..MAX),
       keyLength INTEGER (1..MAX) OPTIONAL,
       prf AlgorithmIdentifier {{PBKDF2-PRFs}} DEFAULT
       algid-hmacWithSHA1 }

got it! PBKDF2-params is specified in [rfc2898](https://tools.ietf.org/html/rfc2898) as follows and we didn't handle the optional keyLength... ``` PBKDF2-params ::= SEQUENCE { salt CHOICE { specified OCTET STRING, otherSource AlgorithmIdentifier {{PBKDF2-SaltSources}} }, iterationCount INTEGER (1..MAX), keyLength INTEGER (1..MAX) OPTIONAL, prf AlgorithmIdentifier {{PBKDF2-PRFs}} DEFAULT algid-hmacWithSHA1 } ```

👍 1

Rows
Columns

Improved pkcs8 support #404