summaryrefslogtreecommitdiff
path: root/builtin/receive-pack.c
AgeCommit message (Collapse)AuthorFilesLines
2021-04-27hash: provide per-algorithm null OIDsLibravatar brian m. carlson1-1/+1
Up until recently, object IDs did not have an algorithm member, only a hash. Consequently, it was possible to share one null (all-zeros) object ID among all hash algorithms. Now that we're going to be handling objects from multiple hash algorithms, it's important to make sure that all object IDs have a correct algorithm field. Introduce a per-algorithm null OID, and add it to struct hash_algo. Introduce a wrapper function as well, and use it everywhere we used to use the null_oid constant. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-19Merge branch 'rs/calloc-array'Libravatar Junio C Hamano1-2/+2
CALLOC_ARRAY() macro replaces many uses of xcalloc(). * rs/calloc-array: cocci: allow xcalloc(1, size) use CALLOC_ARRAY git-compat-util.h: drop trailing semicolon from macro definition
2021-03-19Merge branch 'jc/calloc-fix'Libravatar Junio C Hamano1-5/+3
Code clean-up. * jc/calloc-fix: xcalloc: use CALLOC_ARRAY() when applicable
2021-03-15xcalloc: use CALLOC_ARRAY() when applicableLibravatar Junio C Hamano1-5/+3
These are for codebase before Git 2.31 Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13use CALLOC_ARRAYLibravatar René Scharfe1-2/+2
Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-01Merge branch 'jt/transfer-fsck-across-packs'Libravatar Junio C Hamano1-1/+1
The approach to "fsck" the incoming objects in "index-pack" is attractive for performance reasons (we have them already in core, inflated and ready to be inspected), but fundamentally cannot be applied fully when we receive more than one pack stream, as a tree object in one pack may refer to a blob object in another pack as ".gitmodules", when we want to inspect blobs that are used as ".gitmodules" file, for example. Teach "index-pack" to emit objects that must be inspected later and check them in the calling "fetch-pack" process. * jt/transfer-fsck-across-packs: fetch-pack: print and use dangling .gitmodules fetch-pack: with packfile URIs, use index-pack arg http-fetch: allow custom index-pack args http: allow custom index-pack args
2021-02-22fetch-pack: print and use dangling .gitmodulesLibravatar Jonathan Tan1-1/+1
Teach index-pack to print dangling .gitmodules links after its "keep" or "pack" line instead of declaring an error, and teach fetch-pack to check such lines printed. This allows the tree side of the .gitmodules link to be in one packfile and the blob side to be in another without failing the fsck check, because it is now fetch-pack which checks such objects after all packfiles have been downloaded and indexed (and not index-pack on an individual packfile, as it is before this commit). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10gpg-interface: improve interface for parsing tagsLibravatar brian m. carlson1-2/+2
We have a function which parses a buffer with a signature at the end, parse_signature, and this function is used for signed tags. However, we'll need to store values for multiple algorithms, and we'll do this by using a header for the non-default algorithm. Adjust the parse_signature interface to store the parsed data in two strbufs and turn the existing function into parse_signed_buffer. The latter is still used in places where we know we always have a signed buffer, such as push certs. Adjust all the callers to deal with this new interface. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-08Merge branch 'js/trace2-session-id'Libravatar Junio C Hamano1-0/+15
The transport layer was taught to optionally exchange the session ID assigned by the trace2 subsystem during fetch/push transactions. * js/trace2-session-id: receive-pack: log received client session ID send-pack: advertise session ID in capabilities upload-pack, serve: log received client session ID fetch-pack: advertise session ID in capabilities transport: log received server session ID serve: advertise session ID in v2 capabilities receive-pack: advertise session ID in v0 capabilities upload-pack: advertise session ID in v0 capabilities trace2: add a public function for getting the SID docs: new transfer.advertiseSID option docs: new capability to advertise session IDs
2020-11-11receive-pack: log received client session IDLibravatar Josh Steadmon1-0/+7
When receive-pack receives a session-id capability from the client, log the received session ID via a trace2 data event. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11receive-pack: advertise session ID in v0 capabilitiesLibravatar Josh Steadmon1-0/+8
When transfer.advertiseSID is true, advertise receive-pack's session ID via the new session-id capability. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11receive-pack: use default version 0 for proc-receiveLibravatar Jiang Xin1-1/+6
In the verison negotiation phase between "receive-pack" and "proc-receive", "proc-receive" can send an empty flush-pkt to end the negotiation and use default version 0. Capabilities (such as "push-options") are not supported in version 0. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11receive-pack: gently write messages to proc-receiveLibravatar Jiang Xin1-24/+63
Johannes found a flaky hang in `t5411/test-0013-bad-protocol.sh` in the osx-clang job of the CI/PR builds, and ran into an issue when using the `--stress` option with the following error messages: fatal: unable to write flush packet: Broken pipe send-pack: unexpected disconnect while reading sideband packet fatal: the remote end hung up unexpectedly In this test case, the "proc-receive" hook sends an error message and dies earlier. While "receive-pack" on the other side of the pipe should forward the error message of the "proc-receive" hook to the client side, but it fails to do so. This is because "receive-pack" uses `packet_write_fmt()` and `packet_flush()` to write pkt-line message to "proc-receive" hook, and these functions die immediately when pipe is broken. Using "gently" forms for these functions will get more predicable output. Add more "--die-*" options to test helper to test different stages of the protocol between "receive-pack" and "proc-receive" hook. Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25Merge branch 'jx/proc-receive-hook'Libravatar Junio C Hamano1-9/+463
"git receive-pack" that accepts requests by "git push" learned to outsource most of the ref updates to the new "proc-receive" hook. * jx/proc-receive-hook: doc: add documentation for the proc-receive hook transport: parse report options for tracking refs t5411: test updates of remote-tracking branches receive-pack: new config receive.procReceiveRefs doc: add document for capability report-status-v2 New capability "report-status-v2" for git-push receive-pack: feed report options to post-receive receive-pack: add new proc-receive hook t5411: add basic test cases for proc-receive hook transport: not report a non-head push as a branch
2020-08-27receive-pack: new config receive.procReceiveRefsLibravatar Jiang Xin1-7/+99
Add a new multi-valued config variable "receive.procReceiveRefs" for `receive-pack` command, like the follows: git config --system --add receive.procReceiveRefs refs/for git config --system --add receive.procReceiveRefs refs/drafts If the specific prefix strings given by the config variables match the reference names of the commands which are sent from git client to `receive-pack`, these commands will be executed by an external hook (named "proc-receive"), instead of the internal `execute_commands` function. For example, if it is set to "refs/for", pushing to a reference such as "refs/for/master" will not create or update reference "refs/for/master", but may create or update a pull request directly by running the hook "proc-receive". Optional modifiers can be provided in the beginning of the value to filter commands for specific actions: create (a), modify (m), delete (d). A `!` can be included in the modifiers to negate the reference prefix entry. E.g.: git config --system --add receive.procReceiveRefs ad:refs/heads git config --system --add receive.procReceiveRefs !:refs/heads Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-27New capability "report-status-v2" for git-pushLibravatar Jiang Xin1-2/+52
The new introduced "proc-receive" hook may handle a command for a pseudo-reference with a zero-old as its old-oid, while the hook may create or update a reference with different name, different new-oid, and different old-oid (the reference may exist already with a non-zero old-oid). Current "report-status" protocol cannot report the status for such reference rewrite. Add new capability "report-status-v2" and new report protocol which is not backward compatible for report of git-push. If a user pushes to a pseudo-reference "refs/for/master/topic", and "receive-pack" creates two new references "refs/changes/23/123/1" and "refs/changes/24/124/1", for client without the knowledge of "report-status-v2", "receive-pack" will only send "ok/ng" directives in the report, such as: ok ref/for/master/topic But for client which has the knowledge of "report-status-v2", "receive-pack" will use "option" directives to report more attributes for the reference given by the above "ok/ng" directive. ok refs/for/master/topic option refname refs/changes/23/123/1 option new-oid <new-oid> ok refs/for/master/topic option refname refs/changes/24/124/1 option new-oid <new-oid> The client will report two new created references to the end user. Suggested-by: Junio C Hamano <gitster@pobox.com> Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-27receive-pack: feed report options to post-receiveLibravatar Jiang Xin1-4/+26
When commands are fed to the "post-receive" hook, report options will be parsed and the real old-oid, new-oid, reference name will feed to the "post-receive" hook. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-27receive-pack: add new proc-receive hookLibravatar Jiang Xin1-3/+293
Git calls an internal `execute_commands` function to handle commands sent from client to `git-receive-pack`. Regardless of what references the user pushes, git creates or updates the corresponding references if the user has write-permission. A contributor who has no write-permission, cannot push to the repository directly. So, the contributor has to write commits to an alternate location, and sends pull request by emails or by other ways. We call this workflow as a distributed workflow. It would be more convenient to work in a centralized workflow like what Gerrit provided for some cases. For example, a read-only user who cannot push to a branch directly can run the following `git push` command to push commits to a pseudo reference (has a prefix "refs/for/", not "refs/heads/") to create a code review. git push origin \ HEAD:refs/for/<branch-name>/<session> The `<branch-name>` in the above example can be as simple as "master", or a more complicated branch name like "foo/bar". The `<session>` in the above example command can be the local branch name of the client side, such as "my/topic". We cannot implement a centralized workflow elegantly by using "pre-receive" + "post-receive", because Git will call the internal function "execute_commands" to create references (even the special pseudo reference) between these two hooks. Even though we can delete the temporarily created pseudo reference via the "post-receive" hook, having a temporary reference is not safe for concurrent pushes. So, add a filter and a new handler to support this kind of workflow. The filter will check the prefix of the reference name, and if the command has a special reference name, the filter will turn a specific field (`run_proc_receive`) on for the command. Commands with this filed turned on will be executed by a new handler (a hook named "proc-receive") instead of the internal `execute_commands` function. We can use this "proc-receive" command to create pull requests or send emails for code review. Suggested by Junio, this "proc-receive" hook reads the commands, push-options (optional), and send result using a protocol in pkt-line format. In the following example, the letter "S" stands for "receive-pack" and letter "H" stands for the hook. # Version and features negotiation. S: PKT-LINE(version=1\0push-options atomic...) S: flush-pkt H: PKT-LINE(version=1\0push-options...) H: flush-pkt # Send commands from server to the hook. S: PKT-LINE(<old-oid> <new-oid> <ref>) S: ... ... S: flush-pkt # Send push-options only if the 'push-options' feature is enabled. S: PKT-LINE(push-option) S: ... ... S: flush-pkt # Receive result from the hook. # OK, run this command successfully. H: PKT-LINE(ok <ref>) # NO, I reject it. H: PKT-LINE(ng <ref> <reason>) # Fall through, let 'receive-pack' to execute it. H: PKT-LINE(ok <ref>) H: PKT-LINE(option fall-through) # OK, but has an alternate reference. The alternate reference name # and other status can be given in options H: PKT-LINE(ok <ref>) H: PKT-LINE(option refname <refname>) H: PKT-LINE(option old-oid <old-oid>) H: PKT-LINE(option new-oid <new-oid>) H: PKT-LINE(option forced-update) H: ... ... H: flush-pkt After receiving a command, the hook will execute the command, and may create/update different reference. For example, a command for a pseudo reference "refs/for/master/topic" may create/update different reference such as "refs/pull/123/head". The alternate reference name and other status are given in option lines. The list of commands returned from "proc-receive" will replace the relevant commands that are sent from user to "receive-pack", and "receive-pack" will continue to run the "execute_commands" function and other routines. Finally, the result of the execution of these commands will be reported to end user. The reporting function from "receive-pack" to "send-pack" will be extended in latter commit just like what the "proc-receive" hook reports to "receive-pack". Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30strvec: rename struct fieldsLibravatar Jeff King1-7/+7
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: fix indentation in renamed callsLibravatar Jeff King1-21/+21
Code which split an argv_array call across multiple lines, like: argv_array_pushl(&args, "one argument", "another argument", "and more", NULL); was recently mechanically renamed to use strvec, which results in mis-matched indentation like: strvec_pushl(&args, "one argument", "another argument", "and more", NULL); Let's fix these up to align the arguments with the opening paren. I did this manually by sifting through the results of: git jump grep 'strvec_.*,$' and liberally applying my editor's auto-format. Most of the changes are of the form shown above, though I also normalized a few that had originally used a single-tab indentation (rather than our usual style of aligning with the open paren). I also rewrapped a couple of obvious cases (e.g., where previously too-long lines became short enough to fit on one), but I wasn't aggressive about it. In cases broken to three or more lines, the grouping of arguments is sometimes meaningful, and it wasn't worth my time or reviewer time to ponder each case individually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert builtin/ callers away from argv_array nameLibravatar Jeff King1-34/+34
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts all of the files in builtin/ to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add builtin/". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: rename files from argv-array to strvecLibravatar Jeff King1-1/+1
This requires updating #include lines across the code-base, but that's all fairly mechanical, and was done with: git ls-files '*.c' '*.h' | xargs perl -i -pe 's/argv-array.h/strvec.h/' Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-06Merge branch 'bc/sha-256-part-2'Libravatar Junio C Hamano1-0/+10
SHA-256 migration work continues. * bc/sha-256-part-2: (44 commits) remote-testgit: adapt for object-format bundle: detect hash algorithm when reading refs t5300: pass --object-format to git index-pack t5704: send object-format capability with SHA-256 t5703: use object-format serve option t5702: offer an object-format capability in the test t/helper: initialize the repository for test-sha1-array remote-curl: avoid truncating refs with ls-remote t1050: pass algorithm to index-pack when outside repo builtin/index-pack: add option to specify hash algorithm remote-curl: detect algorithm for dumb HTTP by size builtin/ls-remote: initialize repository based on fetch t5500: make hash independent serve: advertise object-format capability for protocol v2 connect: parse v2 refs with correct hash algorithm connect: pass full packet reader when parsing v2 refs Documentation/technical: document object-format for protocol v2 t1302: expect repo format version 1 for SHA-256 builtin/show-index: provide options to determine hash algo t5302: modernize test formatting ...
2020-05-27builtin/receive-pack: detect when the server doesn't support our hashLibravatar brian m. carlson1-0/+9
Detect when the server doesn't support our hash algorithm and abort. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-27remote: advertise the object-format capability on the server sideLibravatar brian m. carlson1-0/+1
Advertise the current hash algorithm in use by using the object-format capability as part of the ref advertisement. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-13Merge branch 'tb/shallow-cleanup'Libravatar Junio C Hamano1-1/+2
Code cleanup. * tb/shallow-cleanup: shallow: use struct 'shallow_lock' for additional safety shallow.h: document '{commit,rollback}_shallow_file' shallow: extract a header file for shallow-related functions commit: make 'commit_graft_pos' non-static
2020-05-08Merge branch 'cb/avoid-colliding-with-netbsd-hmac'Libravatar Junio C Hamano1-3/+3
The <stdlib.h> header on NetBSD brings in its own definition of hmac() function (eek), which conflicts with our own and unrelated function with the same name. Our function has been renamed to work around the issue. * cb/avoid-colliding-with-netbsd-hmac: builtin/receive-pack: avoid generic function name hmac()
2020-05-05builtin/receive-pack: avoid generic function name hmac()Libravatar Carlo Marcelo Arenas Belón1-3/+3
fabec2c5c3 (builtin/receive-pack: switch to use the_hash_algo, 2019-08-18) renames hmac_sha1 to hmac, as it was updated to use the hash function used by git (which won't be sha1 in the future). hmac() is provided by NetBSD >= 8 libc and therefore conflicts as shown by : builtin/receive-pack.c:421:13: error: conflicting types for 'hmac' static void hmac(unsigned char *out, ^~~~ In file included from ./git-compat-util.h:172:0, from ./builtin.h:4, from builtin/receive-pack.c:1: /usr/include/stdlib.h:305:10: note: previous declaration of 'hmac' was here ssize_t hmac(const char *, const void *, size_t, const void *, size_t, void *, ^~~~ Rename it again to hmac_hash to reflect it will use the git's defined hash function and avoid the conflict, while at it update a comment to better describe the HMAC function that was used. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-01Merge branch 'tb/reset-shallow'Libravatar Junio C Hamano1-2/+2
Fix in-core inconsistency after fetching into a shallow repository that broke the code to write out commit-graph. * tb/reset-shallow: shallow.c: use '{commit,rollback}_shallow_file' t5537: use test_write_lines and indented heredocs for readability
2020-04-30shallow: use struct 'shallow_lock' for additional safetyLibravatar Taylor Blau1-1/+1
In previous patches, the functions 'commit_shallow_file' and 'rollback_shallow_file' were introduced to reset the shallowness validity checks on a repository after potentially modifying '.git/shallow'. These functions can be made safer by wrapping the 'struct lockfile *' in a new type, 'shallow_lock', so that they cannot be called with a raw lock (and potentially misused by other code that happens to possess a lockfile, but has nothing to do with shallowness). This patch introduces that type as a thin wrapper around 'struct lockfile', and updates the two aforementioned functions and their callers to use it. Suggested-by: Junio C Hamano <gitster@pobox.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-30shallow: extract a header file for shallow-related functionsLibravatar Taylor Blau1-0/+1
There are many functions in commit.h that are more related to shallow repositories than they are to any sort of generic commit machinery. Likely this began when there were only a few shallow-related functions, and commit.h seemed a reasonable enough place to put them. But, now there are a good number of shallow-related functions, and placing them all in 'commit.h' doesn't make sense. This patch extracts a 'shallow.h', which takes all of the declarations from 'commit.h' for functions which already exist in 'shallow.c'. We will bring the remaining shallow-related functions defined in 'commit.c' in a subsequent patch. For now, move only the ones that already are implemented in 'shallow.c', and update the necessary includes. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-28Merge branch 'bc/constant-memequal'Libravatar Junio C Hamano1-1/+22
Validation of push certificate has been made more robust against timing attacks. * bc/constant-memequal: receive-pack: compilation fix builtin/receive-pack: use constant-time comparison for HMAC value
2020-04-24shallow.c: use '{commit,rollback}_shallow_file'Libravatar Taylor Blau1-2/+2
In bd0b42aed3 (fetch-pack: do not take shallow lock unnecessarily, 2019-01-10), the author noted that 'is_repository_shallow' produces visible side-effect(s) by setting 'is_shallow' and 'shallow_stat'. This is a problem for e.g., fetching with '--update-shallow' in a shallow repository with 'fetch.writeCommitGraph' enabled, since the update to '.git/shallow' will cause Git to think that the repository isn't shallow when it is, thereby circumventing the commit-graph compatibility check. This causes problems in shallow repositories with at least shallow refs that have at least one ancestor (since the client won't have those objects, and therefore can't take the reachability closure over commits when writing a commit-graph). Address this by introducing thin wrappers over 'commit_lock_file' and 'rollback_lock_file' for use specifically when the lock is held over '.git/shallow'. These wrappers (appropriately called 'commit_shallow_file' and 'rollback_shallow_file') call into their respective functions in 'lockfile.h', but additionally reset validity checks used by the shallow machinery. Replace each instance of 'commit_lock_file' and 'rollback_lock_file' with 'commit_shallow_file' and 'rollback_shallow_file' when the lock being held is over the '.git/shallow' file. As a result, 'prune_shallow' can now only be called once (since 'check_shallow_file_for_update' will die after calling 'reset_repository_shallow'). But, this is OK since we only call 'prune_shallow' at most once per process. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-22receive-pack: compilation fixLibravatar Junio C Hamano1-1/+3
We do not use C99 "for loop initial declaration" in our codebase (yet), but one snuck in. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-09builtin/receive-pack: use constant-time comparison for HMAC valueLibravatar brian m. carlson1-1/+20
When we're comparing a push cert nonce, we currently do so using strcmp. Most implementations of strcmp short-circuit and exit as soon as they know whether two values are equal. This, however, is a problem when we're comparing the output of HMAC, as it leaks information in the time taken about how much of the two values match if they do indeed differ. In our case, the nonce is used to prevent replay attacks against our server via the embedded timestamp and replay attacks using requests from a different server via the HMAC. Push certs, which contain the nonces, are signed, so an attacker cannot tamper with the nonces without breaking validation of the signature. They can, of course, create their own signatures with invalid nonces, but they can also create their own signatures with valid nonces, so there's nothing to be gained. Thus, there is no security problem. Even though it doesn't appear that there are any negative consequences from the current technique, for safety and to encourage good practices, let's use a constant time comparison function for nonce verification. POSIX does not provide one, but they are easy to write. The technique we use here is also used in NaCl and the Go standard library and relies on the fact that bitwise or and xor are constant time on all known architectures. We need not be concerned about exiting early if the actual and expected lengths differ, since the standard cryptographic assumption is that everyone, including an attacker, knows the format of and algorithm used in our nonces (and in any event, they have the source code and can determine it easily). As a result, we assume everyone knows how long our nonces should be. This philosophy is also taken by the Go standard library and other cryptographic libraries when performing constant time comparisons on HMAC values. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-30oid_array: rename source file from sha1-arrayLibravatar Jeff King1-1/+1
We renamed the actual data structure in 910650d2f8 (Rename sha1_array to oid_array, 2017-03-31), but the file is still called sha1-array. Besides being slightly confusing, it makes it more annoying to grep for leftover occurrences of "sha1" in various files, because the header is included in so many places. Let's complete the transition by renaming the source and header files (and fixing up a few comment references). I kept the "-" in the name, as that seems to be our style; cf. fc1395f4a4 (sha1_file.c: rename to use dash in file name, 2018-04-10). We also have oidmap.h and oidset.h without any punctuation, but those are "struct oidmap" and "struct oidset" in the code. We _could_ make this "oidarray" to match, but somehow it looks uglier to me because of the length of "array" (plus it would be a very invasive patch for little gain). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24receive.denyCurrentBranch: respect all worktreesLibravatar Hariom Verma1-17/+19
The receive.denyCurrentBranch config option controls what happens if you push to a branch that is checked out into a non-bare repository. By default, it rejects it. It can be disabled via `ignore` or `warn`. Another yet trickier option is `updateInstead`. However, this setting was forgotten when the git worktree command was introduced: only the main worktree's current branch is respected. With this change, all worktrees are respected. That change also leads to revealing another bug, i.e. `receive.denyCurrentBranch = true` was ignored when pushing into a non-bare repository's unborn current branch using ref namespaces. As `is_ref_checked_out()` returns 0 which means `receive-pack` does not get into conditional statement to switch `deny_current_branch` accordingly (ignore, warn, refuse, unconfigured, updateInstead). receive.denyCurrentBranch uses the function `refs_resolve_ref_unsafe()` (called via `resolve_refdup()`) to resolve the symbolic ref HEAD, but that function fails when HEAD does not point at a valid commit. As we replace the call to `refs_resolve_ref_unsafe()` with `find_shared_symref()`, which has no problem finding the worktree for a given branch even if it is unborn yet, this bug is fixed at the same time: receive.denyCurrentBranch now also handles worktrees with unborn branches as intended even while using ref namespaces. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Hariom Verma <hariom18599@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19builtin/receive-pack: replace sha1_to_hexLibravatar brian m. carlson1-4/+4
Since sha1_to_hex is limited to SHA-1, replace it with hash_to_hex. Rename several variables to indicate that they can contain any hash. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19builtin/receive-pack: switch to use the_hash_algoLibravatar brian m. carlson1-23/+21
The push cert code uses HMAC-SHA-1 to create a nonce. This is a secure use of SHA-1 which is not affected by its collision resistance (or lack thereof). However, it makes sense for us to use a better algorithm if one is available, one which may even be more performant. Futhermore, until we have specialized functions for computing the hex value of an arbitrary function, it simplifies the code greatly to use the same hash algorithm everywhere. Switch this code to use GIT_MAX_BLKSZ and the_hash_algo for computing the push cert nonce, and rename the hmac_sha1 function to simply "hmac". Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-19Merge branch 'jk/check-connected-with-alternates'Libravatar Junio C Hamano1-1/+0
The tips of refs from the alternate object store can be used as starting point for reachability computation now. * jk/check-connected-with-alternates: check_everything_connected: assume alternate ref tips are valid object-store.h: move for_each_alternate_ref() from transport.h
2019-07-09Merge branch 'ds/close-object-store'Libravatar Junio C Hamano1-1/+1
The commit-graph file is now part of the "files that the runtime may keep open file descriptors on, all of which would need to be closed when done with the object store", and the file descriptor to an existing commit-graph file now is closed before "gc" finalizes a new instance to replace it. * ds/close-object-store: packfile: rename close_all_packs to close_object_store packfile: close commit-graph in close_all_packs commit-graph: use raw_object_store when closing
2019-07-01object-store.h: move for_each_alternate_ref() from transport.hLibravatar Jeff King1-1/+0
There's nothing inherently transport-related about enumerating the alternate ref tips. The code has lived in transport.[ch] because the only use so far had been advertising available tips during transport. But it could be used for more, and a future patch will teach rev-list to access these refs. Let's move it alongside the other alt-odb code, declaring it in object-store.h with the implementation in sha1-file.c. This lets us drop the inclusion of transport.h from receive-pack, which perhaps shows how it was misplaced (though receive-pack is about transporting objects, transport.h is mostly about the client side). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-12packfile: rename close_all_packs to close_object_storeLibravatar Derrick Stolee1-1/+1
The close_all_packs() method is now responsible for more than just pack-files. It also closes the commit-graph and the multi-pack-index. Rename the function to be more descriptive of its larger role. The name also fits because the input parameter is a raw_object_store. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-13receive-pack: drop unused "commands" from prepare_shallow_update()Libravatar Jeff King1-3/+2
We pass in the list of proposed ref updates to prepare_shallow_update(), but that function doesn't actually need it (and never has since its inception in 0a1bc12b6e4). Only its caller, update_shallow_info(), needs to look at the command list. Let's drop the unused parameter to reduce confusion. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07Merge branch 'ab/receive-pack-use-after-free-fix'Libravatar Junio C Hamano1-8/+15
Memfix. * ab/receive-pack-use-after-free-fix: receive-pack: fix use-after-free bug
2019-02-22trace2:data: add trace2 hook classificationLibravatar Jeff Hostetler1-0/+4
Classify certain child processes as hooks. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-20receive-pack: fix use-after-free bugLibravatar Ævar Arnfjörð Bjarmason1-8/+15
The resolve_ref_unsafe() function can, and sometimes will in the case of this codepath, return the char * passed to it to the caller. In this case we construct a strbuf, free it, and then continue using the dst_name after that free(). The code being fixed dates back to da3efdb17b ("receive-pack: detect aliased updates which can occur with symrefs", 2010-04-19). When it was originally added it didn't have this bug, it was introduced when it was subsequently modified to use strbuf in 6b01ecfe22 ("ref namespaces: Support remote repositories via upload-pack and receive-pack", 2011-07-08). This is theoretically a security issue, the C standard makes no guarantees that a value you use after free() hasn't been poked at or changed by something else on the system, but in practice modern OSs will have mapped the relevant page to this process, so nothing else would have used it. We do no further allocations between the free() and use-after-free, so we ourselves didn't corrupt or change the value. Jeff investigated that and found: "It probably would be an issue if the allocation were larger. glibc at least will use mmap()/munmap() after some cutoff[1], in which case we'd get a segfault from hitting the unmapped page. But for small allocations, it just bumps brk() and the memory is still available for further allocations after free(). [...] If you had a sufficiently large refname you might be able to trigger the bug [...]. I tried to push such a ref. I had to manually make a packed-refs file with the long name to avoid filesystem limits (though probably you could have a long a/b/c/ name on ext4). But the result can't actually be pushed, because it all has to fit into a 64k pkt-line as part of the push protocol.". An a alternative and more succinct way of implementing this would have been to do the strbuf_release() at the end of check_aliased_update() and use "goto out" instead of the early "return" statements. Hopefully this approach of using a helper instead makes it easier to follow. 1. Jeff: "Weirdly, the mmap() cutoff on my glibc system is 135168 bytes. Which is...2^17 + 2^12? 33 pages? I'm sure there's a good reason for that, but I didn't dig into it." Reported-by: 王健强 <jianqiang.wang@securitygossip.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-02pack-protocol.txt: accept error packets in any contextLibravatar Masaya Suzuki1-1/+3
In the Git pack protocol definition, an error packet may appear only in a certain context. However, servers can face a runtime error (e.g. I/O error) at an arbitrary timing. This patch changes the protocol to allow an error packet to be sent instead of any packet. Without this protocol spec change, when a server cannot process a request, there's no way to tell that to a client. Since the server cannot produce a valid response, it would be forced to cut a connection without telling why. With this protocol spec change, the server can be more gentle in this situation. An old client may see these error packets as an unexpected packet, but this is not worse than having an unexpected EOF. Following this protocol spec change, the error packet handling code is moved to pkt-line.c. Implementation wise, this implementation uses pkt-line to communicate with a subprocess. Since this is not a part of Git protocol, it's possible that a packet that is not supposed to be an error packet is mistakenly parsed as an error packet. This error packet handling is enabled only for the Git pack protocol parsing code considering this. Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-02Use packet_reader instead of packet_read_lineLibravatar Masaya Suzuki1-29/+31
By using and sharing a packet_reader while handling a Git pack protocol request, the same reader option is used throughout the code. This makes it easy to set a reader option to the request parsing code. Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-30Merge branch 'jc/receive-deny-current-branch-fix'Libravatar Junio C Hamano1-3/+9
The receive.denyCurrentBranch=updateInstead codepath kicked in even when the push should have been rejected due to other reasons, such as it does not fast-forward or the update-hook rejects it, which has been corrected. * jc/receive-deny-current-branch-fix: receive: denyCurrentBranch=updateinstead should not blindly update