-----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.
