[aerogear-dev] Push server...master secrets, secrets and some refactoring proposal

[Bruno Oliveira]

Good morning, while I was working on "that" passphrase encryption thing,
I found some improvements for UPS. But would like to discuss before.

1. Currently UPS stores master secret and secrets (from Variants) into
the database. IMO this information should never be persisted into the
database and such kind of information only belongs to the owner of that
application. How?
https://github.com/abstractj/aerogear-unifiedpush-server/tree/master-secret

2. Master secret and Secret make use of UUID from Java, not truly a PRNG
once some attributes are predictable, you are not collision free. "WTF
are you saying Bruno!?". From JDK 7:

|public static UUID randomUUID() {
        SecureRandom ng = numberGenerator;
        if (ng == null) {
            numberGenerator = ng = new SecureRandom();
        }

        byte[] randomBytes = new byte[16];
        ng.nextBytes(randomBytes);
        randomBytes[6]  &= 0x0f;  /* clear version        */
        randomBytes[6]  |= 0x40;  /* set to version 4     */
        randomBytes[8]  &= 0x3f;  /* clear variant        */
        randomBytes[8]  |= 0x80;  /* set to IETF variant  */
        return new UUID(randomBytes);
    }|

 Solution:
https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293#diff-dc07653bd3c83d4e82c45b70d8ad34dfR47

3. I noticed the fact that PushApplication and Variant have some
duplicated attributes with different names.

public abstract class Variant extends BaseModel {

    private String variantID = UUID.randomUUID().toString();

    private String secret = UUID.randomUUID().toString();

....
}

public class PushApplication extends BaseModel {

    private String pushApplicationID = UUID.randomUUID().toString();

    private String masterSecret = UUID.randomUUID().toString();
...
}

I can see two alternatives to avoid it:

- Move these attributes to an |Embeddable| class, something like:

|@Embeddable
*public class ApplicationKey*{

|    private String id = UUID.randomUUID().toString();

    private String secret = UUID.randomUUID().toString();
|
}|

public abstract class Variant extends BaseModel {

|    @Embedded
    @AttributeOverrides( {
        @AttributeOverride(name="variantID")),
        @AttributeOverride(name="secret"))
    })
    *private ApplicationKey*key;|

....
}

public class PushApplication extends BaseModel {

    |@Embedded
    @AttributeOverrides( {
        @AttributeOverride(name="pushApplicationID")),
        @AttributeOverride(name="masterSecret"))
    })
    *private ApplicationKey*key;|
...
}

- Second alternative (more simple) pull up the attributes to BaseModel

public abstract class BaseModel implements Serializable {

    private String id = UUID.randomUUID().toString();

    private String secret = UUID.randomUUID().toString(); 
...
}

Why this is important?

1. Avoid sensitive data to be stored into the database, make use of KDF
functions to store secrets, master secrets....
2. Avoid code duplication. With the inclusion of encryption for
passphrases and other kind of sensitive data, I pretty much have to
duplicate code like this
(https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293)
between both classes.

Wdyt?


-- 
abstractj

 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
Url : http://lists.jboss.org/pipermail/aerogear-dev/attachments/20140416/1b3c353f/attachment.bin