GOST VKO support in ECDH: multiply by an optional UKM value #10

wolneykien · 2019-10-22T11:17:20Z

Subj.

lumag · 2019-10-22T11:44:00Z

cipher/ecc.c

+      if (DBG_CIPHER)
+        log_printmpi ("UKM: ", salt);
+      _gcry_mpi_rshift (data, data, ukm_blen);
+    }


This UKM format has to be documented somewhere.

Also if salt == 0 then salt = 1 should be implemented.

Ok, I'll add some info to the top encrypt/decrypt comment.

Also if salt == 0 then salt = 1 should be implemented.

Why?

At least you should check that salt != 0 (see RFC 7836).

Added && gcry_mpi_cmp_ui (salt, 0).

lumag · 2019-10-22T11:45:20Z

cipher/ecc.c

+          log_printmpi ("ecc_encrypt public key e", mpi_e);
+        }
+      rc = sexp_build (r_ciph, NULL, "(enc-val(ecdh(s%m)(e%m)))", mpi_s, mpi_e);
+    }


debuging. can this be dropped?

There already are some debug output in the encrypt/decrypt code. New code was added, new debug output was added along with it. What's wrong with that?

lumag · 2019-10-22T11:46:03Z

cipher/ecc.c

                                    sk.E.p, sk.E.a, sk.E.b);

+  /* For GOST extract the UKM value. */
+  if ((flags & PUBKEY_FLAG_GOST) || 0 == strncmp ("GOST", sk.E.name, 4))


It's a bad idea to depend on exact curve name.

strNcmp() isn't an exact comparison but a prefix check.

Do you mean it is necessary to check sk.E.name to be an OID?

No. It is just a bad idea to depend on the curve names. It can be an OID. One can use it with any other curve. Etc.

Dropped strncmp(). Only the flags now.

lumag · 2019-10-22T11:46:26Z

cipher/ecc.c

+  /* For GOST extract the UKM value. */
+  if ((flags & PUBKEY_FLAG_GOST) || 0 == strncmp ("GOST", sk.E.name, 4))
+    {
+      // FIXME: Expect an uncompressed point format 0x04...


C++-style comment.
Also would you like to fix the FIXME?

OK, I'll use /* */ instead.

Probably no, it's just a note to ease debugging due to a possible change of the input point format in the future.

Then it should not be a FIXME

Dropped FIXME.

lumag · 2019-10-22T11:46:42Z

cipher/ecc.c

  if (DBG_CIPHER)
    log_printpnt ("ecc_decrypt    kG", &kG, NULL);

+


extra newlines

Ok, I'll fix it.

* cipher/cipher.c (_gcry_cipher_encrypt): Fix log "cipher_decrypt: ..." to "cipher_encrypt: ...". -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* cipher/rijndael-ppc.c (vec_aligned_ld, vec_load_be, vec_aligned_st) (vec_store_be): Add "r0" to clobber list for load/store instructions. -- Register r0 must not be used for RA input for vector load/store instructions as r0 is not read as register but as value '0'. Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* cipher/rijndael-ppc.c (ALIGNED_LOAD, ALIGNED_STORE, VEC_LOAD_BE) (VEC_STORE_BE): Rewrite. (VEC_BE_SWAP, VEC_LOAD_BE_NOSWAP, VEC_STORE_BE_NOSWAP): New. (PRELOAD_ROUND_KEYS, AES_ENCRYPT, AES_DECRYPT): Adjust to new input parameters for vector load macros. (ROUND_KEY_VARIABLES_ALL, PRELOAD_ROUND_KEYS_ALL) (AES_ENCRYPT_ALL): New. (vec_bswap32_const_neg): New. (vec_aligned_ld, vec_aligned_st, vec_load_be_const): Rename to... (asm_aligned_ls, asm_aligned_st, asm_load_be_const): ...these. (asm_be_swap, asm_vperm1, asm_load_be_noswap) (asm_store_be_noswap): New. (vec_add_uint128): Rename to... (asm_add_uint128): ...this. (asm_xor, asm_cipher_be, asm_cipherlast_be, asm_ncipher_be) (asm_ncipherlast_be): New inline assembly functions with volatile keyword to allow manual instruction ordering. (_gcry_aes_ppc8_setkey, aes_ppc8_prepare_decryption) (_gcry_aes_ppc8_encrypt, _gcry_aes_ppc8_decrypt) (_gcry_aes_ppc8_cfb_enc, _gcry_aes_ppc8_cbc_enc) (_gcry_aes_ppc8_ocb_auth): Update to use new&rewritten helper macros. (_gcry_aes_ppc8_cfb_dec, _gcry_aes_ppc8_cbc_dec) (_gcry_aes_ppc8_ctr_enc, _gcry_aes_ppc8_ocb_crypt) (_gcry_aes_ppc8_xts_crypt): Update to use new&rewritten helper macros; Tune 8-block parallel paths with manual instruction ordering. -- Benchmarks on POWER8 (ppc64le, ~3.8Ghz): Before: AES | nanosecs/byte mebibytes/sec cycles/byte CBC enc | 1.06 ns/B 902.2 MiB/s 4.02 c/B CBC dec | 0.208 ns/B 4585 MiB/s 0.790 c/B CFB enc | 1.06 ns/B 900.4 MiB/s 4.02 c/B CFB dec | 0.208 ns/B 4588 MiB/s 0.790 c/B CTR enc | 0.238 ns/B 4007 MiB/s 0.904 c/B CTR dec | 0.238 ns/B 4009 MiB/s 0.904 c/B XTS enc | 0.492 ns/B 1937 MiB/s 1.87 c/B XTS dec | 0.488 ns/B 1955 MiB/s 1.85 c/B OCB enc | 0.243 ns/B 3928 MiB/s 0.922 c/B OCB dec | 0.247 ns/B 3858 MiB/s 0.939 c/B OCB auth | 0.213 ns/B 4482 MiB/s 0.809 c/B After (cbc-dec & cfb-dec & xts & ocb ~6% faster, ctr ~11% faster): AES | nanosecs/byte mebibytes/sec cycles/byte CBC enc | 1.06 ns/B 902.1 MiB/s 4.02 c/B CBC dec | 0.196 ns/B 4877 MiB/s 0.743 c/B CFB enc | 1.06 ns/B 902.2 MiB/s 4.02 c/B CFB dec | 0.195 ns/B 4889 MiB/s 0.741 c/B CTR enc | 0.214 ns/B 4448 MiB/s 0.815 c/B CTR dec | 0.214 ns/B 4452 MiB/s 0.814 c/B XTS enc | 0.461 ns/B 2067 MiB/s 1.75 c/B XTS dec | 0.456 ns/B 2092 MiB/s 1.73 c/B OCB enc | 0.227 ns/B 4200 MiB/s 0.863 c/B OCB dec | 0.234 ns/B 4072 MiB/s 0.890 c/B OCB auth | 0.207 ns/B 4604 MiB/s 0.787 c/B Benchmarks on POWER9 (ppc64le, ~3.8Ghz): Before: AES | nanosecs/byte mebibytes/sec cycles/byte CBC enc | 1.04 ns/B 918.7 MiB/s 3.94 c/B CBC dec | 0.240 ns/B 3982 MiB/s 0.910 c/B CFB enc | 1.04 ns/B 917.6 MiB/s 3.95 c/B CFB dec | 0.241 ns/B 3963 MiB/s 0.914 c/B CTR enc | 0.249 ns/B 3835 MiB/s 0.945 c/B CTR dec | 0.252 ns/B 3787 MiB/s 0.957 c/B XTS enc | 0.505 ns/B 1889 MiB/s 1.92 c/B XTS dec | 0.495 ns/B 1926 MiB/s 1.88 c/B OCB enc | 0.303 ns/B 3152 MiB/s 1.15 c/B OCB dec | 0.305 ns/B 3129 MiB/s 1.16 c/B OCB auth | 0.265 ns/B 3595 MiB/s 1.01 c/B After (cbc-dec & cfb-dec ~6% faster, ctr ~11% faster, ocb ~4% faster): AES | nanosecs/byte mebibytes/sec cycles/byte CBC enc | 1.04 ns/B 917.3 MiB/s 3.95 c/B CBC dec | 0.225 ns/B 4234 MiB/s 0.856 c/B CFB enc | 1.04 ns/B 917.8 MiB/s 3.95 c/B CFB dec | 0.226 ns/B 4214 MiB/s 0.860 c/B CTR enc | 0.221 ns/B 4306 MiB/s 0.842 c/B CTR dec | 0.223 ns/B 4271 MiB/s 0.848 c/B XTS enc | 0.503 ns/B 1897 MiB/s 1.91 c/B XTS dec | 0.495 ns/B 1928 MiB/s 1.88 c/B OCB enc | 0.288 ns/B 3309 MiB/s 1.10 c/B OCB dec | 0.292 ns/B 3266 MiB/s 1.11 c/B OCB auth | 0.267 ns/B 3570 MiB/s 1.02 c/B Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

-- Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

* cipher/ecc-curves.c (domain_parms): Add sm2p256v1 for SM2. * tests/curves.c (N_CURVES): Update N_CURVES for SM2. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* cipher/ecc.c (ecc_generate): Fix wrong flag and elements_enc. -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* cipher/ecc-curves.c (mpi_ec_get_elliptic_curve): Initialize E->G poing -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* doc/gcrypt.texi: Fix GCRYCTL_GET_ALGO_NENC to GCRYCTL_GET_ALGO_NENCR. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* configure.ac (HAVE_ULONG_TYPEDEF): Remove. * mpi/mpi-div.c (_gcry_mpi_fdiv_r_ui): Use unsigned long. (_gcry_mpi_divisible_ui): Likewise. * random/rndunix.c (_gcry_rndunix_gather_random): Likewise. * random/rndw32.c (_gcry_rndw32_gather_random_fast): Likewise. (ADDINT): Likewise. * random/rndw32ce.c (_gcry_rndw32ce_gather_random_fast): Likewise. * src/mpi.h: Follow the change. * src/types.h (HAVE_ULONG_TYPEDEF): Remove. Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

* tests/basic.c (check_pubkey): Fix constants of pubkeys. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> Co-authored-by: NIIBE Yutaka <gniibe@fsij.org>

* cipher/ecc-gost.c (_gcry_ecc_gost_sign): Use implemented function. * cipher/ecc.c (ecc_verify): Remove redundant code. -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* configure.ac (enabled_pubkey_ciphers): Add ecc-sm2. * cipher/Makefile.am (EXTRA_libcipher_la_SOURCES): Add ecc-sm2.c. * cipher/pubkey-util.c (_gcry_pk_util_parse_flaglist, _gcry_pk_util_preparse_sigval): Add sm2 flags. * cipher/ecc.c: Support ecc-sm2. * cipher/ecc-common.h: Add declarations for ecc-sm2. * cipher/ecc-sm2.c: New. * src/cipher.h: Define PUBKEY_FLAG_SM2. -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* tests/basic.c (check_pubkey): Add test cases for ecc-sm2. -- Original change was modified by gniibe to limit only for ECDSA. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

* cipher/cipher-gcm-armv8-aarch64-ce.S (_gcry_ghash_setup_armv8_ce_pmull): Set vZZ to zero. -- Reported by "Marvin W." at https://dev.gnupg.org/D497: > > The register vZZ.16b is expected to be always 0 throughout the macros > in cipher/cipher-gcm-armv8-aarch64-ce.S. The PMUL_128x128 and REDUCTION > macros are used in gcry_ghash_setup_armv8_ce_pmull function, however that > function does not set vZZ.16b to zero. If previous use left `vZZ.16b > non-zero before gcry_ghash_setup_armv8_ce_pmull is called, this will cause > invalid GCM auth tag results. > > The patch resets vZZ.16b to 0 at the beginning of > gcry_ghash_setup_armv8_ce_pmull. > [jk: from differential web-ui to commit] Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* src/global.c (_gcry_check_version): Fix missing newline. * src/basic.c (ALWAYS_INLINE, CLUTTER_REGISTER_*, prepare_vector_data) (clutter_vector_registers): New. (progress_handler): Make static function. (check_bulk_cipher_modes, check_one_cipher_core_reset) (check_one_cipher_core, check_one_md, check_one_md_multi) (check_one_md_final, check_one_mac): Clutter vector registers before gcry_* calls to cipher/md/mac algorithms. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

-- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* configure.ac: Include <cet.h> in <config.h> for assembly codes. -- When Intel CET is enabled, include <cet.h> in <config.h> for assembly codes to mark Intel CET support. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

* mpi/config.links: Include <cet.h> in <asm-syntax.h>. -- When Intel CET is enabled, include <cet.h> in <asm-syntax.h> for assembly codes to mark Intel CET support. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

* cipher/camellia-aesni-avx-amd64.S: Always include <config.h>. * cipher/camellia-aesni-avx2-amd64.S: Likewise. * cipher/serpent-avx2-amd64.S: Likewise. -- When Intel CET is enabled, we need to include <cet.h> in assembly codes to mark Intel CET support even if it is empty. We should always include <config.h> in cipher amd64 assembly codes so that they will be marked for Intel CET support when compiling for i686. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

* mpi/i386/mpih-add1.S (_gcry_mpih_add_n): Save and restore %ebx if IBT is enabed. Add _CET_ENDBR to indirect jump targets and adjust jump destination for _CET_ENDBR. * mpi/i386/mpih-sub1.S (_gcry_mpih_sub_n): Likewise. -- i386 mpih-add1.S and mpih-sub1.S use a trick to implment jump tables with LEA. We can't use conditional branches nor normal jump tables since jump table entries use EFLAGS set by jump table index. This patch adds _CET_ENDBR to indirect jump targets and adjust destination for _CET_ENDBR. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

* mpi/i386/mpih-add1.S (_gcry_mpih_add_n) [PIC]: Adjust CFI CFA offset when making call and restoring stack pointer. * mpi/i386/mpih-sub1.S (_gcry_mpih_sub_n) [PIC]: Ditto. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* src/sexp.c (do_vsexp_sscan): Change 'datalen' from 'int' to 'size_t'; Remove &datalen pointer cast to 'size_t *' type. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

* random/random-drbg.c: Include config.h earlier. -- GnuPG-bug-id: 4818 Reported-by: Bruno Haible Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

lumag requested changes Oct 22, 2019

View reviewed changes

wolneykien requested a review from lumag November 13, 2019 12:33

lumag force-pushed the devel3 branch from 8fe212b to df56836 Compare November 24, 2019 12:27

jkivilin and others added 24 commits December 22, 2019 16:52

cipher: fix typo in error log

5b9ea3d

* cipher/cipher.c (_gcry_cipher_encrypt): Fix log "cipher_decrypt: ..." to "cipher_encrypt: ...". -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

Register DCO for Tianjia Zhang.

f34de41

-- Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

Add new curve named sm2p256v1.

d154c1e

* cipher/ecc-curves.c (domain_parms): Add sm2p256v1 for SM2. * tests/curves.c (N_CURVES): Update N_CURVES for SM2. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Update .gitignore

176a5f1

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

ecc: Wrong flag and elements_enc fix.

43cfc16

* cipher/ecc.c (ecc_generate): Fix wrong flag and elements_enc. -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

mpi: Fix error that point not uninitialized

7e3aac7

* cipher/ecc-curves.c (mpi_ec_get_elliptic_curve): Initialize E->G poing -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

gcrypt.texi: fix GCRYCTL_GET_ALGO_NENCR typo

5ebb2f0

* doc/gcrypt.texi: Fix GCRYCTL_GET_ALGO_NENC to GCRYCTL_GET_ALGO_NENCR. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

tests: Fix check_pubkey.

95e9cee

* tests/basic.c (check_pubkey): Fix constants of pubkeys. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> Co-authored-by: NIIBE Yutaka <gniibe@fsij.org>

ecc: Simplify signature code

8d99589

* cipher/ecc-gost.c (_gcry_ecc_gost_sign): Use implemented function. * cipher/ecc.c (ecc_verify): Remove redundant code. -- Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

tests: Add basic test cases for sm2

aa9c78a

* tests/basic.c (check_pubkey): Add test cases for ecc-sm2. -- Original change was modified by gniibe to limit only for ECDSA. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Register DCO for H.J. Lu

8ebbd85

-- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

x86: Add .note.gnu.property section for Intel CET

4c88c2b

* configure.ac: Include <cet.h> in <config.h> for assembly codes. -- When Intel CET is enabled, include <cet.h> in <config.h> for assembly codes to mark Intel CET support. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

mpi: Add .note.gnu.property section for Intel CET

24b4d5c

* mpi/config.links: Include <cet.h> in <asm-syntax.h>. -- When Intel CET is enabled, include <cet.h> in <asm-syntax.h> for assembly codes to mark Intel CET support. Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

sexp: fix cast from 'int' pointer to 'size_t' pointer

8b31091

* src/sexp.c (do_vsexp_sscan): Change 'datalen' from 'int' to 'size_t'; Remove &datalen pointer cast to 'size_t *' type. -- Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

random: Fix include of config.h.

e0898d0

* random/random-drbg.c: Include config.h earlier. -- GnuPG-bug-id: 4818 Reported-by: Bruno Haible Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

wolneykien force-pushed the devel/vko branch from 7767722 to 0aa766f Compare February 4, 2020 14:16

wolneykien added 2 commits February 4, 2020 17:21

GOST VKO support in ECDH: multiply by an optional UKM value

dee702e

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

Updated the top encrypt/decrypt comment with info about salt values

f332185

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

wolneykien added 5 commits February 4, 2020 17:21

Fixed the comment format

14b085b

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

Removed extra newlines

77ab7d3

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

Additional check that salt value isn't 0

3beb55f

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

Fix: Detect GOST mode by the flags only in ecc_decrypt_raw()

c936d71

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

Fix: The note about the uncompressed point is not a real FIXME

6a31820

Signed-off-by: Paul Wolneykien <manowar@altlinux.org>

wolneykien force-pushed the devel/vko branch from 0aa766f to 6a31820 Compare February 4, 2020 14:22

GOST VKO support in ECDH: multiply by an optional UKM value #10

Are you sure you want to change the base?

GOST VKO support in ECDH: multiply by an optional UKM value #10

Uh oh!

Conversation

wolneykien commented Oct 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolneykien Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wolneykien Nov 13, 2019 •

edited

Loading