New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIXBUG:4243 - use hashes to verify bundled packages by default. #46

Merged
merged 1 commit into from Jun 3, 2014

Conversation

Projects
None yet
4 participants
@robmen
Member

robmen commented May 27, 2014

Told the story in the wix.xsd.

have an Authenticode signature then the Bundle will use a hash of the package instead. Set this attribute to "yes"
to suppress the default behavior and force the Bundle to always use the hash of the package even when the package
is signed.
By default, a Bundle will use the hash of a package to verify its contents. If this attribute is explicitly set to "no"

This comment has been minimized.

@heaths

heaths May 27, 2014

Contributor

It's not really in the spirit of the change. I think changing the default is good, but in my fix within our internal "fork" we always verify the hash because the Authenticode check isn't good enough if an end user screws up and puts a layout in the same location as another bundle with validly signed packages.

Perhaps in addition to these changes, the code change to always verify the hash should be made. If the default value of "yes" is set to "no", the bundle will check the hash twice (effectively), but the hash check should always be done.

@heaths

heaths May 27, 2014

Contributor

It's not really in the spirit of the change. I think changing the default is good, but in my fix within our internal "fork" we always verify the hash because the Authenticode check isn't good enough if an end user screws up and puts a layout in the same location as another bundle with validly signed packages.

Perhaps in addition to these changes, the code change to always verify the hash should be made. If the default value of "yes" is set to "no", the bundle will check the hash twice (effectively), but the hash check should always be done.

This comment has been minimized.

@barnson

barnson May 28, 2014

Member

Is it still true if you drop "By default" from "By default, a Bundle will use the hash of a package to verify its contents."? IOW, we always verify the hash and can optionally not verify the signature. Or is there ever a case that Burn verifies nothing?

@barnson

barnson May 28, 2014

Member

Is it still true if you drop "By default" from "By default, a Bundle will use the hash of a package to verify its contents."? IOW, we always verify the hash and can optionally not verify the signature. Or is there ever a case that Burn verifies nothing?

This comment has been minimized.

@rseanhall

rseanhall May 28, 2014

Member

Someone could be relying on the fact that Burn doesn't currently check the hash. They could purposely be putting in a new file that has a valid signature but a different hash. I don't think we can always check the hash in 3.x.

@rseanhall

rseanhall May 28, 2014

Member

Someone could be relying on the fact that Burn doesn't currently check the hash. They could purposely be putting in a new file that has a valid signature but a different hash. I don't think we can always check the hash in 3.x.

This comment has been minimized.

@robmen

robmen May 28, 2014

Member

@barnson, there is never a case where Burn verifies nothing. Either it does hash verification or it does Authenticode signature verification. Is that unclear?

@robmen

robmen May 28, 2014

Member

@barnson, there is never a case where Burn verifies nothing. Either it does hash verification or it does Authenticode signature verification. Is that unclear?

This comment has been minimized.

@robmen

robmen May 28, 2014

Member

@rseanhall exactly. This is the largest change we thought we could take in WiX v3.x. In WiX v4.x, I think we just remove Authenticode signature verification (since it still has holes like @heaths points out) but we will discuss that then.

@robmen

robmen May 28, 2014

Member

@rseanhall exactly. This is the largest change we thought we could take in WiX v3.x. In WiX v4.x, I think we just remove Authenticode signature verification (since it still has holes like @heaths points out) but we will discuss that then.

This comment has been minimized.

@heaths

heaths May 29, 2014

Contributor

Okay, if we're talking about the larger change for v4 I'm not too concerned for 3x of always doing the hash (and perhaps never doing Authenticode, though I still argue the revocation ability for bundles already "out there" is big enough to consider - but that's a conversation for another time).

@heaths

heaths May 29, 2014

Contributor

Okay, if we're talking about the larger change for v4 I'm not too concerned for 3x of always doing the hash (and perhaps never doing Authenticode, though I still argue the revocation ability for bundles already "out there" is big enough to consider - but that's a conversation for another time).

This comment has been minimized.

@barnson

barnson Jun 2, 2014

Member

@robmen : What's verified if SuppressSignatureVerification="no" and there's no Authenticode signature? Does it fall back to the hash?

@barnson

barnson Jun 2, 2014

Member

@robmen : What's verified if SuppressSignatureVerification="no" and there's no Authenticode signature? Does it fall back to the hash?

This comment has been minimized.

@robmen

robmen Jun 3, 2014

Member

Yes. If Authenticode signature is suppressed (new default) or absent, hash is used.

@robmen

robmen Jun 3, 2014

Member

Yes. If Authenticode signature is suppressed (new default) or absent, hash is used.

@barnson barnson merged commit ef67bd0 into wixtoolset:develop Jun 3, 2014

@robmen robmen deleted the robmen:bug4243 branch Jun 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment