Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions draft.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ An [AppImage] which conforms to the type 2 image format:
* **MUST** work when spaces are used in its own filesystem path, in its own file name and in paths and filenames it uses internally
* **MAY** embed [update information] in the ELF section `.upd_info`. If the information in this location is not in one of the known [update information] formats, then it **SHOULD** be empty and/or be ignored
* **MAY** embed a digital signature in the ELF section `.sha256_sig`. If this section exists then it **MUST** either be empty (filled with `0x00` padding) or contain a valid digital signature of the sha256 of the AppImage assuming the ELF section `.sha256_sig` being filled with `0x00` padding ([why?](https://github.com/probonopd/AppImageKit/issues/238#issuecomment-249412813))
* **MAY** embed a digital signature in the ELF section `.sha256_sig`. If this section exists then it **MUST** **EITHER** be empty (filled with `0x00` padding) **OR** start with an OpenGPG ASCII Armored block as defined per [RFC 4880](https://tools.ietf.org/html/rfc4880) containing a detached signature. Trailing space SHOULD be filled with `0x00` padding. Keep special attention to the following RFC note:

> Note that all these Armor Header Lines are to consist of a complete
line. That is to say, there is always a line ending preceding the
starting five dashes, and following the ending five dashes. The
header lines, therefore, MUST start at the beginning of a line, and
MUST NOT have text other than whitespace following them on the same
line.

* This means that a valid digital signature **MUST** begin with a newline character, and end with a newline character, and **SHOULD** be followed by as many `0x00` bytes as required in order to fill the ELF section.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to also describe the .sig_key section, which is currently the only way how to deliver pubkeys along with the signatures to allow for validating at all.

Copy link
Member

@probonopd probonopd Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires discussion. .sig_key is not part of the spec and has never been formally proposed or discussed.

What good is it to deliver the public key along with the payload? Doesn't this circumvent the whole concept? Do you know any example where a public key is delivered in the same file as the payload to be verified?

The way I think signing should work:

  • With the default AppImageLauncher/appimaged/AppImageUpdate/whatever installation, the public key for the official AppImage tools is delivered
  • AppImageUpdate verifies that the signature of the new version matches the signature of the old version (if both are signed). In other words, AppImageUpdate makes sure that by updating you cannot suddenly get an AppImage that has a different signature.
    • If the old AppImage had no signature, then the status is "grey"
    • If the old AppImage had no signature but the new AppImage has a signature, then the state is "grey"
    • If the old AppImage had a signature and the new AppImage has a signature and they do not match, then the state is "red"
    • If the old AppImage had a signature and the new AppImage has a signature and they match, then the state is "yellow"
  • If you as the user want even more security, you need to manually get, verify, and import the public key for that AppImage into you keyring.
    • If the old AppImage had a signature and the new AppImage has a signature and they match and the public key is found in the user's keyring, then the state is "green"

Can you follow my logic? "Grey" means "I don't know". "Red" means "something bad is going on, someone tried to tamper with something". "Yellow" means "basic checks passed, but no full security". "Green" means "full security". Getting "Green" may require the user to do something, or otherwise we cannot really guarantee this security level.

Or can we?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About "any example including the public key along the payload", you can see that with email, for example. In particular, etiquette for key-signing includes each party attaching their public key and signing the multipart data. Protonmail even has UI to include the public key when sending emails, which I would believe is on by default.

In any case, since public key distribution is its own can of worms (and public distribution has issues like the SKS DoS attacks), if we allow including the public key along the payload we solve the distribution problem entirely by leveraging whichever distribution channel the AppImage used to get into the user. Besides, the fact that it is named public key does not mean that you want to distribute the key content, maybe the AppImage content is a trade secret and will never see the light of day but still want to check it's packaged by a trusted source.

Whether you include the public key or not, the security level of the checks can be the same you propose. The statuses can be the same, i.e., we can still check the old version of the AppImage (and the old public key and see if they match) but by including it on the AppImage we remove the requirement to download signatures off-channel, and we allow operation in air-gapped systems. Adding the public key is public information anyway, and therefore cannot be detrimental to security, but it can be convenient.

But what's more, by including the public key, we could even think of adding signatures among keys, or backup keys, so that future versions could be signed by another key but only if there is a signature from the parent key to the next. Anyway, I would move that discussion further down the road, and just stick with single keys for now, since without PKI and OCSP, checking key validity is not straightforward.

About the status:

I would insist on disagreeing with this state rule: when old Appimage has no signature and new AppImage has one, the state should be at least "orange", not "grey".

You could hijack a non-signed AppImage by injecting once a signed one. In this sense, we should at least warn the user that the update will be signed and that henceforth signed-appimage rules will be applied on updates.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheAssassin I had not included it because although I believe what I just said, the discussion as to whether to include the field was happening on another issue, and I would have wanted to see this merged soon (although in retrospective it hasn't happened so soon 😄 )

Copy link
Member

@probonopd probonopd Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should I trust a key that comes inside an AppImage more than the rest of the AppImage?

In that case I would at least need to manually verify the key fingerprint before I could trust the key. Would that bring any advantage over importing the key into the keyring?

Copy link
Member

@TheAssassin TheAssassin Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does not shipping the key solve any issue?

Why should I trust a key that comes inside an AppImage more than the rest of the AppImage?

Well, it's as simple as "you don't". AppImageUpdate doesn't trust these keys automatically. It builds "transitive trust", by enforcing that the key in the old AppImage must be the same with which the new AppImage has been signed. But you first need the key at all.
This concept is not new; in fact, many software projects (e.g., Conversations) have implemented similar mechanisms.
It's been thoroughly discussed back in the days, and I'm pretty sure you were part of it. It just never made its way into the spec.

Arguments like "it's insecure" or anything are just pure nonsense, and shouldn't prevent its inclusion. It's not insecure at all. You just have a wrong imagination of what guarantees the signature can provide. The plain truth is, these signatures don't add any kind of security outside AppImageUpdate, which is the only tool that can make any use of them.

It has never been said that this key is to be trusted automatically. But not shipping doesn't increase security or anything. You are free to verify a key's fingerprint externally.

By the way, you realize that this validate tool has never provided any functionality with regards to security, don't you? Its mere purpose is to check that signing was successfully done in the first place. That's more an integrity check than anything security related.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It builds "transitive trust", by enforcing that the key in the old AppImage must be the same with which the new AppImage has been signed. But you first need the key at all.

This completely makes sense to me. But why do we need to have the key to check that two AppImages were signed by the same key?

Copy link
Member

@TheAssassin TheAssassin Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the key just for that (but also). You need the public key to verify the signature at all. Chicken egg problem: how to verify something if you don't have the key? Without shipping the key, verification can't take place at all. Once you can verify, you can then start to build transitive trust.

Copy link
Member

@probonopd probonopd Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

I guess we should keep a short version of the explainations near the spec (under a "Why?" link).

* The signed plaintext **MUST** be the ASCII string that results from converting into hexadecimal notation the binary bytes of the SHA-256 hash of the ELF file when substituting the `.sha256_sig` and `.upd_info` ELF sections with `0x00` padding.

* **MUST** contain the magic hex `0x414902` at offset 8 ([why?](https://github.com/probonopd/AppImageKit/issues/144))

### Contents of the image
Expand Down