Re: [RFC] Enhance checksum support
On Fri, Jan 18, 2008 at 11:38:55PM +1000, Anthony Towns wrote: > It'd actually be good to be able to break Files in future, so that we're > forced to verify something other than md5sum. Otherwise there will > be code that doesn't check it properly, and that will end up being a > security problem. Hmm, that might indeed be a good idea (the point to remove the Files field would be v3 then). > Having it be: > > Contents: sha256 >28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 355 foo > Files: >4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo > > or similar would be nice and non-redundant, and make it possible to drop I can see the "nice". But once I want to include more than one checksum it quickly gets redundant. So maybe keep the Checksums field and introduce a Contents field that contains no checksums, but only the size and the name? Checksums: md5 4bf7ff17bd9ddf3846d9065b3c594fb4 foo sha256 28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 foo Contents: 355 foo Files: 4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo That makes the parsing more robust and eliminates the need to specifiy the size of a file more than once. If we want we could even declare size also to be a checksum and include only the filenames in the Contents field. Gruesse, -- Frank Lichtenheld <[EMAIL PROTECTED]> www: http://www.djpig.de/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: [RFC] Enhance checksum support
Frank Lichtenheld wrote:
On Fri, Jan 18, 2008 at 11:38:55PM +1000, Anthony Towns wrote:
It'd actually be good to be able to break Files in future, so that we're
forced to verify something other than md5sum. Otherwise there will
be code that doesn't check it properly, and that will end up being a
security problem.
Hmm, that might indeed be a good idea (the point to remove the Files
field would be v3 then).
Having it be:
Contents: sha256
28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 355 foo
Files:
4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo
or similar would be nice and non-redundant, and make it possible to drop
I can see the "nice". But once I want to include more than one checksum
it quickly gets redundant.
So maybe keep the Checksums field and introduce a Contents field that
contains no checksums, but only the size and the name?
Checksums:
md5 4bf7ff17bd9ddf3846d9065b3c594fb4 foo
sha256 28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 foo
Contents:
355 foo
Files:
4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo
That makes the parsing more robust and eliminates the need to specifiy
the size of a file more than once. If we want we could even declare size
also to be a checksum and include only the filenames in the
Contents field.
Gruesse,
Isn't sha256 a little much for a file of this size? Would it be worth
using a smaller hash for smaller files? With both lines you are storing
122 bytes to uniquely identify a 355 byte file named foo. If you really
need multiple checksums, why not do something more of the type:
Checksums: sha1 sha256 sha_N
- {sha256} - foo
{sha1} - - bar
Files:
{md5} 355 foo
{md5} 10 bar
{md5} 1 baz
You wast less space identifying the hash and it is still easy to parse.
I assume the Files section can not break and requires the "md5 size
name" format for older/unsupported tools.
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Please change dselect override priority to optional
Hi, as the subject says, please change dselect overrides' priority to optional. Thanks. -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: [RFC] Enhance checksum support
On Sat, Jan 19, 2008 at 12:20:57PM -0700, Tim Spriggs wrote:
> Isn't sha256 a little much for a file of this size? Would it be worth
> using a smaller hash for smaller files? With both lines you are storing
> 122 bytes to uniquely identify a 355 byte file named foo. If you really
> need multiple checksums, why not do something more of the type:
>
> Checksums: sha1 sha256 sha_N
> - {sha256} - foo
> {sha1} - - bar
> Files:
> {md5} 355 foo
> {md5} 10 bar
> {md5} 1 baz
>
> You wast less space identifying the hash and it is still easy to parse.
I don't think the space is really a issue for anything. Most source
files are actually much bigger than the example sizes we used here...
The current question is whether we want two separate Checksums/Files(*)
fields (which was Raphael's proposal implemented by my second patch)
or if the checksums information should remain in one field with the
file list (which was aj's proposal).
> I assume the Files section can not break and requires the "md5 size
> name" format for older/unsupported tools.
yes. If we want to change the format of the Files: field it might indeed
be better and cleaner to rename it on the way as aj suggested.
Gruesse,
--
Frank Lichtenheld <[EMAIL PROTECTED]>
www: http://www.djpig.de/
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: [RFC] Enhance checksum support
On Sat, 19 Jan 2008, Frank Lichtenheld wrote: > On Fri, Jan 18, 2008 at 11:38:55PM +1000, Anthony Towns wrote: > > It'd actually be good to be able to break Files in future, so that we're > > forced to verify something other than md5sum. Otherwise there will > > be code that doesn't check it properly, and that will end up being a > > security problem. > > Hmm, that might indeed be a good idea (the point to remove the Files > field would be v3 then). Note it also affects *.changes files. > So maybe keep the Checksums field and introduce a Contents field that > contains no checksums, but only the size and the name? > > Checksums: > md5 4bf7ff17bd9ddf3846d9065b3c594fb4 foo > sha256 28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 foo > Contents: > 355 foo > Files: > 4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo > > That makes the parsing more robust and eliminates the need to specifiy > the size of a file more than once. Looks good to me. It means we have quite some duplication until we can drop Files but it's not a big deal IMO. > If we want we could even declare size also to be a checksum and include > only the filenames in the Contents field. This makes sense from the logical point of view as the size is certainly not useful except to verify that the files are the same... on the other hand it would need some special casing as it wouldn't be calculated by an external program. So a slight preference to keep it in Contents for me but I would'nt mind if you decided the other way. Cheers, -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: [RFC] Enhance checksum support
On Sat, Jan 19, 2008 at 08:46:15PM +0100, Raphael Hertzog wrote: > On Sat, 19 Jan 2008, Frank Lichtenheld wrote: > > Hmm, that might indeed be a good idea (the point to remove the Files > > field would be v3 then). > > Note it also affects *.changes files. Yeah, but the decision for both when to break backwards compatibility is independent from each other. [...] > > If we want we could even declare size also to be a checksum and include > > only the filenames in the Contents field. > > This makes sense from the logical point of view as the size is certainly > not useful except to verify that the files are the same... on the other > hand it would need some special casing as it wouldn't be calculated by an > external program. So a slight preference to keep it in Contents for me but > I would'nt mind if you decided the other way. Note that in my patch the size is already handled inside Dpkg::Checksums. But I disagree that its only use is as a checksum. e.g. it is also useful for calculating download sizes for the whole source package. So it is slightly more versatile than other information. Gruesse, -- Frank Lichtenheld <[EMAIL PROTECTED]> www: http://www.djpig.de/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Do you mind receiving BTS mails of dpkg
On Wed, 2008-01-09 at 20:43:38 +0100, Raphael Hertzog wrote: > On Wed, 09 Jan 2008, Christian Perrier wrote: > > Quoting Raphael Hertzog ([EMAIL PROTECTED]): > > > > > Is anyone reading [EMAIL PROTECTED] here that would be bothered if they > > > received > > > the BTS mails related to dpkg ? > > > > Supported. I would just do as you do..:) > > I also discussed with listmasters and if we want to keep BTS mails > separate, they should be able to add a procmail rule to drop the BTS mails > on this list (so that we can still change the Maintainer field). > > Alternatively, if we start receiving BTS mails on this list, they can also > add a rule to not archive BTS mails for this list. As I said my first preference would be to not receive BTS mails here, but given that other people either don't mind or want them here, I would be fine with just not archiving them, as I can filter them locally. If listamsters do the change, I can do the switch for next upload. regards, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Next upload 2008-01-20 (dpkg 1.14.16)
On Fri, 2008-01-18 at 18:04:13 +0100, Christian Perrier wrote: > Quoting Guillem Jover ([EMAIL PROTECTED]): > > So let's target next release for this sunday (latish). dpkg should > > migrate to testing today or so and there has not been major > > regressions. > Could you re-update the po/ directory (unless you're sure that no > string changes happened since last update)? Frank just did this. regards, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Next upload 2008-01-20 (dpkg 1.14.16)
On Fri, 2008-01-18 at 14:23:03 +0100, Frank Lichtenheld wrote: > On Fri, Jan 18, 2008 at 10:21:14AM +0200, Guillem Jover wrote: > > So let's target next release for this sunday (latish). dpkg should > > migrate to testing today or so and there has not been major > > regressions. > > > > I've pending for commit, the feature-removal-schedule and api docs, > > probably merging few more patches from the BTS, and I'll review > > Tollef's filter code, if I feel confortable and doesn't seem too > > disruptive I'll merge it as well, otherwise it will have to wait. > > What about the proposal to change the maintainer address to this mailing > list. I've not heard any objections to that. I knew I was forgetting something, replied to that thread. regards, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Please change dselect override priority to optional
On 11269 March 1977, Raphael Hertzog wrote: > as the subject says, please change dselect overrides' priority to optional. Done. -- bye Joerg Contrary to common belief, Arch:i386 is *not* the same as Arch: any. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Next upload 2008-01-20 (dpkg 1.14.16)
On Sun, Jan 20, 2008 at 12:04:26AM +0200, Guillem Jover wrote: > On Fri, 2008-01-18 at 18:04:13 +0100, Christian Perrier wrote: > > Quoting Guillem Jover ([EMAIL PROTECTED]): > > > So let's target next release for this sunday (latish). dpkg should > > > migrate to testing today or so and there has not been major > > > regressions. > > > Could you re-update the po/ directory (unless you're sure that no > > string changes happened since last update)? > > Frank just did this. Only the scripts/po. The po/ had no changes I think, but I didn't test man/po Gruesse, -- Frank Lichtenheld <[EMAIL PROTECTED]> www: http://www.djpig.de/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: Review of file exclusion branch requested
On Sat, Jan 05, 2008 at 11:18:13AM +0100, Tollef Fog Heen wrote: > I've finally gotten around to fixing up my support for excluding bits > of packages as they are unpacked. It can be gotten from > git://git.err.no/dpkg in the master branch (sorry about that, it > should probably have gone in a separate branch). > > It's still missing in the documentation department, but it seems to > work fine for me in my somewhat light testing. Just some nitpicks: void loadfilter(char *fn) should probably be void loadfilter(const char *fn) At one point you try to return NULL from void loadfilters(). That should probably be just "return;" "gobble replaced file `%.255s'" should probably have "replaced" replaced with something more fitting. (IMHO this message should not even be translated, I don't think anybody except some very few dpkg developers can associate anything with it). And yeah I know you just copied that, no excuse to repeat errors of the past ;) Gruesse, -- Frank Lichtenheld <[EMAIL PROTECTED]> www: http://www.djpig.de/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Re: [RFC] Enhance checksum support
On Sat, Jan 19, 2008 at 06:15:45PM +0100, Frank Lichtenheld wrote:
> > Having it be:
> > Contents: sha256
> >28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 355 foo
> > Files:
> >4bf7ff17bd9ddf3846d9065b3c594fb4 355 foo
> > or similar would be nice and non-redundant, and make it possible to drop
> I can see the "nice". But once I want to include more than one checksum
> it quickly gets redundant.
Well, it's "redundant" in the sense it repeats filename and size info; but
size is an integral part of the hashing (for at least some hashes it's much
easier to break them if you can have different sized files).
The advantage of having all that in one place means you can verify the
hash properly with a simple script like:
cat *.changes |
sed -ne '/^Checksums: sha256/,/^[^ ]/{/^ /p}' |
while read hash size file; do
if [ "$(wc -c $file)" -eq $size ] && \
[ "$(sha256sum < $file) | cut -d\ -f1)" = "$hash" ];
then
echo "$file is OK"
else
echo "$file is BAD"
fi
done
ie, you just need to find the right section of the .changes/.dsc file, but
at that point parsing is trivial. If you don't like sed linenoise, grep-dctrl
does the same job:
gpg < ${changes} 2>/dev/null |
grep-dctrl -s Checksums-sha256 "" | grep '^ ' |
while read hash size file; do ...
> So maybe keep the Checksums field and introduce a Contents field that
> contains no checksums, but only the size and the name?
> Checksums:
> md5 4bf7ff17bd9ddf3846d9065b3c594fb4 foo
> sha256 28ee6a10eb280ede4b19c1b975aff5533016a26de67ba9212d51ffaea020ce34 foo
Having the hash as a parameter instead of in the field is a bit confusing
but still easy to parse; having the size separated out makes things much
more awkward though.
By confusing, I mean things like:
Checksums:
sha1 a0a53d15d7dbc6a9cdfd1889ae30ba8b3dbf7d94 foo
md5 4bf7ff17bd9ddf3846d9065b3c594fb4 bar
become ambiguous -- is it an error, or are you meant to be using md5 to
verify bar and sha1 to verify foo, along the lines of Tim's suggestion?
It's also slightly harder to see what hashes are in use -- you can't just
check the presence of a field, you have to grep the contents of the field.
For comparison, the Release files use the format:
MD5Sum:
{md5} {size} {path}
SHA1:
{sha1} {size} {path}
SHA256:
{sha256} {size} {path}
Cheers,
aj
signature.asc
Description: Digital signature
Re: Review of file exclusion branch requested
Hi,
On Sat, 2008-01-05 at 11:18:13 +0100, Tollef Fog Heen wrote:
> I've finally gotten around to fixing up my support for excluding bits
> of packages as they are unpacked. It can be gotten from
> git://git.err.no/dpkg in the master branch (sorry about that, it
> should probably have gone in a separate branch).
Thanks for working on this. For next time please use git-format-patch,
it makes it easier to review, also it allows one to fix the patches
and resend.
> It's still missing in the documentation department, but it seems to
> work fine for me in my somewhat light testing.
It be really nice if you could write them before I merge this. So few
comments in addition to what Frank wrote:
diff --git a/lib/dpkg.h b/lib/dpkg.h
index ff3ac9a..f1e0c41 100644
--- a/lib/dpkg.h
+++ b/lib/dpkg.h
@@ -181,6 +181,7 @@ extern const char printforhelp[];
umask(022); /* Make sure all our status databases are readable. */\
if (loadcfg)\
loadcfgfile(prog, cmdinfos);\
+ loadfilters();\
myopt(argv,cmdinfos);\
} while (0)
This would get called on all programs, but only dpkg uses the filters,
just call it from dpkg main.
diff --git a/lib/myopt.c b/lib/myopt.c
index 5aed2f3..6defe60 100644
--- a/lib/myopt.c
+++ b/lib/myopt.c
@@ -164,3 +169,91 @@ void myopt(const char *const **argvp, const struct cmdinfo
*cmdinfos) {
}
}
}
+
+struct filterlist *filters = NULL;
+
+void loadfilter(char *fn) {
+ FILE* file;
Asterisk near the variable not the type.
+ char linebuf[1024];
+ struct filterlist *filtertail;
+
+ file = fopen(fn, "r");
+ if (!file) {
+warningf(_("failed to open filter file `%.255s' for reading"), fn);
+return;
+ }
This will give a warning on all systems that do not have that file,
either this should check for ENOENT or we ship an empty one, the
former being better I'd say.
+ while (fgets(linebuf, sizeof(linebuf), file)) {
+struct filterlist *filter;
+
+filter = malloc(sizeof(struct filterlist));
+if (!filter) {
+ ohshite(_("Error allocating memory for filter entry"));
+}
Use m_malloc instead.
+filter->filterstring = malloc(strlen(linebuf));
+if (!filter->filterstring) {
+ ohshite(_("Error allocating memory for filter entry"));
+}
+strcpy(filter->filterstring, &linebuf[1]);
You can use strdup here (will be replaced later on with m_strdup).
+if (! filters) {
+ filters = filter;
+ filtertail = filter;
+} else {
+ filtertail->next = filter;
+ filtertail = filtertail->next;
+}
You could do the same w/o special casing the first element by making
filtertail **. Also you'll have to move filtertail outside the
function, otherwise it will not support being called more than once.
+ }
+
+ if (ferror(file)) ohshite(_("read error in configuration file `%.255s'"),
fn);
+ if (fclose(file)) ohshite(_("error closing configuration file `%.255s'"),
fn);
Please wrap after the if conditional.
+
+}
Empty line.
+void loadfilters() {
Argument should be 'void'. And this function is missing a declaration
in the header.
+ struct dirent *dent;
+ char *dirname = CONFIGDIR "/filters.d";
+ DIR *dir = opendir(dirname);
+ if (!dir) {
+if (errno == ENOENT) {
+ return NULL;
+} else {
+ ohshite(_("Error opening filters.d"));
+}
+ }
In general, no need for braces when it's non-ambiguous and there's only
one statement line.
+ while ((dent = readdir(dir)) != NULL) {
+struct stat statbuf;
+char *file = malloc(strlen(dirname) + 1 + strlen(dent->d_name) + 1);
+if (!file)
+ ohshite(_("Error allocating memory for file"));
m_malloc.
+sprintf(file, "%s/%s", dirname, dent->d_name);
+if (stat(file, &statbuf) != 0) {
+ ohshite(_("Error stating file"));
+}
+if (S_ISREG(statbuf.st_mode)) {
+ loadfilter(file);
+}
+free(file);
+ }
+ closedir(dir);
+}
Only dpkg will be using this so I think it's better to split it into
src/filters.c so that when statically linking the code is not uselessly
duped on all other programs, and we put related code into the same place.
diff --git a/lib/myopt.h b/lib/myopt.h
index cb59f82..ffa27e3 100644
--- a/lib/myopt.h
+++ b/lib/myopt.h
@@ -39,4 +39,12 @@ struct cmdinfo {
void myfileopt(const char* fn, const struct cmdinfo* cmdinfos);
void myopt(const char *const **argvp, const struct cmdinfo *cmdinfos);
void loadcfgfile(const char *prog, const struct cmdinfo *cmdinfos);
+void loadfilter(char *fn);
+
+struct filterlist {
+ int positive;
+ char *filterstring;
+ struct filterlist *next;
+};
+
This does not feel like it has much to do with option parsing.
Probably best moved to src/filters.h as well.
diff --git a/src/archives.c b/src/archives.c
index df21d27..a123940 100644
--- a/src/archives.c
+++ b/src/archives.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#define obstack_chunk_alloc m_malloc
@@ -367,6 +368,8 @@ static int linktosameexistingdir(const struct TarInfo *ti,
return 1
Re: Review of file exclusion branch requested
On Sun, 2008-01-20 at 06:54:09 +0200, Guillem Jover wrote: > On Sat, 2008-01-05 at 11:18:13 +0100, Tollef Fog Heen wrote: > > I've finally gotten around to fixing up my support for excluding bits > > of packages as they are unpacked. It can be gotten from > > git://git.err.no/dpkg in the master branch (sorry about that, it > > should probably have gone in a separate branch). > > Thanks for working on this. For next time please use git-format-patch, > it makes it easier to review, also it allows one to fix the patches > and resend. Oh, forgot to say, please squash the commits in logical patches, as the history was not that interesting anyway. thanks, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
