You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Graham Leggett <mi...@sharp.fm> on 2014/12/01 02:02:54 UTC
[Patch] mod_crypto: A general purpose crypto filter with HLS support
Hi all,
I have attached a proof of concept module that teaches httpd to support symmetrical encryption, initially to support on-the-fly HLS encryption for video streaming.
This requires the apr-crypto-secretkey patch that I just posted to the APR list.
This module also potentially solves a problem like this one: http://serverfault.com/questions/372588/decrypting-aes-files-in-an-apache-module
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Dec 2014, at 12:11 PM, Yann Ylavic <yl...@gmail.com> wrote:
> Sorry, I forgot one question :
> - What is the purpose of the handler returning the secret key?
> Get it through a SSL channel for example?
> At first glance it looks quite scary ;)
In HLS, the key is protected via some kind of login mechanism, and is handed out to authorised people over a secure channel. This handler makes this easy to serve that key, particularly if the key is derived via the expression API.
The key is provided by a hook, so you could extend the module to support rotating keys and other enhancements if the expression API isn’t enough.
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 1, 2014 at 11:03 AM, Yann Ylavic <yl...@gmail.com> wrote:
> A few (first glance) questions :
>
> - The input filter seems to read and return blocksize bytes at once,
> couldn't it read up to readbytes - reabytes % blocksize, or even
> readbytes with retained buckets?
> It seems that buffering (at most blocksize[ - 1]) would benefit the
> output filter too (FLUSH).
>
> - The IV length seems to be forcibly corresponding to the cipher's
> blocksize, this is not applicable to all ciphers though.
>
> - The following is used several times in exec_pass_conf_binary() and
> looks buggy :
>
> + if (len < size) {
> + b = apr_palloc(r->pool, size);
> + memset(b, 0, size - len);
> + [fn](b + size - len, arg, strlen(arg));
> + }
> + else {
> + b = apr_palloc(r->pool, len);
> + [fn](b, arg, strlen(arg), 1,
> + NULL);
> + b += size - len;
>
> size - len is <= 0 here, maybe len - size?
> Also, maybe allocate size bytes only since the first len - size are ignored.
> Finally, when len != size, why not use a key-type passphrase? (that
> would probably better be configurable though).
>
> + }
> + *k = b;
Sorry, I forgot one question :
- What is the purpose of the handler returning the secret key?
Get it through a SSL channel for example?
At first glance it looks quite scary ;)
>
> Regards,
> Yann.
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Graham Leggett <mi...@sharp.fm>.
On 12 Dec 2014, at 12:59 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> Incorporating the above, How about this?
>
> Looks fine, modulo the above (didn't figure out before):
All of this has been included.
I’ve committed this in r1752099 so that further development can occur on trunk.
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Dec 11, 2014 at 3:31 PM, Graham Leggett <mi...@sharp.fm> wrote:
[]
> It has always seemed wrong to me to assume what is in the brigade from an earlier filter (ie that the bucket will be memory resident and therefore non blocking).
Input filters' passed-in brigades usually have the lifetime of the
underlying connection/request.
On the client side, that's not an issue, everything is over when
c->pool is destroyed, and ctx->tmp will never be used.
On the backend side (should this filter be used that way), the proxy
module is supposed take care of the passed-out brigades' lifetime,
since f->r and f->c (hence ctx->tmp) may last shorter than the client
side, still if no one has asked for this data before they are
destroyed, they are lost and won't be used anymore either.
Ideally we should use bb->bucket_alloc (and, as request_rec, something
like bb->baton if it had existed :) ) instead of f->r and
f->c->bucket_alloc for creating ctx->bb/tmp, so that mod_proxy would
not need to setaside/transform buckets' lifetime when forwarding
responses, but that's probably another story...
>
> Incorporating the above, How about this?
Looks fine, modulo the above (didn't figure out before):
+static apr_status_t crypto_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
+ ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
+{
[]
+
+ /* if our buffer is empty, read off the network until the buffer is full */
+ if (APR_BRIGADE_EMPTY(ctx->bb)) {
[]
+
+ while (!ctx->seen_eos && ctx->remaining > 0) {
[]
+ /* if an error was received, bail out now. If the error is
+ * EAGAIN and we have not yet seen an EOS, we will definitely
+ * be called again, at which point we will send our buffered
+ * data. Instead of sending EAGAIN, some filters return an
+ * empty brigade instead when data is not yet available. In
+ * this case, we drop through and pass buffered data, if any.
+ */
+ if (APR_STATUS_IS_EAGAIN(rv)) {
+ if (APR_BRIGADE_EMPTY(ctx->bb)) {
+ return rv;
+ }
+ break;
+ }
Maybe :
if (APR_STATUS_IS_EAGAIN(rv)
|| (sv == APR_SUCCESS
&& block == APR_NONBLOCK_READ
&& APR_BRIGADE_EMPTY(ctx->tmp))) {
if (APR_BRIGADE_EMPTY(ctx->bb)) {
return rv;
}
break;
}
to be consitent with the comment.
+ if (APR_SUCCESS != rv) {
+ return rv;
+ }
+
+ for (e = APR_BRIGADE_FIRST(ctx->tmp);
+ e != APR_BRIGADE_SENTINEL(ctx->tmp);
+ e = APR_BUCKET_NEXT(e)) {
while (!APR_BRIGADE_EMPTY(ctx->tmp)) {
e = APR_BRIGADE_FIRST(ctx->tmp);
since we can't use APR_BUCKET_NEXT(e) after apr_bucket_delete(e) below.
[]
+
+ apr_bucket_delete(e);
+
+ }
+ }
+ }
+
[]
+
+ return rv;
Here we may still have APR_STATUS_IS_EAGAIN(rv), but some buffered
stuff to return, so :
return APR_SUCCESS;
+}
Regards,
Yann.
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Graham Leggett <mi...@sharp.fm>.
On 10 Dec 2014, at 12:22 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>> + rv = ap_get_brigade(f->next, ctx->tmp, mode, block,
>>> ctx->remaining);
>>> +
>
> But if you want to keep the following apr_bucket_read() nonblocking,
> you could do :
>
> if (APR_BRIGADE_EMPTY(ctx->tmp) {
> rv = ap_get_brigade(f->next, ctx->tmp, mode, block, ctx->remaining);
> }
>
> and then below :
>
> rv = apr_bucket_read(e, &data, &size, block);
> if (APR_STATUS_IS_EAGAIN(rv)) {
> if (APR_BRIGADE_EMPTY(ctx->bb) {
> return rv;
> }
> break:
> }
> if (APR_SUCCESS != rv) {
> return rv;
> }
> do_crypto(f, (unsigned char *) data, size, 0);
> ...
It has always seemed wrong to me to assume what is in the brigade from an earlier filter (ie that the bucket will be memory resident and therefore non blocking).
Incorporating the above, How about this?
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 9, 2014 at 11:03 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Dec 9, 2014 at 10:56 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> +static apr_status_t crypto_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
>> + ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
>> +{
> []
>> + /* if our buffer is empty, read off the network until the buffer is full */
>> + if (APR_BRIGADE_EMPTY(ctx->bb)) {
> []
>> + while (!ctx->seen_eos && ctx->remaining > 0) {
>> + const char *data;
>> + apr_size_t size = 0;
>> +
>
> Also I think we should :
> apr_brigade_cleanup(ctx->tmp);
> here.
Hmm, no, this one seems not necessary.
>
>> + rv = ap_get_brigade(f->next, ctx->tmp, mode, block,
>> ctx->remaining);
>> +
But if you want to keep the following apr_bucket_read() nonblocking,
you could do :
if (APR_BRIGADE_EMPTY(ctx->tmp) {
rv = ap_get_brigade(f->next, ctx->tmp, mode, block, ctx->remaining);
}
and then below :
rv = apr_bucket_read(e, &data, &size, block);
if (APR_STATUS_IS_EAGAIN(rv)) {
if (APR_BRIGADE_EMPTY(ctx->bb) {
return rv;
}
break:
}
if (APR_SUCCESS != rv) {
return rv;
}
do_crypto(f, (unsigned char *) data, size, 0);
...
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 9, 2014 at 10:56 PM, Yann Ylavic <yl...@gmail.com> wrote:
> +static apr_status_t crypto_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
> + ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
> +{
[]
> + /* if our buffer is empty, read off the network until the buffer is full */
> + if (APR_BRIGADE_EMPTY(ctx->bb)) {
[]
> + while (!ctx->seen_eos && ctx->remaining > 0) {
> + const char *data;
> + apr_size_t size = 0;
> +
Also I think we should :
apr_brigade_cleanup(ctx->tmp);
here.
> + rv = ap_get_brigade(f->next, ctx->tmp, mode, block,
> ctx->remaining);
> +
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
Hi Graham,
On Tue, Dec 9, 2014 at 5:50 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 01 Dec 2014, at 7:45 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
>> OK, the other way around then (I thought osize was the block size), so
>> it can read more bytes than asked to.
>> This may also be unexpected by the caller...
>>
>> At least for the nonblocking case, I think we should return what we
>> have so far (if any).
>
> Something like this?
Regarding the input filter wrt nonblocking, I'd rather do it this way
(modifications inline) :
+static apr_status_t crypto_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
+ ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
+{
+ apr_bucket *e, *after;
+ apr_status_t rv = APR_SUCCESS;
+ crypto_ctx *ctx = f->ctx;
+
+ if (!ctx->tmp) {
+ ctx->tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+ }
+
+ /* just get out of the way of things we don't want. */
+ if (mode != AP_MODE_READBYTES) {
+ return ap_get_brigade(f->next, bb, mode, block, readbytes);
+ }
+
+ /* if our buffer is empty, read off the network until the buffer is full */
+ if (APR_BRIGADE_EMPTY(ctx->bb)) {
+ ctx->remaining = ctx->osize;
+ ctx->written = 0;
+
+ while (!ctx->seen_eos && ctx->remaining > 0) {
+ const char *data;
+ apr_size_t size = 0;
+
+ rv = ap_get_brigade(f->next, ctx->tmp, mode, block,
ctx->remaining);
+
+ /* if an error was received, bail out now. If the error is
+ * EAGAIN and we have not yet seen an EOS, we will definitely
+ * be called again, at which point we will send our buffered
+ * data. Instead of sending EAGAIN, some filters return an
+ * empty brigade instead when data is not yet available. In
+ * this case, we drop through and pass buffered data, if any.
+ */
+ if (!APR_STATUS_IS_EAGAIN(rv) && APR_SUCCESS != rv) {
+ return rv;
+ }
Here:
if (APR_STATUS_IS_EAGAIN(rv) || APR_BRIGADE_EMPTY(ctx->tmp)) {
if (APR_BRIGADE_EMPTY(ctx->bb)) {
return rv:
}
break;
}
if (APR_SUCCESS != rv) {
return rv;
}
+
+ for (e = APR_BRIGADE_FIRST(ctx->tmp);
+ e != APR_BRIGADE_SENTINEL(ctx->tmp);
+ e = APR_BUCKET_NEXT(e)) {
+
+ /* if we see an EOS, we are done */
+ if (APR_BUCKET_IS_EOS(e)) {
+
+ /* handle any leftovers */
+ do_crypto(f, NULL, 0, 1);
+ apr_brigade_write(ctx->bb, NULL, NULL,
+ (const char *) ctx->out, ctx->written);
+
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ ctx->seen_eos = 1;
+ break;
+ }
+
+ /* flush buckets clear the buffer */
+ if (APR_BUCKET_IS_FLUSH(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ break;
+ }
+
+ /* pass metadata buckets through */
+ if (APR_BUCKET_IS_METADATA(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ continue;
+ }
+
+ /* read the bucket in, pack it into the buffer */
+ rv = apr_bucket_read(e, &data, &size, block);
+ if (APR_SUCCESS == rv || APR_STATUS_IS_EAGAIN(rv)) {
And here instead :
rv = apr_bucket_read(e, &data, &size, APR_BLOCK_READ);
if (APR_SUCCESS == rv) {
since nothing should block (likely not a socket or pipe bucket).
Otherwise we'd have to keep this bucket to restart from it on the next call...
+
+ do_crypto(f, (unsigned char *) data, size, 0);
+ if (!ctx->remaining || APR_STATUS_IS_EAGAIN(rv)) {
+ apr_brigade_write(ctx->bb, NULL, NULL,
+ (const char *) ctx->out, ctx->written);
+ }
+
+ apr_bucket_delete(e);
+ }
+ else {
+ return rv;
+ }
+
+ }
+ }
+ }
+
+ /* give the caller the data they asked for from the buffer */
+ apr_brigade_partition(ctx->bb, readbytes, &after);
+ e = APR_BRIGADE_FIRST(ctx->bb);
+ while (e != after) {
+ if (APR_BUCKET_IS_EOS(e)) {
+ /* last bucket read, step out of the way */
+ ap_remove_input_filter(f);
+ }
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = APR_BRIGADE_FIRST(ctx->bb);
+ }
+
+ /* clear the content length */
+ if (!ctx->clength) {
+ ctx->clength = 1;
+ apr_table_unset(f->r->headers_in, "Content-Length");
+ }
+
+ return rv;
+}
Regards,
Yann.
>
> Regards,
> Graham
> —
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Dec 2014, at 7:45 PM, Yann Ylavic <yl...@gmail.com> wrote:
> OK, the other way around then (I thought osize was the block size), so
> it can read more bytes than asked to.
> This may also be unexpected by the caller...
>
> At least for the nonblocking case, I think we should return what we
> have so far (if any).
Something like this?
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 1, 2014 at 4:08 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 01 Dec 2014, at 12:03 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
>> - The input filter seems to read and return blocksize bytes at once,
>> couldn't it read up to readbytes - reabytes % blocksize, or even
>> readbytes with retained buckets?
>> It seems that buffering (at most blocksize[ - 1]) would benefit the
>> output filter too (FLUSH).
>
> We buffer up to CryptoSize bytes (default: 128k) on both the input and output filters, there is quite a song and dance to get this right.
OK, the other way around then (I thought osize was the block size), so
it can read more bytes than asked to.
This may also be unexpected by the caller...
At least for the nonblocking case, I think we should return what we
have so far (if any).
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Dec 2014, at 12:03 PM, Yann Ylavic <yl...@gmail.com> wrote:
> - The input filter seems to read and return blocksize bytes at once,
> couldn't it read up to readbytes - reabytes % blocksize, or even
> readbytes with retained buckets?
> It seems that buffering (at most blocksize[ - 1]) would benefit the
> output filter too (FLUSH).
We buffer up to CryptoSize bytes (default: 128k) on both the input and output filters, there is quite a song and dance to get this right.
> - The IV length seems to be forcibly corresponding to the cipher's
> blocksize, this is not applicable to all ciphers though.
This is true - the apr_crypto patch has been updated to provide the ivsize separately.
> - The following is used several times in exec_pass_conf_binary() and
> looks buggy :
>
> + if (len < size) {
> + b = apr_palloc(r->pool, size);
> + memset(b, 0, size - len);
> + [fn](b + size - len, arg, strlen(arg));
> + }
> + else {
> + b = apr_palloc(r->pool, len);
> + [fn](b, arg, strlen(arg), 1,
> + NULL);
> + b += size - len;
>
> size - len is <= 0 here, maybe len - size?
Yep, that is definitely wrong.
> Also, maybe allocate size bytes only since the first len - size are ignored.
The various functions used for decoding don’t support arbitrary offsets, we have to decode the whole key/iv, and then take the least significant bits. This is easiest done with a simple move of the start of the key in this case.
> Finally, when len != size, why not use a key-type passphrase? (that
> would probably better be configurable though).
Passphrases are a whole other kettle of fish. You need to supply salts, and the number of iterations for the various algorithms.
I reverse engineered openssl’s passphrase support for symmetrical crypto, and there are a number of hard coded behaviours like a fixed length salt, a specific start constant, etc. Gnupg was different too. Keen to come up with something general enough that any tool could be used on the other side.
I plan to support passphrases, but wanted at the very least for the module to receive some wider public exposure before that happens. HLS is working really nicely, it would be ideal to have a solution out there in the mean time for this only.
Regards,
Graham
—
Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support
Posted by Yann Ylavic <yl...@gmail.com>.
Hi Graham,
nice module, looks very useful.
A few (first glance) questions :
- The input filter seems to read and return blocksize bytes at once,
couldn't it read up to readbytes - reabytes % blocksize, or even
readbytes with retained buckets?
It seems that buffering (at most blocksize[ - 1]) would benefit the
output filter too (FLUSH).
- The IV length seems to be forcibly corresponding to the cipher's
blocksize, this is not applicable to all ciphers though.
- The following is used several times in exec_pass_conf_binary() and
looks buggy :
+ if (len < size) {
+ b = apr_palloc(r->pool, size);
+ memset(b, 0, size - len);
+ [fn](b + size - len, arg, strlen(arg));
+ }
+ else {
+ b = apr_palloc(r->pool, len);
+ [fn](b, arg, strlen(arg), 1,
+ NULL);
+ b += size - len;
size - len is <= 0 here, maybe len - size?
Also, maybe allocate size bytes only since the first len - size are ignored.
Finally, when len != size, why not use a key-type passphrase? (that
would probably better be configurable though).
+ }
+ *k = b;
Regards,
Yann.
On Mon, Dec 1, 2014 at 2:02 AM, Graham Leggett <mi...@sharp.fm> wrote:
> Hi all,
>
> I have attached a proof of concept module that teaches httpd to support symmetrical encryption, initially to support on-the-fly HLS encryption for video streaming.
>
> This requires the apr-crypto-secretkey patch that I just posted to the APR list.
>
> This module also potentially solves a problem like this one: http://serverfault.com/questions/372588/decrypting-aes-files-in-an-apache-module
>
> Regards,
> Graham
> —