Switch from MD5 to PBKDF2 for AES keys

[aaronjwood]

I'd like to propose that Macaron switch from using MD5 to PBKDF2 for its AES keys

macaron/context.go

Line 443 in 78521e4

m := md5.Sum([]byte(secret))

I think using MD5 to generate AES keys has some serious flaws. For example, there's a good chance that someone could figure out the AES key in a few seconds if they have a good rainbow table. Also, you can generate MD5 hashes like crazy on consumer-level GPUs (1-5 billion hashes per second easy on a SINGLE GPU that's less than $500) so the key becomes very easy to attack. PBKDF2 is slow (key stretching) and specifically designed for this use case since it's a key derivation function.

Related to gogs/gogs#4117

[aaronjwood]

unknwon/com#11 is slightly related to this.

[aaronjwood]

Do you want to use AES-128 or AES-256 for this? If I generate a 16-byte key via PBKDF2 that'll be AES-128. 32 byte key will be AES-256. Also, how do you want to handle the salt? Where would you be feeding that from? Can I assume that it'll be sent from the caller so that we can change the signature to func (ctx *Context) SetSuperSecureCookie(secret, salt, name, value string, others ...interface{})?

[unknwon]

Do you want to use AES-128 or AES-256 for this?

I think AES-256 is good.

Also, how do you want to handle the salt? Where would you be feeding that from?

The secret from Gogs is slat + password, and change API is not a good idea..

[aaronjwood]

Sounds good.

PBKDF2 needs the password, salt, iteration count, key length, and a hash function as its inputs. If we change the signature here can we just change Gogs to pass things in separately? I can't pass the salt+password as a single parameter to PBKDF2.

[unknwon]

I can't pass the salt+password as a single parameter to PBKDF2.

In that case... I think we should use something simple in Macaron, but write Gogs's version of SetSuperSecureCookie (not use Macaron's SetSuperSecureCookie anymore) so we don't need to care about API breaks.

[aaronjwood]

Hmm, okay so what do you want to do for Macaron? Just switch to SHA2 for now?
When you say write Gogs's version do you mean to just edit the vendor that's committed as part of the repo itself?

[unknwon]

Hmm, okay so what do you want to do for Macaron? Just switch to SHA2 for now?

Yes, I think so.

When you say write Gogs's version do you mean to just edit the vendor that's committed as part of the repo itself?

Not vendor, but somewhere here, this is very specific to codebase, for example:

// modules/context/context.go

// Context represents context of a request.
type Context struct {
    ...
}

func (ctx *Context) SetSuperSecureCookie(secret, salt, name, value string, others ...interface{}) {
    ... // Gogs's version of function
}

[unknwon]

Just to check, is the PR coming soon? Because right now users are not able to compile Macaron...

[aaronjwood]

Just opened #120

[unknwon]

I guess this can be closed?