[ 
https://issues.apache.org/jira/browse/DERBY-2109?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573059#action_12573059
 ] 

Martin Zaun commented on DERBY-2109:
------------------------------------


I'm sorry to say that I overlooked one line of debug code in SecurityUtil that 
made it into DERBY-2109-10. (I'd forgotten tagged that line as usual.)  
Unfortunately, the line is significant since the published code checks for a 
"dummy" permission I'd put in there for doAs() v. doAsPrivileged() debugging 
purposes:
+                        //AccessController.checkPermission(perm);
+                        AccessController.checkPermission(new 
java.util.PropertyPermission("user.dir", "read"));

I'm right now running junit-all (1 error) and will publish another patch update 
right after.

> Daniel John Debrunner wrote:
> > Bottomline: When evaluating permissions, the (Sun) Java Security Runtime 
> > uses the principal names as found literally in the policy file and not as 
> > returned by SystemPrincipal.getName() (where we could return normalized 
> > names).
> It's hard to see how that is the case, since the policy file is read in and 
> converted to a Policy object containing permissions and for Derby's 
> SystemPermission the class must be o.a.d.security.SystemPermission.

It's correct that we're using our SystemPermission classes only -- but the 
issue is in the realm of Subject/Principal checking by the Java Security 
Runtime when matching the Subject instantiated by our code against the 
Principal declaration in the policy file. This check is not entirely carried 
out by means of getName()/equals()/hashCode()  on our SystemPrincipal class, in 
which case we could have had the name comparisons always done on the normalized 
names.  Instead, the Java Security Runtime uses the literal Principal name as 
declared in the policy file and denies permission when it can't find an exact 
match in our Subject's SystemPrincipal list.

> - figuring out the principal names in the policy files - I'd be interested to 
> see the implementation of SystemPrincipal you used to try and implement the 
> required functionality 

The SystemPrincipal class in DERBY-2109-09 attempted to encapsulate 
Authorization Identifiers and "normalized" the principal name right within the 
constructor, so that getName()/equals()/hashCode()  would operate on 
(normalized) auth ids. 

As commented I also looked into SystemPrincipal implementing 
com.sun.security.auth.PrincipalComparator by adding an implies(Subject) method 
returning true when a the Subject's principal list contains a normalized name 
matching this principal's normalized name.  With a few more tricks I got it 
working -- but that was just for insight, since PrincipalComparator is a 
non-standard interface.

>  - related to the previous one is having more time to understand this:
> + // An alternative approach of normalizing all names within
> + // SystemPrincipal has issues; see comments there.
> + principals.add(new SystemPrincipal(user));
> + principals.add(new SystemPrincipal(getAuthorizationId(user))); 

Adding the normalized identifier ensures that a non-delimited principal "FRED" 
matches users "fred", "Fred", "FRED" ...

Let me know if you think the code needs more comments.

> Could you confirm the format for names in the policy file in the patch? Does 
> it match the description here:
https://issues.apache.org/jira/browse/DERBY-2109?focusedCommentId=12561537#action_12561537
> - does it support delimited identifiers of the form "\"[EMAIL PROTECTED]""?

yes

> - is the only issue that non-delimited identifiers "fred" do not resolve 
> correctly?

They resolve incompletely: non-delimited identifies "fred" only matches "fred", 
while "FRED" matches "fred", "Fred", "FRED" ...

Here's what I've verified.  A policy file with belows grants results in the 
following System Privileges behaviour for users:

  grant principal org.apache.derby.authentication.SystemPrincipal "MARTIN" { 
... }
  checks for users:
     martin -- granted
     marTin -- granted
     MARTIN -- granted
     "marTin" -- denied, missing permission  (delimited identifier, different 
from marTin)

  grant principal org.apache.derby.authentication.SystemPrincipal "edWard" { 
... }
  checks for users:
    edWard -- granted
    edward -- denied, missing permission

  grant principal org.apache.derby.authentication.SystemPrincipal "\"[EMAIL 
PROTECTED]"" { ... }
  checks for users:
    "[EMAIL PROTECTED]" -- granted
    "[EMAIL PROTECTED]" -- denied, missing permission
    [EMAIL PROTECTED] -- denied, illegally formed name as complained by 
IdUtil.getUserAuthorizationId() because it's a non-delimited identifier having 
special characters

However, in the last case the exception is not nicely presented to the client 
and needs improvement (I'd expected this case not to pass authentication, but 
we have to cover for it, especially since we now can have authorization checks 
without prior authentication).

> From a security point of view does this have the potential to allow one user 
> to piggy back on the permissions of another user?

Not really.  Under the rules for Identifiers for authentication and 
authorization, the non-delimited identifiers "fred", "Fred", "FRED" all 
represent the same person.  So, if user "fred" can piggy-back on a principal 
grant to "FRED" -- that's the requested feature that took some effort to 
implement :)

In contrast, user "marTin" cannot piggy-back on a principal grant for 
"\"martin\"".

>  - ensuring the new security objects that are serializable have serialization 
> ids to ensure compatibility across releases

I'm not aware anymore why SystemPrincipal implements Serializable, so, perhaps, 
I should research and add a comment.  I'm not sure there's a good reason for 
this class to be serializable (to the contrary, often information about user 
names etc should not be serialized), but if there is, I agree, a serialization 
id should be there (with the class being so simple, we probably won't need 
readObject() and writeObject()).

Our Permission classes are not serializable, so, I don't think we have other 
security objects to think about.

While NetworkServerControlImpl also does not implement Serializable, I wonder 
if the userArg, passewordArg, and bootpasswordArg fields should be declared 
transient as good pactice/precaution.

> - making the new security objects final

Yes, will do for SystemPrincipal, DatabasePermission and SystemPermission.

Martin

> System privileges
> -----------------
>
>                 Key: DERBY-2109
>                 URL: https://issues.apache.org/jira/browse/DERBY-2109
>             Project: Derby
>          Issue Type: New Feature
>          Components: Security
>    Affects Versions: 10.3.1.4
>            Reporter: Rick Hillegas
>            Assignee: Martin Zaun
>         Attachments: DERBY-2109-02.diff, DERBY-2109-02.stat, 
> derby-2109-03-javadoc-see-tags.diff, DERBY-2109-04.diff, DERBY-2109-04.stat, 
> DERBY-2109-05and06.diff, DERBY-2109-05and06.stat, DERBY-2109-07.diff, 
> DERBY-2109-07.stat, DERBY-2109-08.diff, DERBY-2109-08.stat, 
> DERBY-2109-08_addendum.diff, DERBY-2109-08_addendum.stat, DERBY-2109-09.diff, 
> DERBY-2109-09.stat, DERBY-2109-10.diff, DERBY-2109-10.stat, 
> SystemPrivilegesBehaviour.html, systemPrivs.html, systemPrivs.html, 
> systemPrivs.html, systemPrivs.html
>
>
> Add mechanisms for controlling system-level privileges in Derby. See the 
> related email discussion at 
> http://article.gmane.org/gmane.comp.apache.db.derby.devel/33151.
> The 10.2 GRANT/REVOKE work was a big step forward in making Derby more  
> secure in a client/server configuration. I'd like to plug more client/server 
> security holes in 10.3. In particular, I'd like to focus on  authorization 
> issues which the ANSI spec doesn't address.
> Here are the important issues which came out of the email discussion.
> Missing privileges that are above the level of a single database:
> - Create Database
> - Shutdown all databases
> - Shutdown System
> Missing privileges specific to a particular database:
> - Shutdown that Database
> - Encrypt that database
> - Upgrade database
> - Create (in that Database) Java Plugins (currently  Functions/Procedures, 
> but someday Aggregates and VTIs)
> Note that 10.2 gave us GRANT/REVOKE control over the following  
> database-specific issues, via granting execute privilege to system  
> procedures:
> Jar Handling
> Backup Routines
> Admin Routines
> Import/Export
> Property Handling
> Check Table
> In addition, since 10.0, the privilege of connecting to a database has been 
> controlled by two properties (derby.database.fullAccessUsers and 
> derby.database.defaultConnectionMode) as described in the security section of 
> the Developer's Guide (see 
> http://db.apache.org/derby/docs/10.2/devguide/cdevcsecure865818.html).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to