You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Emil Anca <ea...@hortonworks.com> on 2015/06/04 16:57:40 UTC

Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/
-----------------------------------------------------------

Review request for Ambari, Robert Levas and Vitalyi Brodetskyi.


Bugs: AMBARI-11687
    https://issues.apache.org/jira/browse/AMBARI-11687


Repository: ambari


Description
-------

Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.

*Solution*
# Provide a kerberos-env configuration to optionally create case-insensitive rules
# If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:

~~~
RULE:[1:$1@$0](.*@REALM)s/@.*///L
~~~


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java c599cc1 
  ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java dc5fc75 
  ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml ec50f69 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
  ambari-web/app/data/HDP2/site_properties.js d5310e2 

Diff: https://reviews.apache.org/r/35073/diff/


Testing
-------

* mvn clean test -pl AuthToLocalBuilderTest
* Kerbernized/dekerbenized prop with / without prop
* Added service on kerberized cluster


Thanks,

Emil Anca


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Emil Anca <ea...@hortonworks.com>.

> On June 6, 2015, 10:56 a.m., Robert Levas wrote:
> > This looks good, but for some reason the number of `/` characters seems to be excessive.  We should check to make sure this is correct.  However it could be that the pattern is compressed due to unneeded data:
> > `RULE:[1:$1@$0](.*@%s)s/@.*/(empty string replacment)/(no regex flags, usually g might go here)/L`
> > 
> > Is 'L' a regex flag in this case or a special flag for the Hadoop auth-to-local rules processor? If testing was a success, than I guess the current format is correct.

Not sure but I would think it's part of the regex, testing was successful, but will test some more to confirm format is correct. Nice point


> On June 6, 2015, 10:56 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java, lines 370-371
> > <https://reviews.apache.org/r/35073/diff/2/?file=979950#file979950line370>
> >
> >     This could be problematic in the event `kerberos-env/case_insensitive_username_rules` is not set (or `null`).  Maybe use something like
> >     ```
> >     boolean caseInsensitiveUser = "true".equalsIgnoreCase(existingConfigurations.get("kerberos-env").get("case_insensitive_username_rules"))
> >     ```

"kerberos-env" is expected to be available at this point as it was previously validated within the execution flow. I could double validate if you are concerned. If "case_insensitive_username_rules" however is null it shouldn't be a problem as Boolean.valueOf(null) should return false, which is correct.


> On June 6, 2015, 10:56 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java, line 243
> > <https://reviews.apache.org/r/35073/diff/2/?file=979949#file979949line243>
> >
> >     This seems redundant.  Could the flag be pulled out into a variable and appened as either and empty string or '//L' depending on the value of caseInsensitiveUser?  Or maybe simply add the opation to the end if needed?

Yes your approach would make the code a little more readable.


- Emil


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/#review86908
-----------------------------------------------------------


On June 5, 2015, 1:51 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35073/
> -----------------------------------------------------------
> 
> (Updated June 5, 2015, 1:51 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11687
>     https://issues.apache.org/jira/browse/AMBARI-11687
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.
> 
> *Solution*
> # Provide a kerberos-env configuration to optionally create case-insensitive rules
> # If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:
> 
> ~~~
> RULE:[1:$1@$0](.*@REALM)s/@.*///L
> ~~~
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 76054b7 
>   ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java 5744b53 
>   ambari-web/app/data/HDP2/site_properties.js d6ab273 
> 
> Diff: https://reviews.apache.org/r/35073/diff/
> 
> 
> Testing
> -------
> 
> * mvn clean test -pl AuthToLocalBuilderTest
> * Kerbernized/dekerbenized prop with / without prop
> * Added service on kerberized cluster
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/#review86908
-----------------------------------------------------------

Ship it!


This looks good, but for some reason the number of `/` characters seems to be excessive.  We should check to make sure this is correct.  However it could be that the pattern is compressed due to unneeded data:
`RULE:[1:$1@$0](.*@%s)s/@.*/(empty string replacment)/(no regex flags, usually g might go here)/L`

Is 'L' a regex flag in this case or a special flag for the Hadoop auth-to-local rules processor? If testing was a success, than I guess the current format is correct.


ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java
<https://reviews.apache.org/r/35073/#comment139068>

    This seems redundant.  Could the flag be pulled out into a variable and appened as either and empty string or '//L' depending on the value of caseInsensitiveUser?  Or maybe simply add the opation to the end if needed?



ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
<https://reviews.apache.org/r/35073/#comment139067>

    This could be problematic in the event `kerberos-env/case_insensitive_username_rules` is not set (or `null`).  Maybe use something like
    ```
    boolean caseInsensitiveUser = "true".equalsIgnoreCase(existingConfigurations.get("kerberos-env").get("case_insensitive_username_rules"))
    ```


- Robert Levas


On June 5, 2015, 9:51 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35073/
> -----------------------------------------------------------
> 
> (Updated June 5, 2015, 9:51 a.m.)
> 
> 
> Review request for Ambari, Robert Levas and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11687
>     https://issues.apache.org/jira/browse/AMBARI-11687
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.
> 
> *Solution*
> # Provide a kerberos-env configuration to optionally create case-insensitive rules
> # If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:
> 
> ~~~
> RULE:[1:$1@$0](.*@REALM)s/@.*///L
> ~~~
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 76054b7 
>   ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java 5744b53 
>   ambari-web/app/data/HDP2/site_properties.js d6ab273 
> 
> Diff: https://reviews.apache.org/r/35073/diff/
> 
> 
> Testing
> -------
> 
> * mvn clean test -pl AuthToLocalBuilderTest
> * Kerbernized/dekerbenized prop with / without prop
> * Added service on kerberized cluster
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/#review87163
-----------------------------------------------------------

Ship it!


Ship It!

- Tom Beerbower


On June 9, 2015, 9:55 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35073/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 9:55 a.m.)
> 
> 
> Review request for Ambari, Robert Levas, Tom Beerbower, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11687
>     https://issues.apache.org/jira/browse/AMBARI-11687
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.
> 
> *Solution*
> # Provide a kerberos-env configuration to optionally create case-insensitive rules
> # If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:
> 
> ~~~
> RULE:[1:$1@$0](.*@REALM)s/@.*///L
> ~~~
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 8a5d4fd 
>   ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f8ba840 
>   ambari-web/app/data/HDP2/site_properties.js 484ad38 
> 
> Diff: https://reviews.apache.org/r/35073/diff/
> 
> 
> Testing
> -------
> 
> * mvn clean test -pl AuthToLocalBuilderTest KerberosHelperImpl locally
> * Jenking tests in progress
> * Kerbernized/dekerbenized prop with / without prop while monitoring core-site auth to local rules
> * Added service on kerberized cluster
> * Ran
>  
>    [root@c6401 ~]# hadoop org.apache.hadoop.security.HadoopKerberosName EAnca@EXAMPLE.COM
> Name: EAnca@EXAMPLE.COM to eanca
> 
> to test the mapping of the new generic Rule.
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Emil Anca <ea...@hortonworks.com>.

> On June 9, 2015, 11:17 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java, line 370
> > <https://reviews.apache.org/r/35073/diff/3/?file=981342#file981342line370>
> >
> >     A NPE will be thrown if `kerberos-env/case_insensitive_rules` is null or not present in the map.

if kerberos-env/case_insensitive_rules is null it will evaluate as Boolean.valueOf(null) which will return false, indicating correct behaviour.


- Emil


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/#review87159
-----------------------------------------------------------


On June 9, 2015, 9:55 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35073/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 9:55 a.m.)
> 
> 
> Review request for Ambari, Robert Levas, Tom Beerbower, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11687
>     https://issues.apache.org/jira/browse/AMBARI-11687
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.
> 
> *Solution*
> # Provide a kerberos-env configuration to optionally create case-insensitive rules
> # If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:
> 
> ~~~
> RULE:[1:$1@$0](.*@REALM)s/@.*///L
> ~~~
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 8a5d4fd 
>   ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f8ba840 
>   ambari-web/app/data/HDP2/site_properties.js 484ad38 
> 
> Diff: https://reviews.apache.org/r/35073/diff/
> 
> 
> Testing
> -------
> 
> * mvn clean test -pl AuthToLocalBuilderTest KerberosHelperImpl locally
> * Jenking tests in progress
> * Kerbernized/dekerbenized prop with / without prop while monitoring core-site auth to local rules
> * Added service on kerberized cluster
> * Ran
>  
>    [root@c6401 ~]# hadoop org.apache.hadoop.security.HadoopKerberosName EAnca@EXAMPLE.COM
> Name: EAnca@EXAMPLE.COM to eanca
> 
> to test the mapping of the new generic Rule.
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/#review87159
-----------------------------------------------------------

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
<https://reviews.apache.org/r/35073/#comment139480>

    A NPE will be thrown if `kerberos-env/case_insensitive_rules` is null or not present in the map.


- Robert Levas


On June 9, 2015, 5:55 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35073/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 5:55 a.m.)
> 
> 
> Review request for Ambari, Robert Levas, Tom Beerbower, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11687
>     https://issues.apache.org/jira/browse/AMBARI-11687
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.
> 
> *Solution*
> # Provide a kerberos-env configuration to optionally create case-insensitive rules
> # If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:
> 
> ~~~
> RULE:[1:$1@$0](.*@REALM)s/@.*///L
> ~~~
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 8a5d4fd 
>   ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f8ba840 
>   ambari-web/app/data/HDP2/site_properties.js 484ad38 
> 
> Diff: https://reviews.apache.org/r/35073/diff/
> 
> 
> Testing
> -------
> 
> * mvn clean test -pl AuthToLocalBuilderTest KerberosHelperImpl locally
> * Jenking tests in progress
> * Kerbernized/dekerbenized prop with / without prop while monitoring core-site auth to local rules
> * Added service on kerberized cluster
> * Ran
>  
>    [root@c6401 ~]# hadoop org.apache.hadoop.security.HadoopKerberosName EAnca@EXAMPLE.COM
> Name: EAnca@EXAMPLE.COM to eanca
> 
> to test the mapping of the new generic Rule.
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Emil Anca <ea...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/
-----------------------------------------------------------

(Updated June 9, 2015, 9:55 a.m.)


Review request for Ambari, Robert Levas, Tom Beerbower, and Vitalyi Brodetskyi.


Changes
-------

Addressed reviewer suggestions.


Bugs: AMBARI-11687
    https://issues.apache.org/jira/browse/AMBARI-11687


Repository: ambari


Description
-------

Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.

*Solution*
# Provide a kerberos-env configuration to optionally create case-insensitive rules
# If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:

~~~
RULE:[1:$1@$0](.*@REALM)s/@.*///L
~~~


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
  ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 8a5d4fd 
  ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
  ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f8ba840 
  ambari-web/app/data/HDP2/site_properties.js 484ad38 

Diff: https://reviews.apache.org/r/35073/diff/


Testing (updated)
-------

* mvn clean test -pl AuthToLocalBuilderTest KerberosHelperImpl locally
* Jenking tests in progress
* Kerbernized/dekerbenized prop with / without prop while monitoring core-site auth to local rules
* Added service on kerberized cluster
* Ran
 
   [root@c6401 ~]# hadoop org.apache.hadoop.security.HadoopKerberosName EAnca@EXAMPLE.COM
Name: EAnca@EXAMPLE.COM to eanca

to test the mapping of the new generic Rule.


Thanks,

Emil Anca


Re: Review Request 35073: Kerberos: Force principal names to resolve to lowercase lower usernames in auth-to-local default rules

Posted by Emil Anca <ea...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35073/
-----------------------------------------------------------

(Updated June 5, 2015, 1:51 p.m.)


Review request for Ambari, Robert Levas and Vitalyi Brodetskyi.


Bugs: AMBARI-11687
    https://issues.apache.org/jira/browse/AMBARI-11687


Repository: ambari


Description
-------

Force principals names to resolve to lowercase local usernames in auth-to-local rules. This will help when the KDC is an MIT KDC or an  Active Directory and user accounts have uppercase letters that need to be converted to lowercase letters.  For example:  {{USER1234@REALM}} should resolve to {{user1234}}.

*Solution*
# Provide a kerberos-env configuration to optionally create case-insensitive rules
# If creating case-insensitive rules, _generic_ auth-to-local rules should contain the {{L}} option, as in:

~~~
RULE:[1:$1@$0](.*@REALM)s/@.*///L
~~~


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java 89d0b55 
  ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java 76054b7 
  ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml 6d720a0 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java d1a2bd1 
  ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java 5744b53 
  ambari-web/app/data/HDP2/site_properties.js d6ab273 

Diff: https://reviews.apache.org/r/35073/diff/


Testing
-------

* mvn clean test -pl AuthToLocalBuilderTest
* Kerbernized/dekerbenized prop with / without prop
* Added service on kerberized cluster


Thanks,

Emil Anca