New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared calendars / contacts and fine grained permissions #303

Closed
netgusto opened this Issue Nov 26, 2014 · 17 comments

Comments

Projects
None yet
10 participants
@netgusto
Collaborator

netgusto commented Nov 26, 2014

B2 should really support these !

@schmidp

This comment has been minimized.

Show comment
Hide comment
@schmidp

schmidp Jan 20, 2015

is this something the development version already supports?

schmidp commented Jan 20, 2015

is this something the development version already supports?

@mrbaseman

This comment has been minimized.

Show comment
Hide comment
@mrbaseman

mrbaseman Mar 7, 2015

for managing the permissions to calendars across user accounts in Baikal 0.2.7 I have written a script: https://github.com/mrbaseman/calendar-tools/blob/master/permissions.php (just leaving this comment here for others who are searching for such a tool and stumble across this ticket)

mrbaseman commented Mar 7, 2015

for managing the permissions to calendars across user accounts in Baikal 0.2.7 I have written a script: https://github.com/mrbaseman/calendar-tools/blob/master/permissions.php (just leaving this comment here for others who are searching for such a tool and stumble across this ticket)

@SamuelMoraesF

This comment has been minimized.

Show comment
Hide comment
@SamuelMoraesF

SamuelMoraesF Mar 30, 2015

Would be fantastic an option to share calendars with public link(read-only mode, without login)

SamuelMoraesF commented Mar 30, 2015

Would be fantastic an option to share calendars with public link(read-only mode, without login)

@mrbaseman

This comment has been minimized.

Show comment
Hide comment
@mrbaseman

mrbaseman Mar 31, 2015

actually, this is a special case included in my script. Just add a user, let's call him "web" and assign a simple password to him (e.g. "readonly" - it doesn't matter at all) and remove the default calendar and carddav ressource. So, the user web has no writable ressources at all. Then, use my script to grant him read-access to other user's calendars. The link to access the default calendar of user called "user" is: http://web:readonly@www.examle.com/baikal/cal.php/calendars/user/default

mrbaseman commented Mar 31, 2015

actually, this is a special case included in my script. Just add a user, let's call him "web" and assign a simple password to him (e.g. "readonly" - it doesn't matter at all) and remove the default calendar and carddav ressource. So, the user web has no writable ressources at all. Then, use my script to grant him read-access to other user's calendars. The link to access the default calendar of user called "user" is: http://web:readonly@www.examle.com/baikal/cal.php/calendars/user/default

@dakira

This comment has been minimized.

Show comment
Hide comment
@dakira

dakira May 21, 2015

@mrbaseman, thanks for this! Exactly what I was looking for! Just a hint through: You should start to get used to PDO and use prepared statements to prevent SQL injections.

dakira commented May 21, 2015

@mrbaseman, thanks for this! Exactly what I was looking for! Just a hint through: You should start to get used to PDO and use prepared statements to prevent SQL injections.

@mrbaseman

This comment has been minimized.

Show comment
Hide comment
@mrbaseman

mrbaseman Jun 3, 2015

@dakira,

Just a hint through: You should start to get used to PDO and use prepared statements to prevent SQL injections.
Thanks for the hint. I have been thinking about it for a while, but I
don't quite understand how SQL injections would be an attack vector in
this case. The first thing I do (after including some definitions), is
to check if the user is properly authenticated as admin and I check the
password hash. Without knowledge of that, the script exits in the first
place. Note that the credentials are not stored in the database. They
are defined in constants to which I compare the input coming from the
web server. Only, if this is correctly verified, a connection to the
database is opened.
Then, if you are already authenticated as admin, you anyway have full
access and there is no need to protect against SQL injections. Note:
there is no code sent to the browser for local execution (Javascript and
the like), so also no danger from XSS.

mrbaseman commented Jun 3, 2015

@dakira,

Just a hint through: You should start to get used to PDO and use prepared statements to prevent SQL injections.
Thanks for the hint. I have been thinking about it for a while, but I
don't quite understand how SQL injections would be an attack vector in
this case. The first thing I do (after including some definitions), is
to check if the user is properly authenticated as admin and I check the
password hash. Without knowledge of that, the script exits in the first
place. Note that the credentials are not stored in the database. They
are defined in constants to which I compare the input coming from the
web server. Only, if this is correctly verified, a connection to the
database is opened.
Then, if you are already authenticated as admin, you anyway have full
access and there is no need to protect against SQL injections. Note:
there is no code sent to the browser for local execution (Javascript and
the like), so also no danger from XSS.

@dakira

This comment has been minimized.

Show comment
Hide comment
@dakira

dakira Jun 4, 2015

@mrbaseman it's just a weak link that's avoidable. If you look at the recent Magento attack you'll see that every individual component was secure and well thought out. There were several issues that couldn't be used for an attack individually. Only chained together, an attack was possible. Not using prepared statements could be part of such a chain (even if you can't see an attack yet).

dakira commented Jun 4, 2015

@mrbaseman it's just a weak link that's avoidable. If you look at the recent Magento attack you'll see that every individual component was secure and well thought out. There were several issues that couldn't be used for an attack individually. Only chained together, an attack was possible. Not using prepared statements could be part of such a chain (even if you can't see an attack yet).

@zot24

This comment has been minimized.

Show comment
Hide comment
@zot24

zot24 Jun 26, 2015

How @netgusto are you going to implement the sharing addressbooks between users for a CardDav server?

I was just having a look to your code on BaikalModelBundle and I was wondering if you are gonna use GroupMembers as a kind of pivot table to handle the relations between the principals (users) and the principals (groups of users) to achieve the sharing of an addressbook between the members of a group.

btw great project thanks

zot24 commented Jun 26, 2015

How @netgusto are you going to implement the sharing addressbooks between users for a CardDav server?

I was just having a look to your code on BaikalModelBundle and I was wondering if you are gonna use GroupMembers as a kind of pivot table to handle the relations between the principals (users) and the principals (groups of users) to achieve the sharing of an addressbook between the members of a group.

btw great project thanks

@m0yellow

This comment has been minimized.

Show comment
Hide comment
@m0yellow

m0yellow Jan 7, 2016

@mrbaseman there is no excuse for not using prepared statements, also in the admin menu.
To illustrate that think the following example: even with XSRF, cookie leak or man-in-the-browser attack you could still implement read/export limit, strong history, query log, and the attack surface would be much smaller. If I can hijack the admin with a simple phishing link, I could download and delete the whole calendar in most companies, as there tend to be more admins than needed for install (e.g. HR, for assigning rights).

TL;DR: +1 for proper permission handling in Baikal, as this is the backbone for professional use-cases.

m0yellow commented Jan 7, 2016

@mrbaseman there is no excuse for not using prepared statements, also in the admin menu.
To illustrate that think the following example: even with XSRF, cookie leak or man-in-the-browser attack you could still implement read/export limit, strong history, query log, and the attack surface would be much smaller. If I can hijack the admin with a simple phishing link, I could download and delete the whole calendar in most companies, as there tend to be more admins than needed for install (e.g. HR, for assigning rights).

TL;DR: +1 for proper permission handling in Baikal, as this is the backbone for professional use-cases.

@mrbaseman

This comment has been minimized.

Show comment
Hide comment
@mrbaseman

mrbaseman Jan 9, 2016

@matthias first of all, note that I only have provided the user
management script as a workaround. I haven't been working on other
things concerning the admin menu, yet (because I didn't have the time to
learn how to use the Flake class framework.

Coming back to your main point about prepared statements in the
permission administration script: prepared statements help against first
order sql injections when user supplied data is used in queries. But
that's not the case here. I have queries without user-supplied
components to build up the lists of users, groups, and permissions. Only
these IDs are compared to the posted values. For setting the permissions
I also don't use the posted variables directly in the queries. I only
use them to decide which 'if'-branch to go. The statement itself again
is based on values originally coming directly out of the database. So,
prepared statements would not improve anything here, because the attacks
prevented by prepared statements wouldn't be successful anyway. Prepared
statements also wouldn't help against a man in the browser attack.
Thee is already some protection in place against taking over the admin
session, because the hash value contains the client IP and it expires
every midnight.

Anyhow, I can rewrite the script with prepared statements. It seems that
this is a strong desire for some users...

mrbaseman commented Jan 9, 2016

@matthias first of all, note that I only have provided the user
management script as a workaround. I haven't been working on other
things concerning the admin menu, yet (because I didn't have the time to
learn how to use the Flake class framework.

Coming back to your main point about prepared statements in the
permission administration script: prepared statements help against first
order sql injections when user supplied data is used in queries. But
that's not the case here. I have queries without user-supplied
components to build up the lists of users, groups, and permissions. Only
these IDs are compared to the posted values. For setting the permissions
I also don't use the posted variables directly in the queries. I only
use them to decide which 'if'-branch to go. The statement itself again
is based on values originally coming directly out of the database. So,
prepared statements would not improve anything here, because the attacks
prevented by prepared statements wouldn't be successful anyway. Prepared
statements also wouldn't help against a man in the browser attack.
Thee is already some protection in place against taking over the admin
session, because the hash value contains the client IP and it expires
every midnight.

Anyhow, I can rewrite the script with prepared statements. It seems that
this is a strong desire for some users...

@m0yellow

This comment has been minimized.

Show comment
Hide comment
@m0yellow

m0yellow Jan 9, 2016

thanks for your lengthy and elaborated reply,
as I came here via google, I wanted to give advice more for the anonymous reader.

Please take it as a reference to best current practice.
I appreciate your work, and my ideas about probing and take overs are only from the daily reading at my abuse@ address.

On 09 Jan 2016, at 21:09, Martin Hecht notifications@github.com wrote:

@matthias first of all, note that I only have provided the user
management script as a workaround. I haven't been working on other
things concerning the admin menu, yet (because I didn't have the time to
learn how to use the Flake class framework.

Coming back to your main point about prepared statements in the
permission administration script: prepared statements help against first
order sql injections when user supplied data is used in queries. But
that's not the case here. I have queries without user-supplied
components to build up the lists of users, groups, and permissions. Only
these IDs are compared to the posted values. For setting the permissions
I also don't use the posted variables directly in the queries. I only
use them to decide which 'if'-branch to go. The statement itself again
is based on values originally coming directly out of the database. So,
prepared statements would not improve anything here, because the attacks
prevented by prepared statements wouldn't be successful anyway. Prepared
statements also wouldn't help against a man in the browser attack.
Thee is already some protection in place against taking over the admin
session, because the hash value contains the client IP and it expires
every midnight.

Anyhow, I can rewrite the script with prepared statements. It seems that
this is a strong desire for some users...


Reply to this email directly or view it on GitHub.

m0yellow commented Jan 9, 2016

thanks for your lengthy and elaborated reply,
as I came here via google, I wanted to give advice more for the anonymous reader.

Please take it as a reference to best current practice.
I appreciate your work, and my ideas about probing and take overs are only from the daily reading at my abuse@ address.

On 09 Jan 2016, at 21:09, Martin Hecht notifications@github.com wrote:

@matthias first of all, note that I only have provided the user
management script as a workaround. I haven't been working on other
things concerning the admin menu, yet (because I didn't have the time to
learn how to use the Flake class framework.

Coming back to your main point about prepared statements in the
permission administration script: prepared statements help against first
order sql injections when user supplied data is used in queries. But
that's not the case here. I have queries without user-supplied
components to build up the lists of users, groups, and permissions. Only
these IDs are compared to the posted values. For setting the permissions
I also don't use the posted variables directly in the queries. I only
use them to decide which 'if'-branch to go. The statement itself again
is based on values originally coming directly out of the database. So,
prepared statements would not improve anything here, because the attacks
prevented by prepared statements wouldn't be successful anyway. Prepared
statements also wouldn't help against a man in the browser attack.
Thee is already some protection in place against taking over the admin
session, because the hash value contains the client IP and it expires
every midnight.

Anyhow, I can rewrite the script with prepared statements. It seems that
this is a strong desire for some users...


Reply to this email directly or view it on GitHub.

@evert evert added the enhancement label Jan 21, 2016

@evert evert removed the feature request label Jan 21, 2016

@evert

This comment has been minimized.

Show comment
Hide comment
@evert

evert Jan 21, 2016

Contributor

This issue was moved to fruux/Baikal2#30

Contributor

evert commented Jan 21, 2016

This issue was moved to fruux/Baikal2#30

@evert evert closed this Jan 21, 2016

@lkraav

This comment has been minimized.

Show comment
Hide comment
@lkraav

lkraav Feb 15, 2016

Since this repo just got a fresh pre-release, which repo and issue about sharing should one follow?

lkraav commented Feb 15, 2016

Since this repo just got a fresh pre-release, which repo and issue about sharing should one follow?

@evert

This comment has been minimized.

Show comment
Hide comment
@evert

evert Feb 15, 2016

Contributor

Sharing in baikal 0.x will land when this PR lands: https://github.com/fruux/sabre-dav/pull/696

Contributor

evert commented Feb 15, 2016

Sharing in baikal 0.x will land when this PR lands: https://github.com/fruux/sabre-dav/pull/696

@vbmit3

This comment has been minimized.

Show comment
Hide comment
@vbmit3

vbmit3 Dec 27, 2017

@mrbaseman
Is your script supposed to work for addressbooks too?
I'm in dire need of making a "shared" RO addressbook in baikal 0.2.7, while leaving the others RW.
This script seems to take care of calendars, but it doesn't seem to work for addressbooks.
Any help would be appreciated, thanks.

vbmit3 commented Dec 27, 2017

@mrbaseman
Is your script supposed to work for addressbooks too?
I'm in dire need of making a "shared" RO addressbook in baikal 0.2.7, while leaving the others RW.
This script seems to take care of calendars, but it doesn't seem to work for addressbooks.
Any help would be appreciated, thanks.

@mrbaseman

This comment has been minimized.

Show comment
Hide comment
@mrbaseman

mrbaseman Dec 27, 2017

@vbmit3
since my script makes use of the calendar-proxy-read and calendar-proxy-write principals it only works for the calendars. The principals in sabre-io are something like objects to assign permissions. Perhaps it is possible to do something similar for addressbooks, but I haven't seen anything like "addressbook-proxy" principals so far.

With a newer sabre/dav versions, as shipped with Baikal 0.4.6 for instance, there is a property "d:acl" on the addressbooks however, which contains "d:read" and "d:write" for the owner of the collection. A first step would be to experiment with a dav client (cadaver for instance) and add appropriate properties to the webdav directory. I'm not sure if this helps reading the files therein. Maybe they inherit the acls from the directory when they are created after the acls were set, so this could be a road to go, but I haven't used the address books much yet (I have just started working on this topic by coincidence these days).

Apart from that, a simple approach could be a cron job running somewhere, that synchronizes the addressbooks occasionally. It would have to list the entries in the addressbook that you would like to share "readonly" and copy them to an empty local directory. Next it would have to remove all entries from a different webdav directory and synchronize the local directory again to that remote place. From a security point of view this would be the same as a readonly share. From the users perspective the client would miss the feedback of an error message when it tries to modify the readonly share. It would be able to do so and, well, those changes would get lost in the next synchronization (if the client notices it, depends on how the time stamps are handled during the synchronization - in the context of vcards you might have to update the REV property each time you run the synchronization).

mrbaseman commented Dec 27, 2017

@vbmit3
since my script makes use of the calendar-proxy-read and calendar-proxy-write principals it only works for the calendars. The principals in sabre-io are something like objects to assign permissions. Perhaps it is possible to do something similar for addressbooks, but I haven't seen anything like "addressbook-proxy" principals so far.

With a newer sabre/dav versions, as shipped with Baikal 0.4.6 for instance, there is a property "d:acl" on the addressbooks however, which contains "d:read" and "d:write" for the owner of the collection. A first step would be to experiment with a dav client (cadaver for instance) and add appropriate properties to the webdav directory. I'm not sure if this helps reading the files therein. Maybe they inherit the acls from the directory when they are created after the acls were set, so this could be a road to go, but I haven't used the address books much yet (I have just started working on this topic by coincidence these days).

Apart from that, a simple approach could be a cron job running somewhere, that synchronizes the addressbooks occasionally. It would have to list the entries in the addressbook that you would like to share "readonly" and copy them to an empty local directory. Next it would have to remove all entries from a different webdav directory and synchronize the local directory again to that remote place. From a security point of view this would be the same as a readonly share. From the users perspective the client would miss the feedback of an error message when it tries to modify the readonly share. It would be able to do so and, well, those changes would get lost in the next synchronization (if the client notices it, depends on how the time stamps are handled during the synchronization - in the context of vcards you might have to update the REV property each time you run the synchronization).

@dakira

This comment has been minimized.

Show comment
Hide comment
@dakira

dakira Dec 28, 2017

@evert please re-open this issue. baikal2 development is halted.

dakira commented Dec 28, 2017

@evert please re-open this issue. baikal2 development is halted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment