w3c-ccg / http-signatures

Signing HTTP Messages specification
https://w3c-dvcg.github.io/http-signatures/
Other
34 stars 9 forks source link

Sign content-type when digest is used. #36 #72

Open ioggstream opened 5 years ago

ioggstream commented 5 years ago

This PR

Fixes #36 explicitly signing Content-Type when the Digest HTTP Header is used.

Note

We are not forced to use Digest when sending authentication requests, though it helps increasing the cardinality of the signature input.

liamdennehy commented 5 years ago

This can be problematic. Usage of Digest in this specification is indicative of a way to incorporate the payload in a signature. It is not core to this specification, not required if the implementer does not choose to use it. If there are limitations in the effectiveness of that header they should be addressed to that specification directly.

Additionally, just specifying Content-Type is insufficient, as the most obvious attack is a length extension, so we should require Content-Length too, but now we're specifying yet another constraint on an external standard if we require that header. Note that neither of these headers are required for a receiver to understand the content, and especially simple clients like IoT may omit these headers entirely if the server is tolerant of their absence.

In general, we should secure a message as presented, not constrain implementations on how to generate that message to comply with this specification.

I don't think we should be getting too deep into guidance on such a wide range of topics, at least not in the core spec. There is a Security Considerations appendix (A) where this might be better placed, along with all other non-core aspects of how to use this spec effectively:

@msporny? Is this something we should be tackling?

ioggstream commented 5 years ago

It is not core to this specification, not required if the implementer does not choose to use it.

I agree: it would be reasonable to dis-entangle this spec from Digest and just show one complete example: this simplifies things a lot for this spec.

If there are limitations in the effectiveness of that header they should be addressed to that specification directly.

Yes, we're working on that via this draft.

we should require Content-Length too

Probably if we decide to base Digest on the new http-semantic this will be automatically fixed.

About security considerations: I'd add them on another issue/comment.

ioggstream commented 5 years ago

I'm not a crypto expert, so I have some questions about the length-extension attack:

iiuc a Length-Extension Attack under some conditions:

Question @liamdennehy:

Note

If we want to protect the Signature String, we should prefix it with its length before signing, eg introducing an hidden parameter (length):

(length): 212
(request-target): post /foo?param=value&pet=dog
host: example.com
date: Sun, 05 Jan 2014 21:31:40 GMT
content-type: application/json
digest: SHA-256=X48E9qOokqqrvdts8nOJRJN3OWDUoyWxBf7kbu9DBPE=
content-length: 18
liamdennehy commented 5 years ago

To reiterate: A Length Extension attack can occur when an attacker is able to control the plaintext of a message, and use it to compute either the same hash (in practice this is used as a MAC), or another hash that is meaningful to the recipient that will pass some validation.

As relates to the Digest header:

The most common case is padding with zeros/nulls, and adding information after that, something most JSON parsers will tolerate silently. After that NULL, parsing stops so an attacker can add octets at will without affecting the payload's legitimacy while manipulating the hash. Compare:

( echo -n '{"foo": "bar"}'; echo -n '{"another": "item")') | jq

(Should generate an error)

(echo -n '{"foo": "bar"}'; dd if=/dev/zero bs=1 count=1; echo -n '{"another": "item")') | jq
(echo -n '{"foo": "bar"}'; dd if=/dev/zero bs=1 count=1; dd if=/dev/urandom bs=1 count=32) 2>/dev/null | jq

(Should be parsed without an error, pipe to hexdump -C to compare raw payloads)

Explicitly stating both the content-type and content-length will help mitigate this, but again these are extensions to an external protocol (Digest), which we should not tackle directly but definitely incorporate in our advice if we feel it is necessary. This may not defend against all attacks e.g. audio/video content that is not parsed at the time the resource is accessed, but simply validated and saved but with a content-length unchanged (ie alterations inside the file that may or may not be valid for the file format). Again, not our issue.

As relates to the Signature-String:

The Signature-String is less vulnerable to this type of attack, since it is a derived string and so not as directly under the control of the attacker, as well as having very specific structure. In addition, header values are not valid with either a NULL or octets outside the set:

"!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA

Formulating a Signature-String by manipulating header values inside this character set that is not detectable by implementations MAY be harder, but that's not to say there isn't a HTTP Server/Client that won't silently truncate a header value as soon as it hits a NULL (which SHOULD be a 400-type error). We can't defend against bad implementations though. In all instances we expect headers to be meaningful and correct, so tampering for this purpose should be detectable as garbage/inappropriate values by the implementation.

I hope this was not too rambling...

liamdennehy commented 5 years ago

Specifically on adding a derived (length) value in the Signature-String, this would be somewhat defensive against extensions, but if an attacker has the computational resources to generate hashes to defeat the signature, altering the (length) parameter in each iteration (to match the now-extended length) would not be an expensive addition so provide minimal improvement in security.

ioggstream commented 5 years ago

all json examples do not validate with python3.6, but I understood your attack. I'd solve with the following:

$ which jq
jq ()
{ 
    /usr/bin/env python -mjson.tool
}

Not sure those applies to Digest

iiuc you are exploiting the json parser, not the digest-algorithm. So it doesn't seem to me the same L-E attack described in wp.

Digest validation happens:

If you think this applies, it'd be great if you could file an issue here https://github.com/ioggstream/draft-polli-resource-digests-http/issues !

signing messages as presented

forbidding multiple header instances/multi-line headers stepping outside the purpose of singing a message as presented.

Making an implementation easier to implement and more secure is better than forcefully support exotic cases (probably the only useful multiple header to be signed is x-forwarded-for which is, by design, unsuitable for signing because every intermediary can add a new one or rearrange as a single header containing a list).

(length) in the signature string

if an attacker has the computational resources to generate hashes to defeat the signature

he would rather mine BTC :P

dlongley commented 5 years ago

@liamdennehy,

To reiterate: A Length Extension attack can occur when an attacker is able to control the plaintext of a message, and use it to compute either the same hash (in practice this is used as a MAC), or another hash that is meaningful to the recipient that will pass some validation.

The Digest value would be included in the Signature-String that is digitally signed. My assertion is that attempting to modify this value via a length extension attack would cause the Signature-String to change and, therefore, signature verification to fail. Therefore, I don't think there is an issue. Why is this wrong?

...but if an attacker has the computational resources to generate hashes to defeat the signature

There is no reason to consider this. The security of the hash mechanism requires that the attacker cannot do this. If an attacker can do this, then the hash is not secure and must be replaced with a different algorithm.

I should add that requiring that Content-Length be signed would eliminate the ability to do things like chunked transfer-encoding, which is quite common. I don't think this is necessary or desirable.

ioggstream commented 5 years ago

I should add that requiring that Content-Length be signed would eliminate the ability to do things like chunked transfer-encoding, which is quite common. I don't think this is necessary or desirable.

This is true. Me and @LPardue are gathering some Security Considerations about using Digest in signatures.

I think we can simplify stuff a lot dis-entangling this spec from Digest usage as possible, providing just one complete example of signing Digest, Content-Type, Content-Encoding like specified above.

dlongley commented 5 years ago

@ioggstream,

I think we can simplify stuff a lot dis-entangling this spec from Digest usage as possible, providing just one complete example of signing Digest, Content-Type, Content-Encoding like specified above.

Sounds good. It is important to remember that Web applications (i.e., code running in browsers) are prohibited from setting certain headers, such as Content-Length (see the Fetch spec for a full list), so we should try very hard to avoid recommending anything that would rely on using these headers.

It's also the case that things like a length extension attack are a concern for the Digest spec, but not here, so disentangling is great. In fact, I believe this spec provides the mitigation for that attack via HMAC or asymmetric digital signatures. The Digest spec might want to talk about using the HTTP signatures spec as a means of dealing with that problem if the hash algorithm used is susceptible to it and it is a concern.

ioggstream commented 5 years ago

Web applications (i.e., code running in browsers) are prohibited from setting certain headers, such as Content-Length (see the Fetch spec for a full list), so we should try very hard to avoid recommending anything that would rely on using these headers.

Ok.

this spec provides the mitigation for that attack via HMAC or asymmetric digital signatures. The Digest spec might want to talk about using the HTTP signatures spec as a means of dealing with that problem

As Digest does not provide Authentication nor Header Integrity we mention TLS and signatures in the Security considerations.

About signatures collisions, I just found this interesting article today https://eprint.iacr.org/2019/130.pdf