-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

On 07/19/2017 10:13 AM, Guillermo González de Agüero wrote:

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

This exposes the platform's implementation, and is likely to be vendor specific. 

I don't want to have to know the name of the platform's impl class just to specify that I want the default algorithm -- or any other algorithm the platform offers.

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

It's true the we won't get IDE support for completing names, which is unfortunate. But we wouldn't necessarily get that using classnames, either, particularly when Java SE 9 comes along with modularity, and internal names become invisible.

Specifying the name of the algorithm you want seems like the right balance between simplicity, type safety, and flexibility, to me.

In practice, I'm guessing most platforms are going to offer only the default, or perhaps one or two simply-named alternatives -- e.g., "Argon2". Any more complicated names are likely to be provided by the application itself, and therefore be known to the developer.

All that said, I think most of you have more experience with CDI than I do -- let me know if I'm insisting on an anti-pattern. I just want to keep it simple for developers, and flexible and powerful for platforms and advanced developers, while avoiding exposing algorithm details or platform internals through the API.

Will

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Hi,

On Wednesday, July 19, 2017, Will Hopkins <will.hopkins@...> wrote:

On 07/19/2017 10:13 AM, Guillermo González de Agüero wrote:

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

This exposes the platform's implementation, and is likely to be vendor specific. 

There's no need for that ;) Above I mentioned an interface, which is what the application developer can specify and the server can implement.

CDI unified the concepts. The JSR 375 runtime can ask CDI for either an interface or class without knowing which is which.

Kind regards,

Arjan Tijms

 

I don't want to have to know the name of the platform's impl class just to specify that I want the default algorithm -- or any other algorithm the platform offers.

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

It's true the we won't get IDE support for completing names, which is unfortunate. But we wouldn't necessarily get that using classnames, either, particularly when Java SE 9 comes along with modularity, and internal names become invisible.

Specifying the name of the algorithm you want seems like the right balance between simplicity, type safety, and flexibility, to me.

In practice, I'm guessing most platforms are going to offer only the default, or perhaps one or two simply-named alternatives -- e.g., "Argon2". Any more complicated names are likely to be provided by the application itself, and therefore be known to the developer.

All that said, I think most of you have more experience with CDI than I do -- let me know if I'm insisting on an anti-pattern. I just want to keep it simple for developers, and flexible and powerful for platforms and advanced developers, while avoiding exposing algorithm details or platform internals through the API.

Will

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

 

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Right, but suppose there are multiple implementations of the interface, as is likely in this case (default PBKDF2, Argon2 implemented by some vendor, "MyAppHashAlg" implemented by the application).

How does the DatabaseIdentityStore ask CDI for the correct implementation of the interface?

One way is by asking for a specific class. Another is by asking for a name.

It seems to me that relying on the classname exposes implementation details, and makes the app dependent on them, and therefore less portable.

There doesn't seem to be a type-safety issue with using a name, as all names must refer to a bean that implements the correct interface, which will always be the same (i.e., in my example below, "IdentityStorePasswordHashing"). As long as the runtime can find a named bean that implements the right interface, there is type safety. If the bean can't be found, there's a config error.

On the other hand, I may have an incomplete understanding of how this all could work, being new to CDI.

Will

On 07/19/2017 11:34 AM, Arjan Tijms wrote:

Hi,

On Wednesday, July 19, 2017, Will Hopkins <will.hopkins@...> wrote:

On 07/19/2017 10:13 AM, Guillermo González de Agüero wrote:

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

This exposes the platform's implementation, and is likely to be vendor specific. 

There's no need for that ;) Above I mentioned an interface, which is what the application developer can specify and the server can implement.

CDI unified the concepts. The JSR 375 runtime can ask CDI for either an interface or class without knowing which is which.

Kind regards,

Arjan Tijms

 

I don't want to have to know the name of the platform's impl class just to specify that I want the default algorithm -- or any other algorithm the platform offers.

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

It's true the we won't get IDE support for completing names, which is unfortunate. But we wouldn't necessarily get that using classnames, either, particularly when Java SE 9 comes along with modularity, and internal names become invisible.

Specifying the name of the algorithm you want seems like the right balance between simplicity, type safety, and flexibility, to me.

In practice, I'm guessing most platforms are going to offer only the default, or perhaps one or two simply-named alternatives -- e.g., "Argon2". Any more complicated names are likely to be provided by the application itself, and therefore be known to the developer.

All that said, I think most of you have more experience with CDI than I do -- let me know if I'm insisting on an anti-pattern. I just want to keep it simple for developers, and flexible and powerful for platforms and advanced developers, while avoiding exposing algorithm details or platform internals through the API.

Will

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

 

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

But then, how to select another vendor provided algorithm without depending on its specific implementation?

Regards,

Guillermo González de Agüero

El mié., 19 de julio de 2017 19:49, Arjan Tijms <arjan.tijms@...> escribió:

Hi,

On Wed, Jul 19, 2017 at 6:07 PM, Will Hopkins <will.hopkins@...> wrote:

Right, but suppose there are multiple implementations of the interface, as is likely in this case (default PBKDF2, Argon2 implemented by some vendor, "MyAppHashAlg" implemented by the application).

This is via the "highlander rule" (there can be only one).

So there's only ever one implementation of say "javax.security.enterprise.authentication.Pkdf2HashAlgorithm" in scope.

But javax.security.enterprise.authentication.pkdf2HashAlgorithm is a sub-interface of javax.security.enterprise.authentication.HashAlgorithm, so MyAppHashAlg implements that interface and the actual class is specified in the annotation. 

We did something similar for JSF 2.3 with great success.

So e.g.

@DatabaseIdentityStoreDefinition(

    ...

    hashAlgorithm = javax.security.enterprise.authentication.Pkdf2HashAlgorithm.class // interface extending HashAlgorithm

)

Then in the API:

package javax.security.enterprise.authentication;

public interface Pkdf2HashAlgorithm extends javax.security.enterprise.authentication.HashAlgorithm {

   // ..

} 

And the impl:

package org.glassfish...

public class SoteriaDefaultPkdf2HashAlgorithm implements Pkdf2HashAlgorithm  {

  // ...

}

For an application:

@DatabaseIdentityStoreDefinition(

    ...

    hashAlgorithm = MyAppHashAlg.class // class implementing HashAlgorithm

)

With in the application:

public class MyAppHashAlg implements HashAlgorithm{

  // ...

}

Kind regards,

Arjan Tijms

 

How does the DatabaseIdentityStore ask CDI for the correct implementation of the interface?

One way is by asking for a specific class. Another is by asking for a name.

It seems to me that relying on the classname exposes implementation details, and makes the app dependent on them, and therefore less portable.

There doesn't seem to be a type-safety issue with using a name, as all names must refer to a bean that implements the correct interface, which will always be the same (i.e., in my example below, "IdentityStorePasswordHashing"). As long as the runtime can find a named bean that implements the right interface, there is type safety. If the bean can't be found, there's a config error.

On the other hand, I may have an incomplete understanding of how this all could work, being new to CDI.

Will

On 07/19/2017 11:34 AM, Arjan Tijms wrote:

Hi,

On Wednesday, July 19, 2017, Will Hopkins <will.hopkins@...> wrote:

On 07/19/2017 10:13 AM, Guillermo González de Agüero wrote:

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

This exposes the platform's implementation, and is likely to be vendor specific. 

There's no need for that ;) Above I mentioned an interface, which is what the application developer can specify and the server can implement.

CDI unified the concepts. The JSR 375 runtime can ask CDI for either an interface or class without knowing which is which.

Kind regards,

Arjan Tijms

 

I don't want to have to know the name of the platform's impl class just to specify that I want the default algorithm -- or any other algorithm the platform offers.

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

It's true the we won't get IDE support for completing names, which is unfortunate. But we wouldn't necessarily get that using classnames, either, particularly when Java SE 9 comes along with modularity, and internal names become invisible.

Specifying the name of the algorithm you want seems like the right balance between simplicity, type safety, and flexibility, to me.

In practice, I'm guessing most platforms are going to offer only the default, or perhaps one or two simply-named alternatives -- e.g., "Argon2". Any more complicated names are likely to be provided by the application itself, and therefore be known to the developer.

All that said, I think most of you have more experience with CDI than I do -- let me know if I'm insisting on an anti-pattern. I just want to keep it simple for developers, and flexible and powerful for platforms and advanced developers, while avoiding exposing algorithm details or platform internals through the API.

Will

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

 

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

Hi,

On Wed, Jul 19, 2017 at 8:46 PM, Will Hopkins <will.hopkins@...> wrote:

It also means we'd need to define an interface for every algorithm we think will be supported, and account for all possible variations (PBKDF2 is a known, fixed, algorithm, but the choice of HMAC to use with it can vary, as can choices like salt size, # iterations, etc.). Or, leave unspecified what the interfaces for the other algorithms are, leaving very little that's "standard" about using them.

The "standard" part comes from being able to plug them into the @DatabaseIdentityStoreDefinition annotation.

Algorithms that are proven to be sufficiently successful and popular can be candidates for standardisation in JSR 375 in a next revision.

That's essentially how vendor features in JPA work as well via persistence.xml and/or query hints.

It also, as Guillermo points out, complicates the problem of an app supplying their own version of a "standard" algorithm.

I think it simplifies it really, you can plug-in whatever algorithm you want, be it a version of a standard algorithm or not. It just uses a different type name. 

"javax.security.enterprise.authentication.Pkdf2HashAlgorithm.class" is an interface that the implementation of JSR 375 on which you run implements and which you are not normally supposed to replace. If you want a better version you can use "org.better.Pkdf2HashAlgorithm.class" or whatever.

This is similar to JSF again; the standard data table for instance comes with your JSF implementation. You are not supposed to replace that with a better one. If you want to use a better one, you use a different data table component, like the PrimeFaces one.

Kind regards,

Arjan Tijms

 

I was just looking at the CDI spec, and see where using named beans is not recommended, so perhaps named beans really are an anti-pattern. But the value, to us, is that we can completely specify how this should work, and how apps would request an algorithm, while maintaining complete extensibility for future algorithm support.

It's also simpler to use EL to specify an algorithm name than a type. We could support it for types, using a hashAlgorithmExpression attribute, but then we'd need to write code to extract a classname from a string and instantiate it, possibly tripping over issues around what types are visible to the app vs. the DatabaseIdentityStore impl, etc. And if the names were fully qualified, there would likely be portability issues unless they were all specified in the javax.security.enterprise namespace (as opposed to, say org.glassfish).

I suppose we could go even further and define some sort of SPI/provider scheme, such as JCA does with providers for crypto algorithms, but that seems like overkill here, and more than we can easily specify in the next day or two ...

Will

On 07/19/2017 02:08 PM, Guillermo González de Agüero wrote:

But then, how to select another vendor provided algorithm without depending on its specific implementation?

Regards,

Guillermo González de Agüero

El mié., 19 de julio de 2017 19:49, Arjan Tijms <arjan.tijms@...> escribió:

Hi,

On Wed, Jul 19, 2017 at 6:07 PM, Will Hopkins <will.hopkins@...> wrote:

Right, but suppose there are multiple implementations of the interface, as is likely in this case (default PBKDF2, Argon2 implemented by some vendor, "MyAppHashAlg" implemented by the application).

This is via the "highlander rule" (there can be only one).

So there's only ever one implementation of say "javax.security.enterprise.authentication.Pkdf2HashAlgorithm" in scope.

But javax.security.enterprise.authentication.pkdf2HashAlgorithm is a sub-interface of javax.security.enterprise.authentication.HashAlgorithm, so MyAppHashAlg implements that interface and the actual class is specified in the annotation. 

We did something similar for JSF 2.3 with great success.

So e.g.

@DatabaseIdentityStoreDefinition(

    ...

    hashAlgorithm = javax.security.enterprise.authentication.Pkdf2HashAlgorithm.class // interface extending HashAlgorithm

)

Then in the API:

package javax.security.enterprise.authentication;

public interface Pkdf2HashAlgorithm extends javax.security.enterprise.authentication.HashAlgorithm {

   // ..

} 

And the impl:

package org.glassfish...

public class SoteriaDefaultPkdf2HashAlgorithm implements Pkdf2HashAlgorithm  {

  // ...

}

For an application:

@DatabaseIdentityStoreDefinition(

    ...

    hashAlgorithm = MyAppHashAlg.class // class implementing HashAlgorithm

)

With in the application:

public class MyAppHashAlg implements HashAlgorithm{

  // ...

}

Kind regards,

Arjan Tijms

 

How does the DatabaseIdentityStore ask CDI for the correct implementation of the interface?

One way is by asking for a specific class. Another is by asking for a name.

It seems to me that relying on the classname exposes implementation details, and makes the app dependent on them, and therefore less portable.

There doesn't seem to be a type-safety issue with using a name, as all names must refer to a bean that implements the correct interface, which will always be the same (i.e., in my example below, "IdentityStorePasswordHashing"). As long as the runtime can find a named bean that implements the right interface, there is type safety. If the bean can't be found, there's a config error.

On the other hand, I may have an incomplete understanding of how this all could work, being new to CDI.

Will

On 07/19/2017 11:34 AM, Arjan Tijms wrote:

Hi,

On Wednesday, July 19, 2017, Will Hopkins <will.hopkins@...> wrote:

On 07/19/2017 10:13 AM, Guillermo González de Agüero wrote:

Hi Will,

What do you think about using the implementation class instead of a bean name, wtih the spec providing a default PBKDF2IdentityStorePasswordHashing class?

This exposes the platform's implementation, and is likely to be vendor specific. 

There's no need for that ;) Above I mentioned an interface, which is what the application developer can specify and the server can implement.

CDI unified the concepts. The JSR 375 runtime can ask CDI for either an interface or class without knowing which is which.

Kind regards,

Arjan Tijms

 

I don't want to have to know the name of the platform's impl class just to specify that I want the default algorithm -- or any other algorithm the platform offers.

The problem I see with that is that people would depend on vendor specific classes at compile time to use a different algorithm. But it's worse with named beans, since every IDE provides support for autocompleting classes but it won't probably work for named beans there.

It's true the we won't get IDE support for completing names, which is unfortunate. But we wouldn't necessarily get that using classnames, either, particularly when Java SE 9 comes along with modularity, and internal names become invisible.

Specifying the name of the algorithm you want seems like the right balance between simplicity, type safety, and flexibility, to me.

In practice, I'm guessing most platforms are going to offer only the default, or perhaps one or two simply-named alternatives -- e.g., "Argon2". Any more complicated names are likely to be provided by the application itself, and therefore be known to the developer.

All that said, I think most of you have more experience with CDI than I do -- let me know if I'm insisting on an anti-pattern. I just want to keep it simple for developers, and flexible and powerful for platforms and advanced developers, while avoiding exposing algorithm details or platform internals through the API.

Will

Regards,

Guillermo González de Agüero

El mié., 19 jul. 2017 a las 15:50, Will Hopkins (<will.hopkins@...>) escribió:

A few points.

First, the intent was definitely to define a specific interface, which be the bean type that the DatabaseIdentityStore looked up by name -- name would identify only the instance, not the type. The interface would look like this:

public interface IdentityStorePasswordHashing {

    // used only by idstore impls that support password change,
    // to generate a new password hash.
    // generated hash is encoded as a string, can be stored directly in db.
    String generateHash(char[] password);

    // used for all hash verifications, to allow impls to support constant time compare,
    // be flexible about hash params, etc.
    // hashed password value is exactly as retrieved from db, still encoded.
    boolean verifyHash(char[] password, String hashedPassword);

}

The spec would require support for a named bean called "PBKDF2", with behavior defined by the spec. Server vendors and application developers would be free to provide implementations for other algorithms.

The interface above can support any algorithm, with any encoding. The details of of parameters -- salt size, algorithm iterations, etc. -- are up to the implementation to decide. For PBKDF2, the spec would set a minimum required default, but allow vendors to support other combos. In particular, it would allow vendors to support verification of legacy formats, while requiring new passwords to be hashed using the minimum secure values.

Most developers won't be in a position to choose secure parameter values for an algorithm, so it's generally better left to the server vendors, with a minimum floor set by the spec. Those who have need of a specific algorithm or a specific set of parameter values, can code it themselves and make their bean available.

Seems like the best of both worlds to me -- simple and secure for those who want that (just specify the name "PBKDF2"), but powerful enough to provide complete flexibility by implementing a simple interface as a CDI bean and referencing the bean's name in your config.

We could still support EL for determining the bean's name -- the output of the expression would be a string name used to look up a bean.

Will

On 07/19/2017 04:17 AM, Arjan Tijms wrote:

Hi,

I'm not a fan of just providing the named bean, but the .class should work.

With the interface, we could also provide PBKDF2 as a sub-interface that implementations (such as Soteria) can implement, and thus provide standard and custom algorithms via one mechanism.

For provided algorithms we really do need parameters in the annotation though. Any thoughts about the proposed name/value array? @DataSourceDefinition uses the same approach, so this should be familiar to users.

Kind regards,

Arjan Tijms

On Wednesday, July 19, 2017, Guillermo González de Agüero <z06.guillermo@...> wrote:

Hi,

I personally prefer your idea along with a new interface, since this would be more type safe.

The interface-less approach remembers me about the JSF backing-bean based validator (see [1] "Validation using a Method in Backing Bean" for an example) where users have to figure out the method signature.

I specially like Rudy's type-safe proposal setting a bean class instead of a bean name, like:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = PasswordVerifier.class

)

That class would be instantiated via CDI.select(PasswordVerifier.class).get() and would just require to implement said interface. For dynamic cases, a hashAlgorithmExpression could be provided, but I prefer setting the class in most cases.

Regards,

Guillermo González de Agüero

[1] https://incepttechnologies.blogspot.com.es/p/validation-in-jsf.html

El mar., 18 jul. 2017 a las 23:19, Will Hopkins (<will.hopkins@...>) escribió:

Hi Arjan,

Thanks for implementing this. I've actually been spending some time thinking about this as well, and was about to make a proposal -- didn't realize you were working on it, too -- and I wonder if it would be better to simply specify a named CDI bean as the hash function? It's simpler (I think) than specifying an expression, which seems like overkill when all we want to do is enable users to indicate a hashing implementation.

Also, I think it's probably better to specify a verify(password, hashedPassword) function -- which does the compare internally -- rather than leave it up to the DatabaseIdentityStore implementation to do the compare. That allows the author of the hash algorithm to, e.g., implement a constant-time compare function, to guard against timing attacks. Better, I think, to abstract all aspects of the crypto.

Thoughts?

(I also have some comments on the PBFDF2 parameters chosen.)

Will

On 07/18/2017 05:06 PM, Arjan Tijms wrote:

Hi,

I just did an initial implementation of the EL method expression proposal for the hash function and the default PBKDF2 hash.

See: https://github.com/javaee-security-spec/soteria/commit/7b7432794a493a186295d3e87211943e48f30502

The code looks as follows:

    if (hasAnyELExpression(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            ELContext elContext = CdiUtils.getELProcessor().getELManager().getELContext();

            

            MethodExpression hashMethodExpression = ExpressionFactory.newInstance().createMethodExpression(

                elContext, 

                dataBaseIdentityStoreDefinition.hashAlgorithm(), 

                String.class, new  Class<?>[] {String.class} );

            

            hashFunction =  s -> (String) hashMethodExpression.invoke(elContext, new Object[] {s});

        } else if ("PBKDF2".equals(dataBaseIdentityStoreDefinition.hashAlgorithm())) {

            hashFunction = s -> pbkdf2(s, salt);

        } else {

            hashFunction = identity();

        }

The Expression Language part creates a MethodExpression from the expression, which it then invokes via a lambda.

In the validate() method that lambda is called as follows:

  List<String> passwords = executeQuery(

            dataSource, 

            dataBaseIdentityStoreDefinition.callerQuery(),

            usernamePasswordCredential.getCaller()

        ); 

        

        String hashedPassword = hashFunction.apply(usernamePasswordCredential.getPasswordAsString());

        

        if (!passwords.isEmpty() && hashedPassword.equals(passwords.get(0))) {


The following shows an example of how application code uses this:

@DatabaseIdentityStoreDefinition(

    dataSourceLookup="${'java:global/MyDS'}", 

    callerQuery="#{'select password from caller where name = ?'}",

    groupsQuery="select group_name from caller_groups where caller_name = ?",

    hashAlgorithm = "#{applicationConfig.doHash}"

)

@ApplicationScoped

@Named

public class ApplicationConfig {

 

    public String doHash(String in) {

        return in; // can do anything here and get parameters from wherever

    }

    

}

The PBKDF2 hash is implemented as follows:

    public String pbkdf2(String password, byte[] salt) {

        try {

            return 

                Base64.getEncoder()

                      .encodeToString(

                          SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1")

                                          .generateSecret(

                                             new PBEKeySpec(password.toCharArray(), salt, 1024, 64 * 8))

                                          .getEncoded());

            

        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {

            throw new IllegalStateException(e);

        }

    }

Here we need some extra options to specify parameters, basically the salt and the number of iterations. I think the easiest way would be to have a key/value list called "hashAlgorithmParameters" or something in the annotation.

For example:

     * <p>

     *  Parameters are specified using the format:

     *  <i>parameterName=parameterValue</i>  with one parameter per array element.

     * <p>

 

     */

    String[] hashAlgorithmParameters() default {};

Thoughts?

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

 

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803