Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-04 Thread Jonathan Rose

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/
---

(Updated Sept. 4, 2014, 2:51 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Committed in revision 422584


Repository: Asterisk


Description
---

Apparently instead of using the readperm mask, it was using the send_events 
mask... which is somewhat weird.  It's initialized to -1 (which will return 
true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the odd 
behavior came from.

I think this was just a mistake and switching to the readperm mask appears to 
have fixed it.


Diffs
-

  /branches/1.8/main/manager.c 422543 

Diff: https://reviewboard.asterisk.org/r/3969/diff/


Testing
---

Ran through the login process with and without the system read permission.  
With it, I got the FullyBootted event. Without it, I did not.


Thanks,

Jonathan Rose

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread rmudgett

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/#review13224
---



/branches/1.8/main/manager.c
https://reviewboard.asterisk.org/r/3969/#comment23712

You need to have the isolation parens for the bitwise operators.
(send_events  x)  (readperm  x)


- rmudgett


On Sept. 2, 2014, 6:24 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3969/
 ---
 
 (Updated Sept. 2, 2014, 6:24 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Apparently instead of using the readperm mask, it was using the send_events 
 mask... which is somewhat weird.  It's initialized to -1 (which will return 
 true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the 
 odd behavior came from.
 
 I think this was just a mistake and switching to the readperm mask appears to 
 have fixed it.
 
 
 Diffs
 -
 
   /branches/1.8/main/manager.c 422543 
 
 Diff: https://reviewboard.asterisk.org/r/3969/diff/
 
 
 Testing
 ---
 
 Ran through the login process with and without the system read permission.  
 With it, I got the FullyBootted event. Without it, I did not.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread Jonathan Rose


 On Sept. 3, 2014, 4:16 p.m., rmudgett wrote:
  /branches/1.8/main/manager.c, lines 3133-3134
  https://reviewboard.asterisk.org/r/3969/diff/2/?file=67179#file67179line3133
 
  You need to have the isolation parens for the bitwise operators.
  (send_events  x)  (readperm  x)

Order of operations dictates that  will be computed before  anyway ( 
http://en.cppreference.com/w/c/language/operator_precedence ) but for the sake 
of clarity and keeping things the same I'll go ahead and make this fix.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/#review13224
---


On Sept. 2, 2014, 6:24 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3969/
 ---
 
 (Updated Sept. 2, 2014, 6:24 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Apparently instead of using the readperm mask, it was using the send_events 
 mask... which is somewhat weird.  It's initialized to -1 (which will return 
 true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the 
 odd behavior came from.
 
 I think this was just a mistake and switching to the readperm mask appears to 
 have fixed it.
 
 
 Diffs
 -
 
   /branches/1.8/main/manager.c 422543 
 
 Diff: https://reviewboard.asterisk.org/r/3969/diff/
 
 
 Testing
 ---
 
 Ran through the login process with and without the system read permission.  
 With it, I got the FullyBootted event. Without it, I did not.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread Jonathan Rose

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/
---

(Updated Sept. 3, 2014, 5:15 p.m.)


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Address rmudgett's finding.


Repository: Asterisk


Description
---

Apparently instead of using the readperm mask, it was using the send_events 
mask... which is somewhat weird.  It's initialized to -1 (which will return 
true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the odd 
behavior came from.

I think this was just a mistake and switching to the readperm mask appears to 
have fixed it.


Diffs (updated)
-

  /branches/1.8/main/manager.c 422543 

Diff: https://reviewboard.asterisk.org/r/3969/diff/


Testing
---

Ran through the login process with and without the system read permission.  
With it, I got the FullyBootted event. Without it, I did not.


Thanks,

Jonathan Rose

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread rmudgett

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/#review13227
---

Ship it!


Ship It!

- rmudgett


On Sept. 3, 2014, 5:15 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3969/
 ---
 
 (Updated Sept. 3, 2014, 5:15 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Apparently instead of using the readperm mask, it was using the send_events 
 mask... which is somewhat weird.  It's initialized to -1 (which will return 
 true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the 
 odd behavior came from.
 
 I think this was just a mistake and switching to the readperm mask appears to 
 have fixed it.
 
 
 Diffs
 -
 
   /branches/1.8/main/manager.c 422543 
 
 Diff: https://reviewboard.asterisk.org/r/3969/diff/
 
 
 Testing
 ---
 
 Ran through the login process with and without the system read permission.  
 With it, I got the FullyBootted event. Without it, I did not.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread Corey Farrell

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/#review13228
---


I don't think FullyBooted should be a security restricted event.  This will not 
effect me personally since I use read=all, but the FullyBooted event is used by 
AMI clients to determine that Asterisk is ready to receive actions (that's how 
I use it).  Changing the security level on released branches seems to me like a 
breaking change.  The idea that any user logged into AMI can know when the 
system is fully booted does not seem like any security risk to me.  As for the 
inconsistancy, my vote would be to change the security flag in main/asterisk.c 
to 0 for this event, that way all AMI users receive the event so they know 
Asterisk is ready to do stuff.

- Corey Farrell


On Sept. 3, 2014, 6:15 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3969/
 ---
 
 (Updated Sept. 3, 2014, 6:15 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Apparently instead of using the readperm mask, it was using the send_events 
 mask... which is somewhat weird.  It's initialized to -1 (which will return 
 true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the 
 odd behavior came from.
 
 I think this was just a mistake and switching to the readperm mask appears to 
 have fixed it.
 
 
 Diffs
 -
 
   /branches/1.8/main/manager.c 422543 
 
 Diff: https://reviewboard.asterisk.org/r/3969/diff/
 
 
 Testing
 ---
 
 Ran through the login process with and without the system read permission.  
 With it, I got the FullyBootted event. Without it, I did not.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-02 Thread Jonathan Rose

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/
---

Review request for Asterisk Developers and Matt Jordan.


Repository: Asterisk


Description
---

Apparently instead of using the readperm mask, it was using the send_events 
mask... which is somewhat weird.  It's initialized to -1 (which will return 
true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the odd 
behavior came from.

I think this was just a mistake and switching to the readperm mask appears to 
have fixed it.


Diffs
-

  /branches/1.8/main/manager.c 422543 

Diff: https://reviewboard.asterisk.org/r/3969/diff/


Testing
---

Ran through the login process with and without the system read permission.  
With it, I got the FullyBootted event. Without it, I did not.


Thanks,

Jonathan Rose

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-02 Thread Matt Jordan

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/#review13214
---



/branches/1.8/main/manager.c
https://reviewboard.asterisk.org/r/3969/#comment23700

Close. You should actually use both. See the equivalent code in 
process_events:

(s-session-readperm  eqe-category) == 
eqe-category 
(s-session-send_events  eqe-category) == 
eqe-category)

The send_events filter are those permissions that have been dynamically 
modified via the Events header during authentication. If a user's session 
starts out with SYSTEM - but then they authenticate and deliberately remove it 
- we don't want to send the FullyBooted event to them.




- Matt Jordan


On Sept. 2, 2014, 5:36 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3969/
 ---
 
 (Updated Sept. 2, 2014, 5:36 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Apparently instead of using the readperm mask, it was using the send_events 
 mask... which is somewhat weird.  It's initialized to -1 (which will return 
 true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the 
 odd behavior came from.
 
 I think this was just a mistake and switching to the readperm mask appears to 
 have fixed it.
 
 
 Diffs
 -
 
   /branches/1.8/main/manager.c 422543 
 
 Diff: https://reviewboard.asterisk.org/r/3969/diff/
 
 
 Testing
 ---
 
 Ran through the login process with and without the system read permission.  
 With it, I got the FullyBootted event. Without it, I did not.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-02 Thread Jonathan Rose

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3969/
---

(Updated Sept. 2, 2014, 6:24 p.m.)


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Address mjordan's finding.


Repository: Asterisk


Description
---

Apparently instead of using the readperm mask, it was using the send_events 
mask... which is somewhat weird.  It's initialized to -1 (which will return 
true when used with bitwise and on EVENT_FLAG_SYSTEM) and this is where the odd 
behavior came from.

I think this was just a mistake and switching to the readperm mask appears to 
have fixed it.


Diffs (updated)
-

  /branches/1.8/main/manager.c 422543 

Diff: https://reviewboard.asterisk.org/r/3969/diff/


Testing
---

Ran through the login process with and without the system read permission.  
With it, I got the FullyBootted event. Without it, I did not.


Thanks,

Jonathan Rose

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev