[Spice-devel] [PATCH] Check RSA_generate_key_ex return value

[Christophe Fergeau]

On Tue, Mar 04, 2014 at 12:16:50PM -0500, Marc-André Lureau wrote:
> ack
> 
> ----- Original Message -----
> > This can fail in fips mode for example. If we ignore the failure, we'll get
> > a crash:
> > 
> >  #0  0x00007f38d63728a0 in BN_num_bits () from /lib64/libcrypto.so.10
> >  #1  0x00007f38d639661d in RSA_size () from /lib64/libcrypto.so.10
> >  #2  0x00007f38d7991762 in reds_handle_read_link_done () from
> >  /lib64/libspice-server.so.1
> >  #3  0x00007f38d7990c06 in spice_server_add_client () from
> >  /lib64/libspice-server.so.1
> >  #4  0x00007f38d7990c6a in reds_accept () from /lib64/libspice-server.so.1
> >  #5  0x00007f38dc0d2946 in qemu_iohandler_poll (pollfds=0x7f38dedce200,
> >  ret=755449965, ret at entry=1) at iohandler.c:143
> >  #6  0x00007f38dc0d6ea8 in main_loop_wait (nonblocking=<optimized out>) at
> >  main-loop.c:465
> >  #7  0x00007f38dbffd7c0 in main_loop () at vl.c:1988
> >  #8  main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> >  at vl.c:4357
> > 
> > This commit will cause the client connection to fail but qemu won't
> > segfault.
> > ---
> >  server/reds.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index c38fa40..212207b 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -1368,8 +1368,15 @@ static int reds_send_link_ack(RedLinkInfo *link)
> >          return FALSE;
> >      }
> >  
> > -    RSA_generate_key_ex(link->tiTicketing.rsa, SPICE_TICKET_KEY_PAIR_LENGTH,
> > link->tiTicketing.bn,
> > -                        NULL);
> > +    if (RSA_generate_key_ex(link->tiTicketing.rsa,
> > +                            SPICE_TICKET_KEY_PAIR_LENGTH,
> > +                            link->tiTicketing.bn,
> > +                            NULL) != 1) {
> > +        spice_warning("Failed to generate %d bits RSA key: %s",
> > +                      SPICE_TICKET_KEY_PAIR_LENGTH,
> > +                      ERR_error_string(ERR_get_error(), NULL));
> > +        return FALSE;

This leaks 'bio' in error cases, I'll send a v2.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140305/11a57a05/attachment.pgp>