SWupdate CORS support
diff mbox series

Message ID 17b4ecbd-4639-4ba6-a0a8-11456071cda6@googlegroups.com
State New
Headers show
Series
  • SWupdate CORS support
Related show

Commit Message

Simon Wamelink May 15, 2019, 9:25 a.m. UTC
Hi,

The current mongoose interface implementation has no CORS support. Even 
worse, when trying to send a OPTIONS request the mongoose server does not 
send any response when build for ARM. Below is a proposed patch to add 
Access-Control-Allow-Origin headers and send a response when doing an 
OPTIONS request at /upload. 

Kind regards,

Simon Wamelink


                mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, 
(int) fus->len);
@@ -427,6 +428,14 @@ static void upload_handler(struct mg_connection *nc, 
int ev, void *p)
                mp->user_data = NULL;
                free(fus);
                break;
+       case MG_EV_HTTP_REQUEST:
+               mg_send_response_line(nc, 200,
+                       "Content-Type: text/plain\r\n"
+                       "Access-Control-Allow-Origin: *\r\n"
+                       "Connection: close");
+               mg_send(nc, "\r\n", 2);
+               nc->flags |= MG_F_SEND_AND_CLOSE;
+               break;
        }
 }

Comments

Stefano Babic May 17, 2019, 9:08 a.m. UTC | #1
Hi Simon,

On 15/05/19 11:25, Simon Wamelink wrote:
> Hi,
> 
> The current mongoose interface implementation has no CORS support. Even
> worse, when trying to send a OPTIONS request the mongoose server does
> not send any response when build for ARM. Below is a proposed patch to
> add Access-Control-Allow-Origin headers and send a response when doing
> an OPTIONS request at /upload. 
> 

The explained issue is not enough for me - I cannot understand which is
the probel that cause. As far as I understand, setting a
Access-Control-Allow-Origin header allow to run (or block) scripts
fetched from another domain as the domain of origin. This could be just
caused if the website on the device is not directly accessed, but via
some kind of proxy. Nevertheless, if this is an issue, why we should add
Access-Control-Allow-Origin: *, that is, allowed from any domain ?

Best regards,
Stefano Babic

> Kind regards,
> 
> Simon Wamelink
> 
> 
> diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
> index 65e7a08..68ff675 100644
> --- a/mongoose/mongoose_interface.c
> +++ b/mongoose/mongoose_interface.c
> @@ -419,6 +419,7 @@ static void upload_handler(struct mg_connection *nc,
> int ev, void *p)
>  
>                 mg_send_response_line(nc, 200,
>                         "Content-Type: text/plain\r\n"
> +                       "Access-Control-Allow-Origin: *\r\n"
>                         "Connection: close");
>                 mg_send(nc, "\r\n", 2);
>                 mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name,
> (int) fus->len);
> @@ -427,6 +428,14 @@ static void upload_handler(struct mg_connection
> *nc, int ev, void *p)
>                 mp->user_data = NULL;
>                 free(fus);
>                 break;
> +       case MG_EV_HTTP_REQUEST:
> +               mg_send_response_line(nc, 200,
> +                       "Content-Type: text/plain\r\n"
> +                       "Access-Control-Allow-Origin: *\r\n"
> +                       "Connection: close");
> +               mg_send(nc, "\r\n", 2);
> +               nc->flags |= MG_F_SEND_AND_CLOSE;
> +               break;
>         }
>  }
> 
> -- 
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+unsubscribe@googlegroups.com
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To post to this group, send email to swupdate@googlegroups.com
> <mailto:swupdate@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.
Simon Wamelink May 17, 2019, 1:31 p.m. UTC | #2
Hi Stefano,

Thanks for you reply. I'll try to understand a bit more what I'm trying to 
do and what is not working. 

For the project I'm working on we have a website used for device 
configuration at port 80. In this website I want to add a flow to update 
the device using swupdate with mongoose running at 8181. This means sending 
a '.swu' image from `ip`:80 to `ip`:8181/upload and is already seen as a 
cross origin. Therefore a preflight request needs to be send. This gives us 
problem 1:

1) Swupdate does not respond when sending a preflight request. To fix this 
I've added the MG_EV_HTTP_REQUEST case.

Now there exists a notion in CORS of a so called simple request. For 
certain types of request a preflight request is not needed. For /upload a 
POST request with content-type multipart/form-data is done which falls 
under the simple requests. As of such the preflight request would not be 
strictly necessary. However using a preflight request means that no 
evenlisteners are register on the XMLHttpRequestUpload object which leads 
to issue 2.

2) It is not possible to get updates of the upload progress using simple 
requests.

Since an entire image can easily take over a minute to upload I would 
prefer to have a progress bar n place so users see that something is 
happening. To make matters worse when uploading an image using simple 
requests, the browser will inspect the Access-Control-Allow-Origin head of 
the response. If the origin is not in this header it will not allow 
javascript to access the response. Let me repeat that, in javascript you 
are not allowed to see the response if you are not in Allow-Origin. What 
this means is that while you get event to indicate that the request 
completed you do not why. It might have been successful, it might have been 
interrupted, it might have encoutered at 5xx or 4xxx. You just don't know. 
Or in short:

3) Reading response headers in javascript is not allowed if your origin is 
not in Access-Control-Allow-Origin.

These three problems are my rational for adding the 
Access-Control-Allow-Origin header to the /upload handler. Now it might 
seem that a '*' is a a too loose option. While it might seem like a better 
fit to add a specific origin this would need to be done on a user-by-user 
basis since the network topology used can differ. Of course a user setting 
could be added for this to make it dependent on the specific situation. The 
main goal of CORS is to protect again CSRF which is a class of 
vulnerabilities where the hosting website places to much trust in the user. 
However swupdate does has no need for explicitly trusting its users. When 
an image is signed and has correct hashes its accepted as correct, 
irregardless of its source. As such I would argue that a loose origin 
requirement would improve user experience without degrading security and 
should be considered the best option in this situation.

Hope this clarifies my problem. Please let me know how you feel about the 
issue.

Kind regards,

Simon Wamelink

On Friday, May 17, 2019 at 11:09:01 AM UTC+2, Stefano Babic wrote:
>
> Hi Simon, 
>
> On 15/05/19 11:25, Simon Wamelink wrote: 
> > Hi, 
> > 
> > The current mongoose interface implementation has no CORS support. Even 
> > worse, when trying to send a OPTIONS request the mongoose server does 
> > not send any response when build for ARM. Below is a proposed patch to 
> > add Access-Control-Allow-Origin headers and send a response when doing 
> > an OPTIONS request at /upload.  
> > 
>
> The explained issue is not enough for me - I cannot understand which is 
> the probel that cause. As far as I understand, setting a 
> Access-Control-Allow-Origin header allow to run (or block) scripts 
> fetched from another domain as the domain of origin. This could be just 
> caused if the website on the device is not directly accessed, but via 
> some kind of proxy. Nevertheless, if this is an issue, why we should add 
> Access-Control-Allow-Origin: *, that is, allowed from any domain ? 
>
> Best regards, 
> Stefano Babic 
>
> > Kind regards, 
> > 
> > Simon Wamelink 
> > 
> > 
> > diff --git a/mongoose/mongoose_interface.c 
> b/mongoose/mongoose_interface.c 
> > index 65e7a08..68ff675 100644 
> > --- a/mongoose/mongoose_interface.c 
> > +++ b/mongoose/mongoose_interface.c 
> > @@ -419,6 +419,7 @@ static void upload_handler(struct mg_connection *nc, 
> > int ev, void *p) 
> >   
> >                 mg_send_response_line(nc, 200, 
> >                         "Content-Type: text/plain\r\n" 
> > +                       "Access-Control-Allow-Origin: *\r\n" 
> >                         "Connection: close"); 
> >                 mg_send(nc, "\r\n", 2); 
> >                 mg_printf(nc, "Ok, %s - %d bytes.\r\n", mp->file_name, 
> > (int) fus->len); 
> > @@ -427,6 +428,14 @@ static void upload_handler(struct mg_connection 
> > *nc, int ev, void *p) 
> >                 mp->user_data = NULL; 
> >                 free(fus); 
> >                 break; 
> > +       case MG_EV_HTTP_REQUEST: 
> > +               mg_send_response_line(nc, 200, 
> > +                       "Content-Type: text/plain\r\n" 
> > +                       "Access-Control-Allow-Origin: *\r\n" 
> > +                       "Connection: close"); 
> > +               mg_send(nc, "\r\n", 2); 
> > +               nc->flags |= MG_F_SEND_AND_CLOSE; 
> > +               break; 
> >         } 
> >  } 
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> > Groups "swupdate" group. 
> > To unsubscribe from this group and stop receiving emails from it, send 
> > an email to swup...@googlegroups.com <javascript:> 
> > <mailto:swup...@googlegroups.com <javascript:>>. 
> > To post to this group, send email to swup...@googlegroups.com 
> <javascript:> 
> > <mailto:swup...@googlegroups.com <javascript:>>. 
> > To view this discussion on the web visit 
> > 
> https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com 
> > <
> https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com?utm_medium=email&utm_source=footer>. 
>
> > For more options, visit https://groups.google.com/d/optout. 
>
>
> -- 
> ===================================================================== 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk 
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany 
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de 
> <javascript:> 
> ===================================================================== 
>
Stefano Babic May 31, 2019, 11:25 a.m. UTC | #3
Hi Simon,

On 17/05/19 15:31, Simon Wamelink wrote:
> Hi Stefano,
> 
> Thanks for you reply. I'll try to understand a bit more what I'm trying
> to do and what is not working. 
> 
> For the project I'm working on we have a website used for device
> configuration at port 80. In this website I want to add a flow to update
> the device using swupdate with mongoose running at 8181. This means
> sending a '.swu' image from `ip`:80 to `ip`:8181/upload and is already
> seen as a cross origin. Therefore a preflight request needs to be send.
> This gives us problem 1:
> 

Wait...this is not enough. How are you trying to reach the goal ? Which
webserver are you running on port 89 ?

I know that both nginx and lighttpd support proxy and requests send to a
specific URL of the main webserver can then redirect to SWUpdate's
webserver. What are you using in your case ?

And after reading your post: if a proxy module is running, does the
browser still a different origin for JS ? Is it not masked by the proxy
webserver (I presumed that) ?

> 1) Swupdate does not respond when sending a preflight request. To fix
> this I've added the MG_EV_HTTP_REQUEST case.
> 
> Now there exists a notion in CORS of a so called simple request. For
> certain types of request a preflight request is not needed. For /upload
> a POST request with content-type multipart/form-data is done which falls
> under the simple requests. As of such the preflight request would not be
> strictly necessary. However using a preflight request means that no
> evenlisteners are register on the XMLHttpRequestUpload object which
> leads to issue 2.
> 
> 2) It is not possible to get updates of the upload progress using simple
> requests.
> 
> Since an entire image can easily take over a minute to upload

Also several minutes - but a progress bar (under the name) is already shown.

> I would
> prefer to have a progress bar n place so users see that something is
> happening. To make matters worse when uploading an image using simple
> requests, the browser will inspect the Access-Control-Allow-Origin head
> of the response. If the origin is not in this header it will not allow
> javascript to access the response. Let me repeat that, in javascript you
> are not allowed to see the response if you are not in Allow-Origin. What
> this means is that while you get event to indicate that the request
> completed you do not why. It might have been successful, it might have
> been interrupted, it might have encoutered at 5xx or 4xxx. You just
> don't know. Or in short:
> 
> 3) Reading response headers in javascript is not allowed if your origin
> is not in Access-Control-Allow-Origin.
> 

ok, that makes generally sense.

> These three problems are my rational for adding the
> Access-Control-Allow-Origin header to the /upload handler. Now it might
> seem that a '*' is a a too loose option.

I do not see issues.

> While it might seem like a
> better fit to add a specific origin this would need to be done on a
> user-by-user basis since the network topology used can differ. Of course
> a user setting could be added for this to make it dependent on the
> specific situation. The main goal of CORS is to protect again CSRF which
> is a class of vulnerabilities where the hosting website places to much
> trust in the user. However swupdate does has no need for explicitly
> trusting its users. When an image is signed and has correct hashes its
> accepted as correct,

Right - and if it is not signed, update runs in a protected environment
and these measures are not needed. I think that '*' is ok for SWUpdate.

> irregardless of its source. As such I would argue
> that a loose origin requirement would improve user experience without
> degrading security and should be considered the best option in this
> situation.
> 
> Hope this clarifies my problem. Please let me know how you feel about
> the issue.

ok - can you also clarify if there are compatibility problems, also if
used together with different browsers ?

Best regards,
Stefano Babic

> On Friday, May 17, 2019 at 11:09:01 AM UTC+2, Stefano Babic wrote:
> 
>     Hi Simon,
> 
>     On 15/05/19 11:25, Simon Wamelink wrote:
>     > Hi,
>     >
>     > The current mongoose interface implementation has no CORS support.
>     Even
>     > worse, when trying to send a OPTIONS request the mongoose server does
>     > not send any response when build for ARM. Below is a proposed
>     patch to
>     > add Access-Control-Allow-Origin headers and send a response when
>     doing
>     > an OPTIONS request at /upload. 
>     >
> 
>     The explained issue is not enough for me - I cannot understand which is
>     the probel that cause. As far as I understand, setting a
>     Access-Control-Allow-Origin header allow to run (or block) scripts
>     fetched from another domain as the domain of origin. This could be just
>     caused if the website on the device is not directly accessed, but via
>     some kind of proxy. Nevertheless, if this is an issue, why we should
>     add
>     Access-Control-Allow-Origin: *, that is, allowed from any domain ?
> 
>     Best regards,
>     Stefano Babic
> 
>     > Kind regards,
>     >
>     > Simon Wamelink
>     >
>     >
>     > diff --git a/mongoose/mongoose_interface.c
>     b/mongoose/mongoose_interface.c
>     > index 65e7a08..68ff675 100644
>     > --- a/mongoose/mongoose_interface.c
>     > +++ b/mongoose/mongoose_interface.c
>     > @@ -419,6 +419,7 @@ static void upload_handler(struct
>     mg_connection *nc,
>     > int ev, void *p)
>     >  
>     >                 mg_send_response_line(nc, 200,
>     >                         "Content-Type: text/plain\r\n"
>     > +                       "Access-Control-Allow-Origin: *\r\n"
>     >                         "Connection: close");
>     >                 mg_send(nc, "\r\n", 2);
>     >                 mg_printf(nc, "Ok, %s - %d bytes.\r\n",
>     mp->file_name,
>     > (int) fus->len);
>     > @@ -427,6 +428,14 @@ static void upload_handler(struct mg_connection
>     > *nc, int ev, void *p)
>     >                 mp->user_data = NULL;
>     >                 free(fus);
>     >                 break;
>     > +       case MG_EV_HTTP_REQUEST:
>     > +               mg_send_response_line(nc, 200,
>     > +                       "Content-Type: text/plain\r\n"
>     > +                       "Access-Control-Allow-Origin: *\r\n"
>     > +                       "Connection: close");
>     > +               mg_send(nc, "\r\n", 2);
>     > +               nc->flags |= MG_F_SEND_AND_CLOSE;
>     > +               break;
>     >         }
>     >  }
>     >
>     > --
>     > You received this message because you are subscribed to the Google
>     > Groups "swupdate" group.
>     > To unsubscribe from this group and stop receiving emails from it,
>     send
>     > an email to swup...@googlegroups.com <javascript:>
>     > <mailto:swup...@googlegroups.com <javascript:>>.
>     > To post to this group, send email to swup...@googlegroups.com
>     <javascript:>
>     > <mailto:swup...@googlegroups.com <javascript:>>.
>     > To view this discussion on the web visit
>     >
>     https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com
>     <https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com>
> 
>     >
>     <https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com?utm_medium=email&utm_source=footer
>     <https://groups.google.com/d/msgid/swupdate/17b4ecbd-4639-4ba6-a0a8-11456071cda6%40googlegroups.com?utm_medium=email&utm_source=footer>>.
> 
>     > For more options, visit https://groups.google.com/d/optout
>     <https://groups.google.com/d/optout>.
> 
> 
>     -- 
>     =====================================================================
>     DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>     HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>     Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email:
>     sba...@denx.de <javascript:>
>     =====================================================================
> 
> -- 
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+unsubscribe@googlegroups.com
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To post to this group, send email to swupdate@googlegroups.com
> <mailto:swupdate@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/cf1fd088-a3cb-4ca8-8499-21af3b3b7236%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/cf1fd088-a3cb-4ca8-8499-21af3b3b7236%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

Patch
diff mbox series

diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
index 65e7a08..68ff675 100644
--- a/mongoose/mongoose_interface.c
+++ b/mongoose/mongoose_interface.c
@@ -419,6 +419,7 @@  static void upload_handler(struct mg_connection *nc, 
int ev, void *p)
 
                mg_send_response_line(nc, 200,
                        "Content-Type: text/plain\r\n"
+                       "Access-Control-Allow-Origin: *\r\n"
                        "Connection: close");
                mg_send(nc, "\r\n", 2);