On Tue, Feb 5, 2019 at 5:38 PM Dmitry Kasatkin
<dmitry.kasatkin@xxxxxxxxx> wrote:
>
> On Wed, Jan 30, 2019 at 5:12 AM Vitaly Chikunov <vt@xxxxxxxxxxxx> wrote:
> >
> > On Mon, Jan 28, 2019 at 08:11:53PM +0300, Vitaly Chikunov wrote:
> > > Convert sign_v2 and related to using EVP_PKEY API instead of RSA API.
> > > This enables more signatures to work out of the box.
> > >
> > > Only in single instance GOST NIDs are checked to produce correct keyid.
> > > Other than that code is quite generic.
> >
> > There is was to generalize it a bit more.
> >
> > > Remove RSA_ASN1_templates[] as it does not needed anymore. OpenSSL sign
> > > is doing proper PKCS1 padding automatically (tested to be compatible
> > > with previous version, except for MD4). This also fixes bug with MD4
> > > which produced wrong signature because of absence of the appropriate
> > > RSA_ASN1_template.
> > >
> > > Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> > > ---
> > > src/evmctl.c | 25 +++---
> > > src/imaevm.h | 4 +-
> > > src/libimaevm.c | 271 +++++++++++++++++++++++++++-----------------------------
> > > 3 files changed, 146 insertions(+), 154 deletions(-)
> > >
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index d9ffa13..bd99c60 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -776,16 +724,32 @@ void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len
> > > log_info("keyid-v1: %s\n", str);
> > > }
> > >
> > > -void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key)
> > > +void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *key)
> > > {
> > > + X509_PUBKEY *pk = NULL;
> > > uint8_t sha1[SHA_DIGEST_LENGTH];
> > > - unsigned char *pkey = NULL;
> > > + const unsigned char *pkey = NULL;
> > > + unsigned char *pp = NULL;
> > > int len;
> > >
> > > - len = i2d_RSAPublicKey(key, &pkey);
> > > -
> > > - SHA1(pkey, len, sha1);
> > > + switch (EVP_PKEY_id(key)) {
> > > + case NID_id_GostR3410_2012_256:
> > > + case NID_id_GostR3410_2012_512:
> > > + X509_PUBKEY_set(&pk, key);
> > > + X509_PUBKEY_get0_param(NULL, &pkey, &len, NULL, pk);
> > > + break;
> > > + default:
> > > + len = i2d_PublicKey(key, &pp);
> >
> > Because two calls to X509_PUBKEY_set and X509_PUBKEY_get0_param can
> > handle more keys (including RSA), call to i2d_PublicKey could be
> > avoided, so switch with Gost NIDs could be removed too. Tested.
> >
> > > + pkey = pp;
> > > + }
> > >
> > > + if (len <= 0) {
> > > + ERR_print_errors_fp(stderr);
> > > + /* Produce invalid key in case of error. */
> > > + len = SHA_DIGEST_LENGTH;
> > > + memset(sha1, 0, len);
> > > + } else
> > > + SHA1(pkey, len, sha1);
> > > /* sha1[12 - 19] is exactly keyid from gpg file */
> > > memcpy(keyid, sha1 + 16, 4);
> > > log_debug("keyid: ");
>
>
> I have tested EVM and IMA signature verification with new patch and it
> seems to be OK.
>
> But when I try to sign with new patch, then signing fails...
>
> $ sudo ./src/evmctl -k keys/privkey_ima.pem ima_sign car
> 139794956297792:error:0608C09B:digital envelope
> routines:EVP_PKEY_sign:buffer too small:../crypto/evp/pmeth_fn.c:65:
> evmctl: evmctl.c:601: sign_ima: Assertion `len < sizeof(sig)' failed.
> Aborted
>
> sign_hash() returns -1
>
> any ideas why?
>
> Dmitry
>
I actually found out...
int EVP_PKEY_sign(EVP_PKEY_CTX *ctx,
unsigned char *sig, size_t *siglen,
const unsigned char *tbs, size_t tbslen);
siglen must contain maxim size of the buffer.
In the patch, the value is uninitialized...
So please fix it.
Thanks,
Dmitry
