[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squeak-vm-dev
Subject:    Re: [Vm-dev] SqueakSSL plugin issue
From:       Tobias Pape <Das.Linux () gmx ! de>
Date:       2021-07-27 18:05:38
Message-ID: CF4880DC-1A12-4352-BA9C-EBBAC5141557 () gmx ! de
[Download RAW message or body]

 


> On 27. Jul 2021, at 16:02, Eliot Miranda <eliot.miranda@gmail.com> wrote:
> 
> 
> Hi Levente,
> 
> > On Jul 27, 2021, at 4:12 AM, Levente Uzonyi <leves@caesar.elte.hu> wrote:
> > 
> > Hi All,
> > 
> > I ran into a case where primitiveEncrypt of the SqueakSSL plugin returns -1.
> > On the image side, SqueakSSL >> #primitiveSSL:encrypt:startingAt:count:into: is \
> > sent with srcBuf and dstBuf being two 4096 long ByteArrays, start is 1 and length \
> > is 4096, so the input has the same size as the output, and all the bytes are to \
> > be encrypted.
> > I enabled logging, and the following two lines were produced:
> > 
> > sqEncryptSSL: Encrypting 4096 bytes
> > sqCopyBioSSL: 4125 bytes pending; buffer size 4096
> > 
> > Somehow, the 4096 bytes became 4125 bytes, which may be normal, but the
> > code clearly expects no more than 4096 bytes to be there[1].
> > The hardcoded -1 is fishy on its own, and perhaps BIO_read is able to
> > handle the situation and read only as many bytes that fit into the output
> > buffer.
> > If so, the guard is unnecessary, but I'm not familiar with the code at
> > all, so I don't know what's the correct thing to do.
> > 
> > I can work it around by providing smaller chunks or larger buffers but I
> > think it would be forth fixing this.
> > Any ideas?
> 
> One style we use in Terf for video codecs is to supply a callback with the input to \
> receive the output. For example the x264 encode primitive takes a pointer to the \
> x264 encoder state, a bits array containing the video frame, and a callback that \
> receives a pointer to the encoded packet (when one is ready) and it's length.  Then \
> the resulting packet byte array is allocated by Smalltalk and initialized from the \
> pointer.

thats more or less how it works atm, the buffers are all smalltalk buffers.
but with kind-of polling on the image side.

> 
> This approach makes SSL depend on either Alien or the FFI (& should look to using \
> the FFI facilities going forward).  But call backs are available in all our \
> platforms now.  I did ARMv8 a couple of weeks ago.

What happens ist that the sqo_SSL_write of the unencrypted data to the ssl object \
encrypts them and the output apparently growth. sqo_BIO_ctrl_pending shows that there \
are now more bytes available to read. than we wrote. 

The combinations of write/read and buffers is as it was in Andreas' original version.

I can just presume that all previous ciphers used never increased in size.
Probably HMAC or something adds to that problem.

We shouldn't just return -1 but ask the provider for a bigger buffeR:

	SQSSL_BUFFER_TOO_SMALL
	https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/d494240736f7c0309e3e819784feb1d53ed0985a/platforms/Cross/plugins/SqueakSSL/SqueakSSL.h#L31


this is what windows already does:

	https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/d494240736f7c0309e3e819784feb1d53ed0985a/platforms/win32/plugins/SqueakSSL/sqWin32SSL.c#L771



Best regards
	-Tobias



> 
> > 
> > 
> > Levente
> > 
> > [1] https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/d494240736f7c0309e3e819784feb1d53ed0985a/platforms/unix/plugins/SqueakSSL/sqUnixOpenSSL.inc#L70
> > 


[prev in list] [next in list] [prev in thread] [next in thread]