Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-12-23 Thread Bernhard R. Link
* Ximin Luo  [191223 12:58]:
> dpkg and all other debian tools support it right now. It is only reprepro 
> with this artifical constraint, which makes it not work for packages that are 
> processable by dpkg and other debian tools.

If it is artifical, then it is artifically high. It is 128 times more than
what almost every single package needs and more than five times what the most
absurd package before needed and twice what other tools are said to have
had as limit there. I will increase it in reprepro (and maybe might make it
configurable to some extent), but there will always be an upper limit.

> Are you suggesting that dpkg and other tools have a concrete security problem?

dpkg does not check checksums of index files, so it is likely
uneffected. If apt has no limit then that likely makes some attacks
needlessly easy (though it might have other mitigations in that regard,
and there are less things apt has to care about the way it is typically
used).
Accepting absurd input without confirmation is never a secure way to handle
things, though.

Bernhard R. Link



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-12-23 Thread Ximin Luo
Bernhard R. Link:
> [..]
> 
> As the comment describes, accepting arbitrary long control data would
> open all kind of security issues and require quite some hard to properly
> test code. Most of the attacks enabled by having longer control chunks
> might be able to mitigated some way, but that would require all kind of
> different logic that can then have some new bugs.
> 

I don't see why this is the case, but nevertheless this is actually secondary 
to the below point:

> So allowing arbitrary absurdly long control data is not something I want
> to support.
> 

dpkg and all other debian tools support it right now. It is only reprepro with 
this artifical constraint, which makes it not work for packages that are 
processable by dpkg and other debian tools.

Are you suggesting that dpkg and other tools have a concrete security problem?

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-12-23 Thread Bernhard R. Link
* Ximin Luo  [191223 09:16]:
> A long-term fix would be to fix this:
>
> line 151-166:

>   if (f->size - f->ofs <= 2048) {
>   /* Adding code to enlarge the buffer in this case
>* is risky as hard to test properly.
>*
>* Also it is almost certainly caused by some
>* mis-representation of the file or perhaps
>* some attack. Requesting all existing memory in
>* those cases does not sound very useful. */
>
>   fprintf(stderr,
> "Error parsing %s line %d: Ridiculous long (>= 256K) control chunk!\n",
>   f->filename,
>   f->startlinenumber);
>   f->failed = true;
>   return RET_ERROR;
>   }
>
> One reasonable option would be to rip out this code and use whatever dpkg 
> itself is using to parse the fields.

As the comment describes, accepting arbitrary long control data would
open all kind of security issues and require quite some hard to properly
test code. Most of the attacks enabled by having longer control chunks
might be able to mitigated some way, but that would require all kind of
different logic that can then have some new bugs.

So allowing arbitrary absurdly long control data is not something I want
to support.

Bernhard R. Link



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-12-22 Thread Ximin Luo
Control: reassign -1 reprepro 5.3.0-1
Control: retitle -1 reprepro imposes arbitrary limits on control files that are 
successfully parsed by other debian tools

Ximin Luo:
> [..]
> I'll take a look at reprepro in the next 2-3 weeks; arbitrary limits like 
> 256K should be pretty easy to fix (have you tried simply configuring the BDB 
> limits?).

The relevant code in reprepro is indexfile.c

line 66:f->size = 256*1024;

Change this to something like 4MB would be a short hacky fix to the current 
issue, I don't think even the extreme rust examples have a 4MB control field 
yet.

A long-term fix would be to fix this:

line 151-166:
if (f->size - f->ofs <= 2048) {
/* Adding code to enlarge the buffer in this case
 * is risky as hard to test properly.
 *
 * Also it is almost certainly caused by some
 * mis-representation of the file or perhaps
 * some attack. Requesting all existing memory in
 * those cases does not sound very useful. */

fprintf(stderr,
"Error parsing %s line %d: Ridiculous long (>= 256K) control chunk!\n",
f->filename,
f->startlinenumber);
f->failed = true;
return RET_ERROR;
}

One reasonable option would be to rip out this code and use whatever dpkg 
itself is using to parse the fields.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Raphael Hertzog
Hello Ximin,

On Thu, 17 Oct 2019, Ximin Luo wrote:
> >> Do you have some concrete suggestions on how to improve the tool to reduce 
> >> this "abuse"?
> > 
> > Yes, I gave you one.
> 
> It doesn't work.

Look, I'm not a cargo/rust expert, I won't design the tool for you but I
implemented dpkg-gensymbols and the symbols support for dpkg-shlibdeps
and I'm pretty confident that such a solution can work for your case too.

We are not adding a provides to libc6 for each symbol that the library is
exporting. And you should not add a provides for each "interface" (or
whatever it's called in rust) that a package is providing.

You should export the list of interfaces in a separate metadata file thas
is not part of the generated "Packages" file and you should have a tool to
generate the binary dependencies pointing back to the correct package that
is exporting the interface.

It might not be as flexible as the current approach as it might require
rebuilds when the package providing the interface changes, but that's
quite usual in Debian.

> > You are being arrogant. Replying in the same tone, I would say that the
> > design of your tool suck.
> 
> That's cool, and it really doesn't persuade me to have any sympathy
> towards your issue. Note that the next time this package is
> automatically regenerated, your "fixes" will be undone.

Note that if you re-introduce the issue I will ask ftpmasters to
remove the package and/or ask the tech-ctte to decide about it.

(I can play that game too... but it's not helping)

You can't just ignore problems when you are breaking the infrastructure of
derivative distributions and users... right now the problem is limited to
unstable and I'm the first to have discovered it but I'm pretty confident
that others will hit it as well.

And as I said, those servers are not running unstable so if you really
want to go down the route of fixing reprepro (while ignoring the fact
that Ansgar, who is a ftpmaster, is asking you to not continue with such a
Provides header), you will have to get fixes pushed to stable...

> Please be a bit more self-critical about your own opinion. Have you
> considered the possibility that it is the reading tool (reprepro in this
> case) that "sucks" and not the writing tool?

Yes, reprepro sucks in some ways. But a design that puts 270K of metadata
in a single Provides line sucks too.

But reprepro is in wide use and your new package is the first one to
trigger the limit. You can't just ignore the reality, you have to cope
with the fact that we have reprepro users and that we can't deploy
a package with 270K-long Provides header currently.

(And IMO we should never allow this but that's another discussion)

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Ansgar
Ximin Luo writes:
> Raphael Hertzog:
>> Don't abuse the "Provides" field when you have such a volume of
>> interfaces to document.
>
> Can you please explain why 256 KB provides field is "abuse"?

The Packages index is a shared resource by all packages and every Debian
user has to download and process the full packages index; adding
excessive amounts of data should therefore be avoided.  (The 256 KB
added to the Packages index are larger than the entire source
(compressed) source package...)

> Do you have some concrete suggestions on how to improve the tool to
> reduce this "abuse"?

Don't generate virtual packages (Provides) for every feature; don't
generate four virtual packages for every feature.

Ansgar



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Ximin Luo
Raphael Hertzog:
> On Thu, 17 Oct 2019, Ximin Luo wrote:
>> Can you please explain why 256 KB provides field is "abuse"?
> 
> Because that's the amount of metadata required for 250 common packages.
> 

So? There are some Debian packages that have much more than 250 times the data 
of common packages. No tools break with that, nor are there suggestions that 
those big packages "suck".

>> Do you have some concrete suggestions on how to improve the tool to reduce 
>> this "abuse"?
> 
> Yes, I gave you one.
> 

It doesn't work.

>> BTW, the tool is run not at build time but to generate the source
>> package. So it can't use these "foo.cargo" files, because you don't need
>> to install all of the dependencies in order to use the tool.
> 
> If you run a tool to generate the source package, you can include
> whatever call you want during your source package build. i.e. you
> control debian/rules too. And you can process the source package
> and/or the binary package built to create those meta-information
> and also to use the existing meta-information on the system.
> 

This isn't a concrete suggestion, it's a generic vacuous statement about how 
package builds work, and is true for what already happens. The existing tool 
already "processes the source package [..] to create those meta-information", 
namely Provides fields corresponding to what's needed according to what's 
defined by upstream.

>> It is 2019. If a tool can't handle 256 KB of data, I'd say the tool is
>> at fault and not the 256 KB of data.
> 
> You are being arrogant. Replying in the same tone, I would say that the
> design of your tool suck.
> 

That's cool, and it really doesn't persuade me to have any sympathy towards 
your issue. Note that the next time this package is automatically regenerated, 
your "fixes" will be undone.

Please be a bit more self-critical about your own opinion. Have you considered 
the possibility that it is the reading tool (reprepro in this case) that 
"sucks" and not the writing tool?

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Raphael Hertzog
On Thu, 17 Oct 2019, Ximin Luo wrote:
> Can you please explain why 256 KB provides field is "abuse"?

Because that's the amount of metadata required for 250 common packages.

> Do you have some concrete suggestions on how to improve the tool to reduce 
> this "abuse"?

Yes, I gave you one.

> BTW, the tool is run not at build time but to generate the source
> package. So it can't use these "foo.cargo" files, because you don't need
> to install all of the dependencies in order to use the tool.

If you run a tool to generate the source package, you can include
whatever call you want during your source package build. i.e. you
control debian/rules too. And you can process the source package
and/or the binary package built to create those meta-information
and also to use the existing meta-information on the system.

> It is 2019. If a tool can't handle 256 KB of data, I'd say the tool is
> at fault and not the 256 KB of data.

You are being arrogant. Replying in the same tone, I would say that the
design of your tool suck.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Ximin Luo
Ximin Luo:
> Raphael Hertzog:
>> On Thu, 17 Oct 2019, Ximin Luo wrote:
>>> Control: tags -1 + wontfix
>>
>> This is clearly not acceptable. You can't ignore problems like this one.
>> I saw you already broke debian-installer once with the former packages
>> that overflowed the 16K limit of cdebootstrap. Now it's the turn of
>> reprepro and this one is harder to fix because there are real servers
>> running stable version of reprepro, etc.
>>
>>> The tool's algorithm was suggested by the maintainer of dpkg and has his
>>> blessing. It is partly due to limitations in dpkg, see #901827 for
>>> details.
>>
>> The algorithm is one thing... but the design of your tool is another
>> thing.
>>
>> dpkg has dpkg-shlibdeps to build dependencies based on exported
>> information by various package (through
>> /var/lib/dpkg/info/*.{shlibs,symbols}).
>>
>> cargo should build the same infrastructure, i.e. have a
>> /var/lib/dpkg/info/foo.cargo used by dh-cargo to build the correct
>> dependency.
>>
>> Don't abuse the "Provides" field when you have such a volume of
>> interfaces to document.
>>
> 
> Can you please explain why 256 KB provides field is "abuse"?
> 
> Do you have some concrete suggestions on how to improve the tool to reduce 
> this "abuse"?
> 

BTW, the tool is run not at build time but to generate the source package. So 
it can't use these "foo.cargo" files, because you don't need to install all of 
the dependencies in order to use the tool.

So yes, we need a concrete suggestion on improving the tool, rather than wild 
hyperbole that its output is "abuse".

It is 2019. If a tool can't handle 256 KB of data, I'd say the tool is at fault 
and not the 256 KB of data.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git



Bug#942487: [Pkg-rust-maintainers] Bug#942487: Bug#942487: rust-web-sys: Provides header is more than 256K long and it breaks reprepro...

2019-10-17 Thread Ximin Luo
Raphael Hertzog:
> On Thu, 17 Oct 2019, Ximin Luo wrote:
>> Control: tags -1 + wontfix
> 
> This is clearly not acceptable. You can't ignore problems like this one.
> I saw you already broke debian-installer once with the former packages
> that overflowed the 16K limit of cdebootstrap. Now it's the turn of
> reprepro and this one is harder to fix because there are real servers
> running stable version of reprepro, etc.
> 
>> The tool's algorithm was suggested by the maintainer of dpkg and has his
>> blessing. It is partly due to limitations in dpkg, see #901827 for
>> details.
> 
> The algorithm is one thing... but the design of your tool is another
> thing.
> 
> dpkg has dpkg-shlibdeps to build dependencies based on exported
> information by various package (through
> /var/lib/dpkg/info/*.{shlibs,symbols}).
> 
> cargo should build the same infrastructure, i.e. have a
> /var/lib/dpkg/info/foo.cargo used by dh-cargo to build the correct
> dependency.
> 
> Don't abuse the "Provides" field when you have such a volume of
> interfaces to document.
> 

Can you please explain why 256 KB provides field is "abuse"?

Do you have some concrete suggestions on how to improve the tool to reduce this 
"abuse"?

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git