[FFmpeg-cvslog] Add a protocol handler for AES CBC decryption with PKCS7 padding

[Reimar Döffinger]

On Sun, Apr 24, 2011 at 07:41:53PM +0300, Martin Storsjö wrote:
> On Sun, 24 Apr 2011, Reimar Döffinger wrote:
> 
> > On Sun, Apr 24, 2011 at 03:50:08AM +0200, Martin Storsjö wrote:
> > > +    // We avoid using the last block until we've found EOF,
> > > +    // since we'll remove PKCS7 padding at the end. So make
> > > +    // sure we've got at least 2 blocks, so we can decrypt
> > > +    // at least one.
> > > +    while (c->indata - c->indata_used < 2*BLOCKSIZE) {
> > > +        int n = ffurl_read(c->hd, c->inbuffer + c->indata,
> > > +                           sizeof(c->inbuffer) - c->indata);
> > > +        if (n <= 0) {
> > > +            c->eof = 1;
> > 
> > Huh? That doesn't necessarily indicate EOF to my knowledge.
> > Of course doing it correct means a risk of very high CPU usage,
> > so this would have to copy or reuse retry_transfer_wrapper
> > from avio.c
> 
> Hmm, EAGAIN should be handled internally within ffurl_read at least, so 
> the rest would only be to distinguish between "real errors", AVERROR_EOF 
> and 0. Not distinguishing between these only means a few byts too little 
> may be returned at a non-EOF error, when interpreted as padding.

I missed that ffurl_read was changed, it should be ok then
(though it does duplicate some code that exists already in
retry_transfer_wrapper).

> > > +    if (c->indata_used >= sizeof(c->inbuffer)/2) {
> > > +        memmove(c->inbuffer, c->inbuffer + c->indata_used,
> > > +                c->indata - c->indata_used);
> > 
> > Why memmove? I see no way those memory ranges could overlap.
> 
> Hmm, yes, that's right, currently they can't overlap. I still prefer 
> memmove for clarity, since it's a move of data within the same buffer, and 
> if the condition for moving the data is adjusted, it could overlap.

Well, I thought it rather confusing. In principle the whole copying
should be avoidable by using it more ringbuffer-like but that's
not very important I guess.

> > > +    if (c->hd)
> > > +        ffurl_close(c->hd);
> > > +    av_free(c->aes);
> > > +    av_free(c->key);
> > > +    av_free(c->iv);
> > 
> > Why not av_freep?
> 
> No particular reason, but since this is the close function, we shouldn't 
> have to worry about it being called multiple time, as far as I know. Sure 
> it would be even more robust with av_freep though.

The point of av_freep is also to not leave pointers lying around,
they are no help and at best make it easier to run exploits.