HttpEce.buildInfo(...) possible Nullpointer

[Famberlin]

Hi there, in HttpEce.deriveKey(...) method you have following flow:

byte[] secret = null;
byte[] context = null;//follow this one
if (key != null) {
    secret = key;
} else if (dh != null) {
    byte[][] bytes = deriveDH(keyId, dh);
    secret = bytes[0];
    context = bytes[1]; //the only place you initialize context
} else if (keyId != null) {
    secret = keys.get(keyId).getPublic().getEncoded();
}
// no more changes to the context var
keyinfo = buildInfo("aesgcm", context);

and in buildinfo method you have immediately:

protected static byte[] buildInfo(String type, byte[] context) {
    ByteBuffer buffer = ByteBuffer.allocate(19 + type.length() + context.length);

so it seems that the only way to Not get NullPointer is to have key == null && dh != null, so first & third conditions will never work anyway, and in code key is indeed passed as null when used. Maybe I'm missing a bigger picture here?

[martijndwars]

You're right, I wonder why this NPE did not pop up earlier. Were you actually confronted with an NPE?

Note that the HttpEce standard and WebPush standard are two separate standards. WebPush depends on HttpEce, but there may be an error in our implementation of HttpEce that does not affect WebPush. Nevertheless, I'd like to thank you for opening the issue, this is something we should fix.

[Famberlin]

I was just rewriting your library to kotlin so this stuff highlights rather quickly. No NPE for webpush was due to usage in library:

httpEce.encrypt(buffer, salt, null, "server-key-id", userPublicKey, userAuth, padSize);

that way you always get null passed for key. By the way, I did not look deeply into the standart, but does "server-key-id" play any role for webpush or it is just a part of HttpsEce standard that is not used for WebPush?

[martijndwars]

First, let me get back at the NPE. The HttpEce implementation is based on draft-ietf-httpbis-encryption-encoding-02, but as you can see the optional context string has been dropped since draft-ietf-httpbis-encryption-encoding-04. Modern implementations of Encrypted Content-Encoding for HTTP can drop the context part.

By the way, I did not look deeply into the standart, but does "server-key-id" play any role for webpush or it is just a part of HttpsEce standard that is not used for WebPush?

No, this is an implementation detail. If I recall correctly, the intention was to allow multiple keys to be configured and have the user specify the key being used. I should refactor this some day.

[Famberlin]

Yes, you are right, in my case it looks like this:

fun buildInfo(type: String, context: ByteArray?): ByteArray =
    "Content-Encoding: $type${'\u0000'}".toByteArray() + (context ?: byteArrayOf())

in case of Java you just add 2 checks(allocation and put).