git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/6] fsck: check tag objects' headers

From
Jeff King <peff@peff.net>
Date
Sep 3, 2014, 22:29 UTC
Message-ID
<20140903222937.GA30560@peff.net>
In-Reply-To
<xmqqwq9o2s6l.fsf@gitster.dls.corp.google.com>
On Sun, Aug 31, 2014 at 03:46:42PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> If "git fsck" were a tool to validate that the objects and refs are
> in line with how "git-core" plumbing and Porcelain toolset uses the
> underlying Git data model, it makes sense to insist a tag has a name
> that is suitable for a refname, and the tag is pointed by a ref in
> "refs/tags/" followed by its name.  The rules such a "git fsck" should
> implement would be stricter than what the underlying Git data model
> could represent and existing Git tools could handle (i.e. a commit
> with broken ident line may not be usable with "shortlog -e" and would
> be flagged as corrupt).

This is a bit of an aside, but why do we have the "tag" line in the tag object in the first place?

It is part of the object contents, and therefore is part of the signature (which the refname is not). That's somewhat redundant with the tag message itself. E.g., the git v2.0.4 tag says:

  object 32f56600bb6ac6fc57183e79d2c1515dfa56672f
  type commit
  tag v2.0.4
  tagger Junio C Hamano <gitster@pobox.com> 1406755201 -0700
  Git 2.0.4
  -----BEGIN PGP SIGNATURE-----
  ...

Imagine an evil person pushed the signed v2.0.4 tag to refs/tags/v2.1.0 (perhaps because there is a bug in v2.0.4, and they want you to run the wrong version so they can exploit it). You can check with "git show" that the "tag" field is actually v2.0.4, but then you could similarly check that the message says "Git 2.0.4".

The main advantage of the "tag" field is that it is machine-readable, and that your verification process can check that "git verify-tag v2.1.0" actually returns a tag that says "tag v2.1.0". But I do not think we do that verification at all. I wonder if that is something we should add support for.

You gave examples later in your email of tags that would not necessarily care about this tag field (and anyway, if "for-linus" is used over and over, it is subject to these sorts of replays), so I do not think it is something we would want unconditionally in verify-tag.

I think this may need to be filed under "possible policy flags for verifying" that we discussed earlier (i.e., in the same boat as "does the committer ident match the commit signature", as it is a porcelain-ish policy, not an integral part of the plumbing).

So this is mostly food for thought at this point.
-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 16 of 68 in “Improve tag checking in fsck and with transfer.fsckobjects”
  1. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Aug 28, 2014
  2. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Aug 28, 2014
  3. Junio C HamanoAug 28, 2014
  4. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Aug 28, 2014
  5. Junio C HamanoAug 28, 2014
  6. Jeff KingAug 29, 2014
  7. Jeff KingAug 29, 2014
  8. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Aug 28, 2014
  9. Junio C HamanoAug 28, 2014
  10. Jeff KingAug 29, 2014
  11. 4/6 fsck: check tag objects' headersJohannes Schindelin, Aug 28, 2014
  12. Junio C HamanoAug 28, 2014
  13. Junio C HamanoAug 28, 2014
  14. Jeff KingAug 29, 2014
  15. Junio C HamanoAug 31, 2014
  16. Jeff KingSep 3, 2014
  17. Junio C HamanoSep 3, 2014
  18. Jeff KingSep 4, 2014
  19. Jeff KingAug 29, 2014
  20. Junio C HamanoSep 2, 2014
  21. Jeff KingSep 3, 2014
  22. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Aug 28, 2014
  23. 6/6 Make sure that index-pack --strict fails upon invalid tag objectsJohannes Schindelin, Aug 28, 2014
  24. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 10, 2014
  25. Johannes SchindelinSep 10, 2014
  26. Junio C HamanoSep 10, 2014
  27. Junio C HamanoSep 10, 2014
  28. Johannes SchindelinSep 11, 2014
  29. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 11, 2014
  30. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 11, 2014
  31. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 11, 2014
  32. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 11, 2014
  33. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 11, 2014
  34. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 11, 2014
  35. 6/6 Make sure that index-pack --strict checks tag objectsJohannes Schindelin, Sep 11, 2014
  36. Junio C HamanoSep 11, 2014
  37. Junio C HamanoSep 11, 2014
  38. 0/3 hash-object --literallyJunio C Hamano, Sep 11, 2014
  39. 1/3 hash-object: reduce file-scope staticsJunio C Hamano, Sep 11, 2014
  40. 2/3 hash-object: pass 'write_object' as a flagJunio C Hamano, Sep 11, 2014
  41. 3/3 hash-object: add --literally optionJunio C Hamano, Sep 11, 2014
  42. Johannes SchindelinSep 12, 2014
  43. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 12, 2014
  44. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 12, 2014
  45. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 12, 2014
  46. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 12, 2014
  47. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 12, 2014
  48. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 12, 2014
  49. 6/6 Make sure that index-pack --strict checks tag objectsJohannes Schindelin, Sep 12, 2014
  50. Junio C HamanoSep 12, 2014
  51. Johannes SchindelinSep 13, 2014
  52. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 10, 2014
  53. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 10, 2014
  54. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 10, 2014
  55. Junio C HamanoSep 10, 2014
  56. Johannes SchindelinSep 11, 2014
  57. Junio C HamanoSep 11, 2014
  58. Eric SunshineSep 10, 2014
  59. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 10, 2014
  60. Junio C HamanoSep 10, 2014
  61. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 10, 2014
  62. Junio C HamanoSep 10, 2014
  63. Johannes SchindelinSep 11, 2014
  64. 6/6 Make sure that index-pack --strict fails upon invalid tag objectsJohannes Schindelin, Sep 10, 2014
  65. Junio C HamanoSep 10, 2014
  66. Johannes SchindelinSep 11, 2014
  67. Junio C HamanoSep 11, 2014
  68. Johannes SchindelinSep 11, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.