Topics

LDAP Annotation and Database Hashing Proposal


Will Hopkins
 

I've made some changes to the DatabaseIdentityStoreDefinition and LdapIdentityStoreDefinition on a branch; I'd appreciate review/comments by anyone with time to look at them. The changes are on a branch in the security-api repo (as separate commits):

https://github.com/javaee-security-spec/security-api/tree/wmhopkins-pfd

The database change is to add support for a default PBKDF2 hashing algorithm, and remove the hash encoding attribute, which seems to me bound up with the implementation of the hashing algorithm and therefore not very useful as a separate attribute. I'm supplying PBKDF2 as a string, not as an enum, to allow for using EL expressions instead; it may be appropriate to define a string constant, but kept it simple to start. I also fixed a change (caller -> callers) that got lost while merging Guillermo's changes earlier.

The LDAP change renames a few of the attributes to make their purpose more clear (e.g., baseDn -> bindDn), adds richer support for caller/group lookups vs. direct binding, supports getting groups from a user's "memberOf" attribute as well as by looking up group attributes, cleans up the javadoc a little, and removes a few bits of javadoc that were asserting a particular relationship between base DNs and user/group objects that isn't necessarily correct if users/groups are in a tree rather than a flat space.

The only attributes that are not string based are priority and validationTypes. I did not add additional methods for supporting EL for these, but I'm happy to do so if someone will propose an appropriate syntax/naming convention.

We should also document the ability to use EL expressions; I haven't done that yet because I wasn't sure of the decision on ${} vs #{}. I like the idea of supporting both, but I'm concerned about the implementation cost and the testing cost -- both RI and TCK need to be wrapped up soon, as I believe they have to be delivered, along with the spec, as part of the materials for the Final Approval Ballot, which is coming up very quickly.

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


Arjan Tijms
 

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
 

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


Rudy De Busscher
 

Hi Will,

If I'm following correctly, only specifying a named CDI bean will lead then to (for ex based on Arjans example)

@DatabaseIdentityStoreDefinition(
    dataSourceLookup="${'java:global/MyDS'}", 
    callerQuery="#{'select password from caller where name = ?'}",
    groupsQuery="select group_name from caller_groups where caller_name = ?",
    hashAlgorithm = "applicationConfig"
)

or even this is possible (type safety)

@DatabaseIdentityStoreDefinition(
    dataSourceLookup="${'java:global/MyDS'}", 
    callerQuery="#{'select password from caller where name = ?'}",
    groupsQuery="select group_name from caller_groups where caller_name = ?",
    hashAlgorithm = ApplicationConfig.class
)

But this adds more requirements/confusing
- Developer is confused about the EL, because it doesn't see the typical ${} or #{}
- We need to define some interface because otherwise the code has no clue which method needs to be executed.

So for me, the example of Arjan is much clear and easier for both developer and spec.

best regards
Rudy


On 18 July 2017 at 23:19, Will Hopkins <will.hopkins@...> wrote:
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



Guillermo González de Agüero
 

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

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


Arjan Tijms
 

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
 

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


Guillermo González de Agüero
 

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
 



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


Arjan Tijms
 

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
 

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
 

Perhaps I should ask a different question:  What's the advantage of using a classname (more properly, a type), over a bean name?

On 07/19/2017 12:07 PM, 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).

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


Arjan Tijms
 

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



Guillermo González de Agüero
 

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
 

That also requires sub-classing the interface, which seems unnecessary, because there's nothing special about any other algorithm that would require doing so. The implementations can/will vary, but the interface visible to callers should always be the same.

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. It also, as Guillermo points out, complicates the problem of an app supplying their own version of a "standard" algorithm.

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


Arjan Tijms
 

Hi,

On Wed, Jul 19, 2017 at 8:08 PM, Guillermo González de Agüero <z06.guillermo@...> wrote:
But then, how to select another vendor provided algorithm without depending on its specific implementation?

Well, PBKDF2 is part of JSR 375, so you wouldn't normally mix implementations from others vendors, if that's what you mean at least.

I.e. normally you wouldn't say use the navigation rules from MyFaces with Mojarra either. Likewise, say if JBoss implements its own version of JSR 375, then I don't think we should expect that their implementation of PBKDF2 would be usable with Soteria just like that. As another example, I don't think we'd expect that the DatabaseIdentityStore of that hypothetical JBoss implementation would be directly usable with Soteria either (as it can depend on much JBoss internal stuff for instance).

For all other algorithms it simply depends on what the vendor in questions exports. It can be the class or an interface.

E.g. on JBoss:

@DatabaseIdentityStoreDefinition(
    ...
    hashAlgorithm = JBossSuperAlgorithm.class // class implementing HashAlgorithm
)

Kind regards,
Arjan Tijms

 

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




Arjan Tijms
 

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
 

I guess I just don't see the value proposition for using interfaces. It complicates implementation for both vendors and application developers, making it more brittle, without adding anything useful that I can see.

In the end, these interfaces function exactly like names; they're purely marker interfaces. They don't provide any type safety beyond what the parent interface provides, and they complicate the EL support (since the DatabaseIdentityStore will have to handle EL expressions that generate type names).

If there's a compelling reason to avoid named beans I'm open to alternatives, including IF types, but so far I haven't heard the argument against names, other than the line in the CDI spec about not recommending them, for unspecified reasons.

Additional comments inline.

Will

On 07/19/2017 05:29 PM, Arjan Tijms wrote:
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.

Perhaps. But I'm not sure there's much to standardize beyond the top level algorithm name (PBKDF2, Argon2, etc.). There's always going to be infinite variation driven by the requirements of different environments for security and legacy support.

The value of specifying PBKDF2 is so that apps can rely on there being at least one default implementation that's reasonably secure and available everywhere.

That's essentially how vendor features in JPA work as well via persistence.xml and/or query hints.
You mean that functions/behaviors are defined/declared as type names? Or that vendor-specific features are gradually standardized?
 
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.
I get that, but named beans would seemingly achieve the exact same result, with simpler implementation and usage. I yet haven't heard the argument for why names aren't a good idea.

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.
I think that's a different use case.

For hashing algorithms, it's not really a case of "better" or "worse" implementations, and there's no point trying to discourage the use of other algorithms. The default impl isn't going to work for everyone. It's there to provide a reasonable default that portable applications can rely on, but many platform vendors, and many applications, will have specific requirements that force them to use a different algorithm -- or, more likely, a variation on an existing algorithm. It's not a case of better or worse, it's a case of application-specific requirements. To the extent that a set of common implementations emerge that are widely used, it's likely to result from flexible implementations that can adapt to a lot of variation (or from reliance on the default).


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


Rudy De Busscher
 

I guess I just don't see the value proposition for using interfaces. It complicates implementation for both vendors and application developers, making it more brittle, without adding anything useful that I can see.
If there's a compelling reason to avoid named beans I'm open to alternatives, including IF types, but so far I haven't heard the argument against names, other than the line in the CDI spec about not recommending them, for unspecified reasons.

Well, Strings aren't type safe, do not play well during refactoring and thus are very brittle for me.

If we use interfaces, like the JBoss example from Arjan,

@DatabaseIdentityStoreDefinition(
    ...
    hashAlgorithm = JBossSuperAlgorithm.class // class implementing HashAlgorithm
)

The compiler can check that the JBossSuperAlgorithm implements at least the HashAlgorithm.

Strings (like EL expressions) and named CDI beans doesn't have that capability. We will only discover at runtime if it is usable or not.

With the following pseudocode, we can retrieve whatever algorithm/implementation is specified, and we know for sure that it is the correct type.

HashAlgorithm hashAlgo = BeanProvider.getContextualReference(definition.hashAlgorithm()) 

And for the vendors, if their implementation is based on the correct interface, it will integrate. I don't see any other difficulty.

Best regards
Rudy


On 20 July 2017 at 05:34, Will Hopkins <will.hopkins@...> wrote:
I guess I just don't see the value proposition for using interfaces. It complicates implementation for both vendors and application developers, making it more brittle, without adding anything useful that I can see.

In the end, these interfaces function exactly like names; they're purely marker interfaces. They don't provide any type safety beyond what the parent interface provides, and they complicate the EL support (since the DatabaseIdentityStore will have to handle EL expressions that generate type names).

If there's a compelling reason to avoid named beans I'm open to alternatives, including IF types, but so far I haven't heard the argument against names, other than the line in the CDI spec about not recommending them, for unspecified reasons.

Additional comments inline.

Will

On 07/19/2017 05:29 PM, Arjan Tijms wrote:
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.

Perhaps. But I'm not sure there's much to standardize beyond the top level algorithm name (PBKDF2, Argon2, etc.). There's always going to be infinite variation driven by the requirements of different environments for security and legacy support.

The value of specifying PBKDF2 is so that apps can rely on there being at least one default implementation that's reasonably secure and available everywhere.

That's essentially how vendor features in JPA work as well via persistence.xml and/or query hints.
You mean that functions/behaviors are defined/declared as type names? Or that vendor-specific features are gradually standardized?
 
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.
I get that, but named beans would seemingly achieve the exact same result, with simpler implementation and usage. I yet haven't heard the argument for why names aren't a good idea.

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.
I think that's a different use case.

For hashing algorithms, it's not really a case of "better" or "worse" implementations, and there's no point trying to discourage the use of other algorithms. The default impl isn't going to work for everyone. It's there to provide a reasonable default that portable applications can rely on, but many platform vendors, and many applications, will have specific requirements that force them to use a different algorithm -- or, more likely, a variation on an existing algorithm. It's not a case of better or worse, it's a case of application-specific requirements. To the extent that a set of common implementations emerge that are widely used, it's likely to result from flexible implementations that can adapt to a lot of variation (or from reliance on the default).



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



Guillermo González de Agüero
 

Hi,

El mié., 19 jul. 2017 a las 23:30, Arjan Tijms (<arjan.tijms@...>) escribió:
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.
But in JSF you just "request" a Datatable, and the runtime maps that component to the needed class. Which class is backing the component is left as an implementation detail and you don't need to know about it.

But here, we would need to know the class coupling it to a specific vendor. So if I have:

@DatabaseIdentityStoreDefinition(
    ...
    hashAlgorithm = JBossSuperAlgorithm.class // class implementing HashAlgorithm
)

Then the application will fail to deploy on GlassFish as it won't find the needed class. That's the main problem I see with classes.

On the other hand, named beans provide a "false" portability ilusion, since each vendor will name its algorithms with a different strategy. I can imagine a "sha512" bean and then an "enchancedSha512" (just an example) and applications will suffer from the same portability issues.

In either case, people won't likely want to become vendor dependent, so the most probable scenario for legacy applications is that everyone will implement their needed algorithms (md5, sha1, etc) on its own application or in in-house libraries. And for that case, I prefer classes over named beans. An alternative "hashAlgorithmExpression" could be provided that would provide enough flexibility for more complex cases.

As for the possible modularity problems, does anybody know if there will be any difference between the two approaches? That's a real thing to keep in mind I think, although I imagine vendors will export those algorithm classes if they are to be used by end users.


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