-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On Sat, Jul 25, 2020 at 10:46:40AM +0000, WillyPillow wrote: > On Saturday, July 25, 2020 12:18 AM, Wojtek Porczyk > <w...@invisiblethingslab.com> wrote: > > On Thu, Jul 23, 2020 at 05:45:56PM +0000, WillyPillow wrote: > > > > > > One issue is that from the qrexec client side it is basically impossible > > > to > > > distinguish between the two. (Consider the case where a field contains > > > `xxx\\na:b:c`.) > > > > > If there are more colons that there are supposed to be, there is no need to > > distinguish anything anymore, just error out for "malformed input" or > > something. > > > > > In Python I like to do it with tuple assingment: > > > > > try: > > field1, field2, field3 = untrusted_line.split(':') > > except TypeError: > > raise ParseError('error message') > > By "distinguish between the two" I meant correct input and malformed input. > Sorry for the confusion. > > The point I was trying to make in that example was that when there are colons > *and* newlines, there could be cases like the following: > > ``` > field1 = 'a' > field2 = 'b' > field3 = 'c\nd:e:f' > encoded = ':'.join([field1, field2, field3]) # a single entry > encoded_list = '\n'.join([encoded] * 10) # simulate multiple entries > # on the other side... > decoded_list = encoded_list.split('\n') # becomes 20 rows > for untrusted_line in decoded_list: > field1, field2, field3 = untrusted_line.split(':') # this does not error > out > ```
- - Yes, this breaks something and is not distinguishable from template manager. - - No, it's not malformed, because it conforms to specification. - - It's OK from security perspective: You cannot cause installation of anything the user didn't explicitly mean (by choosing an item on a list and/or accepting a signing key). To reiterate: in optimal case this should be avoided from the sending side, but it't not a very high priority. It would be also OK if the tool and specification did something like "only the first line with ':' skipped". The worst case is something can be shown on the list of available templates that actually cannot be installed. > > It's as simple as that. The big advantage is that there aren't many ways to > > do > > something wrong. > > > > > > Security-wise, this is unlikely to cause issues as an entity that can do > > > this > > > can probably modify the repo contents directly. > > > > > The point is, we don't know. The repo content is untrusted, and yes, > > attacker > > can modify it. What counts is signature on RPM. > > > > > > However, if the repo, by accident, does contain packages with, say, > > > colons in > > > summaries, it may be an issue usability-wise as it's hard to give > > > meaningful > > > error messages when things break. > > > > > "Malformed input" is OK. If we break loudly, template maintainers (the > > honest > > among them) won't publish such summary, because it will break. > > The issue is that it's not always possible to detect such errors, as > elaborated > above. Of course, as long as we keep this "wart" explicit, I'm okay with > having > this as an "undefined behavior". (After all, the repo listing *is* considered > untrusted anyway.) Sure. > > > There's also the original issue with descriptions (assuming that we don't > > > omit > > > them), which contains newlines a lot of the time. > > > That being said, if we treat such errors as "repo errors" and leave to > > > the repo > > > maintainers to ensure that the fields follow a certain format, then we > > > can just > > > use a special character for the separator [5] and ban the character from > > > the > > > fields. > > > > > Yes, and IIUC the current proposal is to have ':' as that special character. > > Am I missing something? > > The reason I'm bringing this up is twofold: > > 1. The idea of having the repo maintainer ensure such constraints have not > been > explicitly brought up before AFAIK. > 2. I'm unsure whether `:` is the best choice for this, as it's not an uncommon > character, and banning it might be undesirable. Something like ASCII 28-31 > are possible alternatives. I think this should be one of the printable characters, so the output is easy to read and audit. Though ':' might well not be the best character, but there are others like '|' or even '!' (and I'd argue banning '!' from descriptions would do good to avoid shouting at people). > > > [5]: The separator may also need to be placed at the end of the format > > > string. > > > > > I don't think so. > > If there is no such character at EOLs, then `\n` becomes another "special > character", as stuff would break when doing the `.split('\n')` step. If, on > the > other hand, there is such a character, we can be sure that every entry > consists > of exactly k (some positive number) <separator>-seperated entries. Having separator at the end gains nothing. > Thanks, > WillyPillow Thank you for this week! Tomorrow Marmarek returns, so he'll carry on. - -- pozdrawiam / best regards Wojtek Porczyk Graphene / Invisible Things Lab I do not fear computers, I fear lack of them. -- Isaac Asimov -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEaO0VFfpr0tEF6hYkv2vZMhA6I1EFAl8d8a8ACgkQv2vZMhA6 I1FZ7hAAjBwaVy2uZ9t4dxo9CfHya8VcMagCQoTkwrUpv5GgRzStCiC0SJNQVOgS V2cS7waqb2pC8HSe5ku055MpGLJwA96MnSqZii1zhtqRqdo9MxEUc3oSK1WFI+MC 0XoGTEGbmlGjwdRoqeM464KOOk7gzNoqloZH0VLCyRDaWj97UR4I1HKh5FWV5alD WXUeqHhdZnns7LeX8LSn8WIUWXfdSgMsB9APVNrsXlnPVddlKsSoGUNjEKEb5viS MxRl6RHS5kCGZa4yy+09j5gxYeha/lCrrMpAK3/qhRx4bzAhc2nmbv1qf8xVW+G+ Mj0Sf5BgLDmCE9wxvbAQm2PIVaduF9G8Y5lwqYX7TTV4QC2zW71RJGJu7zlNYkpE SkvNCH06zhY0zWsxZ/91KOafsMjajbaYfX0VrbXa3vVfanLy+iRJhZ3gIt3HLOOv 4hLZV0k/sy+ooqLXBwN9cE1CyYXVKVSGw403NV9yumFrUMhFuzehyPAdxOI6nCj6 2OV2luEKYrMSZWfaPHSk5b+JKplp39oaX6aHs0TBW2ahQmOhmrg4mnDchc9XwzOr M4MuZkX+/qtgfKH6/RZeS6OKmKCmQEED7ibfPRgWf9pMgB2ERfDA1zrU4lJx2fSQ FsFwjjByrE1+W247oniKoDpv61v6tU86yUjVS1klyxofc0/W2co= =mHmD -----END PGP SIGNATURE----- -- You received this message because you are subscribed to the Google Groups "qubes-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to qubes-devel+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/qubes-devel/20200726211214.GF2122%40invisiblethingslab.com.