<div dir="ltr">Hi Bruno ! <div>Cool stuff some comments inline</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Apr 16, 2014 at 7:06 AM, Bruno Oliveira <span dir="ltr"><<a href="mailto:bruno@abstractj.com" target="_blank">bruno@abstractj.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Good morning, while I was working on "that" passphrase encryption thing,<br>
I found some improvements for UPS. But would like to discuss before.<br>
<br>
1. Currently UPS stores master secret and secrets (from Variants) into<br>
the database. IMO this information should never be persisted into the<br>
database and such kind of information only belongs to the owner of that<br>
application. How?<br>
<a href="https://github.com/abstractj/aerogear-unifiedpush-server/tree/master-secret" target="_blank">https://github.com/abstractj/aerogear-unifiedpush-server/tree/master-secret</a></blockquote><div><br></div><div>I tried out your branch and created some applications.</div>
<div>This is what I see now when browsing to my app details, is this the valid ID/Secret pair that the application owner / administrator can use to send psuh notifications ?</div><div> </div><div><img src="http://s4.postimg.org/di81izlz1/secret.png" alt="secret"></div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
<br>
2. Master secret and Secret make use of UUID from Java, not truly a PRNG<br>
once some attributes are predictable, you are not collision free. "WTF<br>
are you saying Bruno!?". From JDK 7:<br>
<br>
|public static UUID randomUUID() {<br>
SecureRandom ng = numberGenerator;<br>
if (ng == null) {<br>
numberGenerator = ng = new SecureRandom();<br>
}<br>
<br>
byte[] randomBytes = new byte[16];<br>
ng.nextBytes(randomBytes);<br>
randomBytes[6] &= 0x0f; /* clear version */<br>
randomBytes[6] |= 0x40; /* set to version 4 */<br>
randomBytes[8] &= 0x3f; /* clear variant */<br>
randomBytes[8] |= 0x80; /* set to IETF variant */<br>
return new UUID(randomBytes);<br>
}|<br>
<br>
Solution:<br>
<a href="https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293#diff-dc07653bd3c83d4e82c45b70d8ad34dfR47" target="_blank">https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293#diff-dc07653bd3c83d4e82c45b70d8ad34dfR47</a></blockquote>
<div><br></div><div>Nice improvement </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
<br>
3. I noticed the fact that PushApplication and Variant have some<br>
duplicated attributes with different names.<br>
<br>
public abstract class Variant extends BaseModel {<br>
<br>
private String variantID = UUID.randomUUID().toString();<br>
<br>
private String secret = UUID.randomUUID().toString();<br>
<br>
....<br>
}<br>
<br>
public class PushApplication extends BaseModel {<br>
<br>
private String pushApplicationID = UUID.randomUUID().toString();<br>
<br>
private String masterSecret = UUID.randomUUID().toString();<br>
...<br>
}<br>
<br>
I can see two alternatives to avoid it:<br>
<br>
- Move these attributes to an |Embeddable| class, something like:<br>
<br>
|@Embeddable<br>
*public class ApplicationKey*{<br>
<br>
| private String id = UUID.randomUUID().toString();<br>
<br>
private String secret = UUID.randomUUID().toString();<br>
|<br>
}|<br>
<br>
public abstract class Variant extends BaseModel {<br>
<br>
| @Embedded<br>
@AttributeOverrides( {<br>
@AttributeOverride(name="variantID")),<br>
@AttributeOverride(name="secret"))<br>
})<br>
*private ApplicationKey*key;|<br>
<br>
....<br>
}<br>
<br>
public class PushApplication extends BaseModel {<br>
<br>
|@Embedded<br>
@AttributeOverrides( {<br>
@AttributeOverride(name="pushApplicationID")),<br>
@AttributeOverride(name="masterSecret"))<br>
})<br>
*private ApplicationKey*key;|<br>
...<br>
}<br>
<br>
- Second alternative (more simple) pull up the attributes to BaseModel<br>
<br>
public abstract class BaseModel implements Serializable {<br>
<br>
private String id = UUID.randomUUID().toString();<br>
<br>
private String secret = UUID.randomUUID().toString();<br>
...<br>
}<br></blockquote><div><br></div><div>Make sense </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Why this is important?<br>
<br>
1. Avoid sensitive data to be stored into the database, make use of KDF<br>
functions to store secrets, master secrets....<br>
2. Avoid code duplication. With the inclusion of encryption for<br>
passphrases and other kind of sensitive data, I pretty much have to<br>
duplicate code like this<br>
(<a href="https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293" target="_blank">https://github.com/abstractj/aerogear-unifiedpush-server/commit/a9993c35edc39501eef7d7a26f3d8ea29b38e293</a>)<br>
between both classes.<br>
<br>
Wdyt?<br>
<span class=""><font color="#888888"><br>
<br>
--<br>
abstractj<br>
<br>
<br>
<br>
</font></span><br>_______________________________________________<br>
aerogear-dev mailing list<br>
<a href="mailto:aerogear-dev@lists.jboss.org">aerogear-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/aerogear-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/aerogear-dev</a><br></blockquote></div><br></div></div>