SEC-1472: Add support for bcrypt password encoding #1711

Closed
spring-issuemaster opened this Issue Apr 30, 2010 · 6 comments

Comments

Projects
None yet
1 participant
@spring-issuemaster

Taylor Leese (Migrated from SEC-1472) said:

It would be great to have build-in support for bcrypt hashing in Spring Security. I would foresee the usage to be something similar to below and then add a BcryptPasswordEncoder (similar to ShaPasswordEncoder).

        <password-encoder hash="bcrypt" rounds="10">
            <salt-source user-property="someProperty" />
        </password-encoder>

This is a good example of custom bcrypt usage with Spring Security: http://sziebert.net/posts/using-bcrypt-with-spring-security/

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster May 4, 2010

Luke Taylor said:

I'm against doing this for the time being. We get requests from time to time to support less mainstream password-encoding algorithms but I don't want to add additional external core dependencies for what is really a niche requirement. It's also significant that the bCrypt implementation referred to had a serious vulnerability as recently as February of this year. If we are adding external dependencies on cryptography libraries I would prefer them to have seen more mainstream usage/scrutiny.

People tend to focus too much on issues like "which hashing/encryption algorithm is 'best'" when this is unlikely to be the main vulnerability of a system. If you are using SHA hashes with random salt values then this should not be a major focus area.

It is also trivial to implement the PasswordEncoder interface if users have specific requirements, and custom implementations are easily used with the namespace.

Luke Taylor said:

I'm against doing this for the time being. We get requests from time to time to support less mainstream password-encoding algorithms but I don't want to add additional external core dependencies for what is really a niche requirement. It's also significant that the bCrypt implementation referred to had a serious vulnerability as recently as February of this year. If we are adding external dependencies on cryptography libraries I would prefer them to have seen more mainstream usage/scrutiny.

People tend to focus too much on issues like "which hashing/encryption algorithm is 'best'" when this is unlikely to be the main vulnerability of a system. If you are using SHA hashes with random salt values then this should not be a major focus area.

It is also trivial to implement the PasswordEncoder interface if users have specific requirements, and custom implementations are easily used with the namespace.

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster May 4, 2010

Taylor Leese said:

Understandable. What about support for SHA-512? It doesn't look like that is currently supported.

Taylor Leese said:

Understandable. What about support for SHA-512? It doesn't look like that is currently supported.

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster Dec 20, 2010

Udai Gupta said:

latest bcrypt 0.3 has solved the issue reported in feb

Udai Gupta said:

latest bcrypt 0.3 has solved the issue reported in feb

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster Nov 2, 2011

Dave Syer said:

I sent a pull request.

Dave Syer said:

I sent a pull request.

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster Nov 2, 2011

Luke Taylor said:

Implementation added by Dave Syer.

Luke Taylor said:

Implementation added by Dave Syer.

@spring-issuemaster

This comment has been minimized.

Show comment
Hide comment
@spring-issuemaster

spring-issuemaster Jan 15, 2012

Taylor Leese said:

It would be nice to have bcrypt as a password-encocer hash. Looks like it needs to be added to the schema definition in spring-security-3.1.xsd. I'm sure there are other corresponding changes as well.

<xs:attributeGroup name="hash">
<xs:attribute name="hash" use="required">
xs:annotation
xs:documentationDefines the hashing algorithm used on user passwords. We recommend strongly against using MD4, as it is a very weak hashing algorithm./xs:documentation
/xs:annotation
xs:simpleType
<xs:restriction base="xs:token">
<xs:enumeration value="plaintext"/>
<xs:enumeration value="sha"/>
<xs:enumeration value="sha-256"/>
<xs:enumeration value="md5"/>
<xs:enumeration value="md4"/>
<xs:enumeration value="{sha}"/>
<xs:enumeration value="{ssha}"/>
/xs:restriction
/xs:simpleType
/xs:attribute
/xs:attributeGroup

Taylor Leese said:

It would be nice to have bcrypt as a password-encocer hash. Looks like it needs to be added to the schema definition in spring-security-3.1.xsd. I'm sure there are other corresponding changes as well.

<xs:attributeGroup name="hash">
<xs:attribute name="hash" use="required">
xs:annotation
xs:documentationDefines the hashing algorithm used on user passwords. We recommend strongly against using MD4, as it is a very weak hashing algorithm./xs:documentation
/xs:annotation
xs:simpleType
<xs:restriction base="xs:token">
<xs:enumeration value="plaintext"/>
<xs:enumeration value="sha"/>
<xs:enumeration value="sha-256"/>
<xs:enumeration value="md5"/>
<xs:enumeration value="md4"/>
<xs:enumeration value="{sha}"/>
<xs:enumeration value="{ssha}"/>
/xs:restriction
/xs:simpleType
/xs:attribute
/xs:attributeGroup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment