On Tue, Oct 20, 2015 at 11:27 PM, Gayan Gunarathne <gay...@wso2.com> wrote: > > >> >>> Regarding table names in SQL queries; this is not the best approach to >>> design the API but these table names are escaped from request parameters >>> [2] which would minimize the risk of a SQL injection attack. This is >>> definitely a potential security issue as well as an API design issue we >>> need to fix. But I think fixing this will need a major refactoring to the >>> Jaggery files. wdyt? >>> >> >> No, we can simply fix this by creating an API per table/entity. >> > > You mean to have separate API for each table in the database? So if there > 10 tables , there will be more than 10 APIs. I think it wont work when > there are more no of tables. >
We already have separate APIs for each entity. Yes may be we can have one API implementation which could internally identify the entity based on the context path (similar to Java RESTful services). On Tue, Oct 20, 2015 at 11:33 PM, Akila Ravihansa Perera <raviha...@wso2.com > wrote: > > > SSO can be used to authenticate the user who is accessing the > monitoring/metering dashboard, thereby authenticating him to the API as > well. We don't need to maintain a session token/cookie for this which is an > overhead for this scenario, IMO. > > Yes that's true but in a such design any other external system will not be able to to access the APIs using API Keys without having to integrate with the same SSO solution. Imagine Twitter has exposed set of APIs using Twitter SSO solution. Now if we were to consume those APIs in a X system. This X system needs to integrate with Twitter SSO solution first. IMO this is not a good design, ideally those APIs should be accessible with API Keys. This is how actually Twitter has exposed their APIs. > >> > If we are using basic auth, the user needs to submit the credentials via a > form, which is again an overhead. Any implementation of > authToken/JSESSIONID would require the user to submit credential at some > layer. What I'm suggesting is to authenticate the user via SSO to the DAS > dashboard, which would authenticate him to the API as well. I'm -1 on > developing a separate login screen for this. > > May be this was not clear. We do not need a login page to make a request to an API endpoint which is secured with Basic Auth. We can make a HTTP call with an Authorization header having credentials (Base64 encoded) via HTTPS. > >> > This would not be manageable as we add entities in future I think. We can > handle this internally by not using the request parameter variable to > create the query. But rather choose the correct sql query conditionally > based on the request parameter. This would basically result in rewriting > all Jaggery files. > Yes ideally it should be the context path (/api/member -> member table, /api/cluster/health -> cluster health table, etc) Thanks On Tue, Oct 20, 2015 at 11:40 PM, Akila Ravihansa Perera <raviha...@wso2.com > wrote: > Hi Gayan, > > The code reference you pointed at [1] is used in a prepared SQL statement > [2]. The risk of SQL injection attack is minimal in that case, AFAIU. > > [1] > https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L49 > [2] > https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L53 > > Thanks. > > On Tue, Oct 20, 2015 at 11:33 PM, Akila Ravihansa Perera < > raviha...@wso2.com> wrote: > >> Hi Imesh, >> >> Please see inline. >> >> On Tue, Oct 20, 2015 at 11:08 PM, Imesh Gunaratne <im...@wso2.com> wrote: >> >>> On Tue, Oct 20, 2015 at 10:25 PM, Akila Ravihansa Perera < >>> raviha...@wso2.com> wrote: >>>> >>>> >>>> I think the proper way to secure the Jaggery services is by using SSO. >>>> >>> >>> I tend to disagree on this statement. SSO is used when authenticating a >>> human to a series of software systems. An API should not use SSO for >>> authentication rather it should use session based authentication either by >>> creating session tokens or API Keys, refer this [3]. >>> >> >> SSO can be used to authenticate the user who is accessing the >> monitoring/metering dashboard, thereby authenticating him to the API as >> well. We don't need to maintain a session token/cookie for this which is an >> overhead for this scenario, IMO. >> >> >>> >>> >>>> According to the thread on wso2dev@ with subject "SingleSignOn support >>>> in DAS Analytics Dashboard" this is not yet supported in DAS. The approach >>>> taken in analytics.jsg as you mentioned require a separate login screen as >>>> in [1]. IMHO, this is not a suitable method to secure a Jaggery based API. >>>> >>>> No, have a look at the analytics.jag authentication logic. It first >>> accepts an Authorization header and creates a session token. Authorization >>> header can accept basic auth, see [4]. Afterwards corresponding calls are >>> authenticated using authToken/JSESSIONID. >>> >> >> If we are using basic auth, the user needs to submit the credentials via >> a form, which is again an overhead. Any implementation of >> authToken/JSESSIONID would require the user to submit credential at some >> layer. What I'm suggesting is to authenticate the user via SSO to the DAS >> dashboard, which would authenticate him to the API as well. I'm -1 on >> developing a separate login screen for this. >> >> >>> >>> >>>> Regarding table names in SQL queries; this is not the best approach to >>>> design the API but these table names are escaped from request parameters >>>> [2] which would minimize the risk of a SQL injection attack. This is >>>> definitely a potential security issue as well as an API design issue we >>>> need to fix. But I think fixing this will need a major refactoring to the >>>> Jaggery files. wdyt? >>>> >>> >>> No, we can simply fix this by creating an API per table/entity. >>> >> >> This would not be manageable as we add entities in future I think. We can >> handle this internally by not using the request parameter variable to >> create the query. But rather choose the correct sql query conditionally >> based on the request parameter. This would basically result in rewriting >> all Jaggery files. >> >> Thanks. >> >> >> >>> >>>> [1] >>>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/theme/templates/login.jag >>>> [2] >>>> https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L26 >>>> >>>> [3] https://www.owasp.org/index.php/REST_Security_Cheat_Sheet >>> [4] >>> https://github.com/wso2/carbon-analytics/blob/d7d4f7c31981eb6aff8921fefba7c030eb11a80a/components/analytics-io/org.wso2.carbon.analytics.jsservice/src/main/java/org/wso2/carbon/analytics/jsservice/Utils.java#L352 >>> >>> Thanks >>> >>> On Tue, Oct 20, 2015 at 10:25 PM, Akila Ravihansa Perera < >>> raviha...@wso2.com> wrote: >>> >>>> Hi Imesh, >>>> >>>> I think the proper way to secure the Jaggery services is by using SSO. >>>> According to the thread on wso2dev@ with subject "SingleSignOn support >>>> in DAS Analytics Dashboard" this is not yet supported in DAS. The approach >>>> taken in analytics.jsg as you mentioned require a separate login screen as >>>> in [1]. IMHO, this is not a suitable method to secure a Jaggery based API. >>>> >>>> Regarding table names in SQL queries; this is not the best approach to >>>> design the API but these table names are escaped from request parameters >>>> [2] which would minimize the risk of a SQL injection attack. This is >>>> definitely a potential security issue as well as an API design issue we >>>> need to fix. But I think fixing this will need a major refactoring to the >>>> Jaggery files. wdyt? >>>> >>>> [1] >>>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/theme/templates/login.jag >>>> [2] >>>> https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L26 >>>> >>>> Thanks. >>>> >>>> On Tue, Oct 20, 2015 at 8:38 PM, Imesh Gunaratne <im...@wso2.com> >>>> wrote: >>>> >>>>> Hi, >>>>> >>>>> It looks like there are two security issues in the APIs exposed by the >>>>> DAS metering and monitoring dashboards [1], [2]: >>>>> >>>>> - APIs have no authentication mechanism >>>>> - Table name is concatenated in the SQL queries >>>>> >>>>> We may need to add an authentication check similar to analytics.jag >>>>> [3]: >>>>> >>>>> var authParam = request.getHeader(AUTHORIZATION_HEADER); >>>>> if (authParam != null) { >>>>> credentials = JSUtils.authenticate(authParam); >>>>> authenticationAdminStub = new >>>>> AuthenticationAdminStub(authenticationWSUrl); >>>>> authenticationAdminStub.login(credentials[0], credentials[1], >>>>> LOCALHOST); >>>>> var serviceContext = >>>>> authenticationAdminStub._getServiceClient().getLastOperationContext() >>>>> .getServiceContext(); >>>>> var sessionCookie = >>>>> serviceContext.getProperty(HTTPConstants.COOKIE_STRING); >>>>> options.setProperty(HTTPConstants.COOKIE_STRING, >>>>> sessionCookie); >>>>> } else { >>>>> var token = session.get(AUTH_TOKEN); >>>>> if (token != null) { >>>>> options.setProperty(HTTPConstants.COOKIE_STRING, token); >>>>> } else { >>>>> log.error("user is not authenticated!"); >>>>> response.status = HTTP_USER_NOT_AUTHENTICATED; >>>>> print('{ "status": "Failed", "message": "User is not >>>>> authenticated." }'); >>>>> return; >>>>> } >>>>> } >>>>> >>>>> In addition we may need to avoid concatenating table names in SQL >>>>> queries. >>>>> >>>>> [1] >>>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files >>>>> [2] >>>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1/extensions/das/artifacts/monitoring-dashboard/jaggery-files >>>>> [3] >>>>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/controllers/apis/analytics.jag#L88 >>>>> >>>>> I think we may need to cancel this vote and do RC2 by fixing these >>>>> problems. >>>>> >>>>> Thanks >>>>> >>>>> On Tue, Oct 20, 2015 at 5:02 PM, Akila Ravihansa Perera < >>>>> raviha...@wso2.com> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> This is the first release candidate of WSO2 Private PaaS 4.1.0. >>>>>> >>>>>> This release fixes the following issues: >>>>>> https://wso2.org/jira/issues/?filter=12464 >>>>>> >>>>>> Please download, test and vote. The vote will be open for 72 hours >>>>>> or as needed. >>>>>> >>>>>> *​Source and binary distribution files:* >>>>>> https://svn.wso2.org/repos/wso2/scratch/PPAAS/wso2ppaas-4.1.0-rc1 >>>>>> >>>>>> *Maven staging repository:* >>>>>> http://maven.wso2.org/nexus/content/repositories/orgwso2ppaas-027/ >>>>>> >>>>>> *The tag to be voted upon:* >>>>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1 >>>>>> >>>>>> >>>>>> [ ] Broken - do not release (explain why) >>>>>> [ ] Stable - go ahead and release >>>>>> >>>>>> >>>>>> Thanks, >>>>>> The WSO2 Private PaaS Team >>>>>> >>>>>> _______________________________________________ >>>>>> Dev mailing list >>>>>> Dev@wso2.org >>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> *Imesh Gunaratne* >>>>> Senior Technical Lead >>>>> WSO2 Inc: http://wso2.com >>>>> T: +94 11 214 5345 M: +94 77 374 2057 >>>>> W: http://imesh.gunaratne.org >>>>> Lean . Enterprise . Middleware >>>>> >>>>> >>>> >>>> >>>> -- >>>> Akila Ravihansa Perera >>>> WSO2 Inc.; http://wso2.com/ >>>> >>>> Blog: http://ravihansa3000.blogspot.com >>>> >>> >>> >>> >>> -- >>> *Imesh Gunaratne* >>> Senior Technical Lead >>> WSO2 Inc: http://wso2.com >>> T: +94 11 214 5345 M: +94 77 374 2057 >>> W: http://imesh.gunaratne.org >>> Lean . Enterprise . Middleware >>> >>> >> >> >> -- >> Akila Ravihansa Perera >> WSO2 Inc.; http://wso2.com/ >> >> Blog: http://ravihansa3000.blogspot.com >> > > > > -- > Akila Ravihansa Perera > WSO2 Inc.; http://wso2.com/ > > Blog: http://ravihansa3000.blogspot.com > -- *Imesh Gunaratne* Senior Technical Lead WSO2 Inc: http://wso2.com T: +94 11 214 5345 M: +94 77 374 2057 W: http://imesh.gunaratne.org Lean . Enterprise . Middleware
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
