summaryrefslogtreecommitdiff
path: root/gpg-interface.c
AgeCommit message (Collapse)Author
2024-02-12Merge branch 'jc/sign-buffer-failure-propagation-fix'Junio C Hamano
A failed "git tag -s" did not necessarily result in an error depending on the crypto backend, which has been corrected. * jc/sign-buffer-failure-propagation-fix: ssh signing: signal an error with a negative return value tag: fix sign_buffer() call to create a signed tag
2024-02-08ssh signing: signal an error with a negative return valueJunio C Hamano
The other backend for the sign_buffer() function followed our usual "an error is signalled with a negative return" convention, but the SSH signer did not. Even though we already fixed the caller that assumed only a negative return value is an error, tighten the callee to signal an error with a negative return as well. This way, the callees will be strict on what they produce, while the callers will be lenient in what they accept. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-08Merge branch 'en/header-cleanup'Junio C Hamano
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2023-12-26treewide: remove unnecessary includes in source filesElijah Newren
Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-08gpg-interface: drop pointless config_error_nonbool() checksJeff King
Config callbacks which use git_config_string() or git_config_pathname() have no need to check for a NULL value. This is handled automatically by those helpers. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-17Merge branch 'cw/compat-util-header-cleanup'Junio C Hamano
Further shuffling of declarations across header files to streamline file dependencies. * cw/compat-util-header-cleanup: git-compat-util: move alloc macros to git-compat-util.h treewide: remove unnecessary includes for wrapper.h kwset: move translation table from ctype sane-ctype.h: create header for sane-ctype macros git-compat-util: move wrapper.c funcs to its header git-compat-util: move strbuf.c funcs to its header
2023-07-06Merge branch 'gc/config-context'Junio C Hamano
Reduce reliance on a global state in the config reading API. * gc/config-context: config: pass source to config_parser_event_fn_t config: add kvi.path, use it to evaluate includes config.c: remove config_reader from configsets config: pass kvi to die_bad_number() trace2: plumb config kvi config.c: pass ctx with CLI config config: pass ctx with config files config.c: pass ctx in configsets config: add ctx arg to config_fn_t urlmatch.h: use config_fn_t type config: inline git_color_default_config
2023-07-05treewide: remove unnecessary includes for wrapper.hCalvin Wan
Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: add ctx arg to config_fn_tGlen Choo
Add a new "const struct config_context *ctx" arg to config_fn_t to hold additional information about the config iteration operation. config_context has a "struct key_value_info kvi" member that holds metadata about the config source being read (e.g. what kind of config source it is, the filename, etc). In this series, we're only interested in .kvi, so we could have just used "struct key_value_info" as an arg, but config_context makes it possible to add/adjust members in the future without changing the config_fn_t signature. We could also consider other ways of organizing the args (e.g. moving the config name and value into config_context or key_value_info), but in my experiments, the incremental benefit doesn't justify the added complexity (e.g. a config_fn_t will sometimes invoke another config_fn_t but with a different config value). In subsequent commits, the .kvi member will replace the global "struct config_reader" in config.c, making config iteration a global-free operation. It requires much more work for the machinery to provide meaningful values of .kvi, so for now, merely change the signature and call sites, pass NULL as a placeholder value, and don't rely on the arg in any meaningful way. Most of the changes are performed by contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every config_fn_t: - Modifies the signature to accept "const struct config_context *ctx" - Passes "ctx" to any inner config_fn_t, if needed - Adds UNUSED attributes to "ctx", if needed Most config_fn_t instances are easily identified by seeing if they are called by the various config functions. Most of the remaining ones are manually named in the .cocci patch. Manual cleanups are still needed, but the majority of it is trivial; it's either adjusting config_fn_t that the .cocci patch didn't catch, or adding forward declarations of "struct config_context ctx" to make the signatures make sense. The non-trivial changes are in cases where we are invoking a config_fn_t outside of config machinery, and we now need to decide what value of "ctx" to pass. These cases are: - trace2/tr2_cfg.c:tr2_cfg_set_fl() This is indirectly called by git_config_set() so that the trace2 machinery can notice the new config values and update its settings using the tr2 config parsing function, i.e. tr2_cfg_cb(). - builtin/checkout.c:checkout_main() This calls git_xmerge_config() as a shorthand for parsing a CLI arg. This might be worth refactoring away in the future, since git_xmerge_config() can call git_default_config(), which can do much more than just parsing. Handle them by creating a KVI_INIT macro that initializes "struct key_value_info" to a reasonable default, and use that to construct the "ctx" arg. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12strbuf: remove global variableCalvin Wan
As a library that only interacts with other primitives, strbuf should not utilize the comment_line_char global variable within its functions. Therefore, add an additional parameter for functions that use comment_line_char and refactor callers to pass it in instead. strbuf_stripspace() removes the skip_comments boolean and checks if comment_line_char is a non-NUL character to determine whether to skip comments or not. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-09Merge branch 'en/header-split-cache-h-part-2'Junio C Hamano
More header clean-up. * en/header-split-cache-h-part-2: (22 commits) reftable: ensure git-compat-util.h is the first (indirect) include diff.h: reduce unnecessary includes object-store.h: reduce unnecessary includes commit.h: reduce unnecessary includes fsmonitor: reduce includes of cache.h cache.h: remove unnecessary headers treewide: remove cache.h inclusion due to previous changes cache,tree: move basic name compare functions from read-cache to tree cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c hash-ll.h: split out of hash.h to remove dependency on repository.h tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h dir.h: move DTYPE defines from cache.h versioncmp.h: move declarations for versioncmp.c functions from cache.h ws.h: move declarations for ws.c functions from cache.h match-trees.h: move declarations for match-trees.c functions from cache.h pkt-line.h: move declarations for pkt-line.c functions from cache.h base85.h: move declarations for base85.c functions from cache.h copy.h: move declarations for copy.c functions from cache.h server-info.h: move declarations for server-info.c functions from cache.h packfile.h: move pack_window and pack_entry from cache.h ...
2023-04-28Merge branch 'jk/gpg-trust-level-fix'Junio C Hamano
The "%GT" placeholder for the "--format" option of "git log" and friends caused BUG() to trigger on a commit signed with an unknown key, which has been corrected. * jk/gpg-trust-level-fix: gpg-interface: set trust level of missing key to "undefined"
2023-04-24commit.h: reduce unnecessary includesElijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24hash-ll.h: split out of hash.h to remove dependency on repository.hElijah Newren
hash.h depends upon and includes repository.h, due to the definition and use of the_hash_algo (defined as the_repository->hash_algo). However, most headers trying to include hash.h are only interested in the layout of the structs like object_id. Move the parts of hash.h that do not depend upon repository.h into a new file hash-ll.h (the "low level" parts of hash.h), and adjust other files to use this new header where the convenience inline functions aren't needed. This allows hash.h and object.h to be fairly small, minimal headers. It also exposes a lot of hidden dependencies on both path.h (which was brought in by repository.h) and repository.h (which was previously implicitly brought in by object.h), so also adjust other files to be more explicit about what they depend upon. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-19gpg-interface: set trust level of missing key to "undefined"Jeff King
In check_signature(), we initialize the trust_level field to "-1", with the idea that if gpg does not return a trust level at all (if there is no signature, or if the signature is made by an unknown key), we'll use that value. But this has two problems: 1. Since the field is an enum, it's up to the compiler to decide what underlying storage to use, and it only has to fit the values we've declared. So we may not be able to store "-1" at all. And indeed, on my system (linux with gcc), the resulting enum is an unsigned 32-bit value, and -1 becomes 4294967295. The difference may seem academic (and you even get "-1" if you pass it to printf("%d")), but it means that code like this: status |= sigc->trust_level < configured_min_trust_level; does not necessarily behave as expected. This turns out not to be a bug in practice, though, because we keep the "-1" only when gpg did not report a signature from a known key, in which case the line above: status |= sigc->result != 'G'; would always set status to non-zero anyway. So only a 'G' signature with no parsed trust level would cause a problem, which doesn't seem likely to trigger (outside of unexpected gpg behavior). 2. When using the "%GT" format placeholder, we pass the value to gpg_trust_level_to_str(), which complains that the value is out of range with a BUG(). This behavior was introduced by 803978da49 (gpg-interface: add function for converting trust level to string, 2022-07-11). Before that, we just did a switch() on the enum, and anything that wasn't matched would end up as the empty string. Curiously, solving this by naively doing: if (level < 0) return ""; in that function isn't sufficient. Because of (1) above, the compiler can (and does in my case) actually remove that conditional as dead code! We can solve both by representing this state as an enum value. We could do this by adding a new "unknown" value. But this really seems to match the existing "undefined" level well. GPG describes this as "Not enough information for calculation". We have tests in t7510 that trigger this case (verifying a signature from a key that we don't have, and then checking various %G placeholders), but they didn't notice the BUG() because we didn't look at %GT for that case! Let's make sure we check all %G placeholders for each case in the formatting tests. The interesting ones here are "show unknown signature with custom format" and "show lack of signature with custom format", both of which would BUG() before, and now turn %GT into "undefined". Prior to 803978da49 they would have turned it into the empty string, but I think saying "undefined" consistently is a reasonable outcome, and probably makes life easier for anyone parsing the output (and any such parser had to be ready to see "undefined" already). The other modified tests produce the same output before and after this patch, but now we're consistently checking both %G? and %GT in all of them. Signed-off-by: Jeff King <peff@peff.net> Reported-by: Rolf Eike Beer <eb@emlix.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21treewide: remove unnecessary includes of cache.hElijah Newren
The last several commits were geared at replacing the include of cache.h in strbuf.c with an include of git-compat-util.h. Unfortunately, I had to drop a patch moving some functions from cache.h to object-name.h, due to excessive conflicts with other in-flight topics. However, even without that patch, the series of patches so far allows us to modify a number of C files to replace an include of cache.h with git-compat-util.h. Do that to reduce our dependencies. (If we could have kept our object-name.h patch in this series, it would have also let us reduce the includes in checkout.c and fmt-merge-msg.c in addition to strbuf.c). Just to ensure that nothing else was bringing in cache.h, all of the affected files have been checked to ensure that gcc -E -I. $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21wrapper.h: move declarations for wrapper.c functions from cache.hElijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21cache.h: remove expand_user_path()Elijah Newren
expand_user_path() was renamed to interpolate_path() back in mid-2021, but reinstated with a #define and a NEEDSWORK comment that we would eventually want to get rid of it. Do so now. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21treewide: be explicit about dependence on gettext.hElijah Newren
Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-17Merge branch 'jc/gpg-lazy-init'Junio C Hamano
Instead of forcing each command to choose to honor GPG related configuration variables, make the subsystem lazily initialize itself. * jc/gpg-lazy-init: drop pure pass-through config callbacks gpg-interface: lazily initialize and read the configuration
2023-03-17Merge branch 'en/header-cleanup'Junio C Hamano
Code clean-up to clarify the rule that "git-compat-util.h" must be the first to be included. * en/header-cleanup: diff.h: remove unnecessary include of object.h Remove unnecessary includes of builtin.h treewide: replace cache.h with more direct headers, where possible replace-object.h: move read_replace_refs declaration from cache.h to here object-store.h: move struct object_info from cache.h dir.h: refactor to no longer need to include cache.h object.h: stop depending on cache.h; make cache.h depend on object.h ident.h: move ident-related declarations out of cache.h pretty.h: move has_non_ascii() declaration from commit.h cache.h: remove dependence on hex.h; make other files include it explicitly hex.h: move some hex-related declarations from cache.h hash.h: move some oid-related declarations from cache.h alloc.h: move ALLOC_GROW() functions from cache.h treewide: remove unnecessary cache.h includes in source files treewide: remove unnecessary cache.h includes treewide: remove unnecessary git-compat-util.h includes in headers treewide: ensure one of the appropriate headers is sourced first
2023-02-24Merge branch 'js/gpg-errors'Junio C Hamano
Error messages given upon a signature verification failure used to discard the errors from underlying gpg program, which has been corrected. * js/gpg-errors: gpg: do show gpg's error message upon failure t7510: add a test case that does not need gpg
2023-02-24ident.h: move ident-related declarations out of cache.hElijah Newren
These functions were all defined in a separate ident.c already, so create ident.h and move the declarations into that file. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-15gpg: do show gpg's error message upon failureJohannes Schindelin
There are few things more frustrating when signing a commit fails than reading a terse "error: gpg failed to sign the data" message followed by the unsurprising "fatal: failed to write commit object" message. In many cases where signing a commit or tag fails, `gpg` actually said something helpful, on its stderr, and Git even consumed that, but then keeps mum about it. Teach Git to stop withholding that rather important information. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10gpg-interface: lazily initialize and read the configurationJunio C Hamano
Instead of forcing the porcelain commands to always read the configuration variables related to the signing and verifying signatures, lazily initialize the necessary subsystem on demand upon the first use. This hopefully would make it more future-proof as we do not have to think and decide whether we should call git_gpg_config() in the git_config() callback for each command. A few git_config() callback functions that used to be custom callbacks are now just a thin wrapper around git_default_config(). We could further remove, git_FOO_config and replace calls to git_config(git_FOO_config) with git_config(git_default_config), but to make it clear which ones are affected and the effect is only the removal of git_gpg_config(), it is vastly preferred not to do such a change in this step (they can be done on top once the dust settled). Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25ssh signing: better error message when key not in agentAdam Szkoda
When signing a commit with a SSH key, with the private key missing from ssh-agent, a confusing error message is produced: error: Load key "/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7": invalid format? fatal: failed to write commit object The temporary file .git_signing_key_tmpkArSj7 created by git contains a valid *public* key. The error message comes from `ssh-keygen -Y sign' and is caused by a fallback mechanism in ssh-keygen whereby it tries to interpret .git_signing_key_tmpkArSj7 as a *private* key if it can't find in the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to 'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent. As a result, when the private key is missing from the agent, a more accurate error message gets produced: error: Couldn't find key in agent [1] https://bugzilla.mindrot.org/show_bug.cgi?id=3429 Signed-off-by: Adam Szkoda <adaszko@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-11Merge branch 'pw/ssh-sign-report-errors'Junio C Hamano
The codepath to sign learned to report errors when it fails to read from "ssh-keygen". * pw/ssh-sign-report-errors: ssh signing: return an error when signature cannot be read
2022-10-05ssh signing: return an error when signature cannot be readPhillip Wood
If the signature file cannot be read we print an error message but do not return an error to the caller. In practice it seems unlikely that the file would be unreadable if the call to ssh-keygen succeeds. The unlink_or_warn() call is moved to the end of the function so that we always try and remove the signature file. This isn't strictly necessary at the moment but it protects us against any extra code being added between trying to read the signature file and the cleanup at the end of the function in the future. unlink_or_warn() only prints a warning if it exists and cannot be removed. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-01git-compat-util.h: use "UNUSED", not "UNUSED(var)"Ævar Arnfjörð Bjarmason
As reported in [1] the "UNUSED(var)" macro introduced in 2174b8c75de (Merge branch 'jk/unused-annotation' into next, 2022-08-24) breaks coccinelle's parsing of our sources in files where it occurs. Let's instead partially go with the approach suggested in [2] of making this not take an argument. As noted in [1] "coccinelle" will ignore such tokens in argument lists that it doesn't know about, and it's less of a surprise to syntax highlighters. This undoes the "help us notice when a parameter marked as unused is actually use" part of 9b240347543 (git-compat-util: add UNUSED macro, 2022-08-19), a subsequent commit will further tweak the macro to implement a replacement for that functionality. 1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/ 2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19config: mark unused callback parametersJeff King
The callback passed to git_config() must conform to a particular interface. But most callbacks don't actually look at the extra "void *data" parameter. Let's mark the unused parameters to make -Wunused-parameter happy. Note there's one unusual case here in get_remote_default() where we actually ignore the "value" parameter. That's because it's only checking whether the option is found at all, and not parsing its value. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-11gpg-interface: add function for converting trust level to stringJaydeep Das
Add new helper function `gpg_trust_level_to_str()` which will convert a given member of `enum signature_trust_level` to its corresponding string (in lowercase). For example, `TRUST_ULTIMATE` will yield the string "ultimate". This will abstract out some code in `pretty.c` relating to gpg signature trust levels. Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Hariom Verma <hariom18599@gmail.com> Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-06-17i18n: fix mismatched camelCase config variablesJiang Xin
Some config variables are combinations of multiple words, and we typically write them in camelCase forms in manpage and translatable strings. It's not easy to find mismatches for these camelCase config variables during code reviews, but occasionally they are identified during localization translations. To check for mismatched config variables, I introduced a new feature in the helper program for localization[^1]. The following mismatched config variables have been identified by running the helper program, such as "git-po-helper check-pot". Lowercase in manpage should use camelCase: * Documentation/config/http.txt: http.pinnedpubkey Lowercase in translable strings should use camelCase: * builtin/fast-import.c: pack.indexversion * builtin/gc.c: gc.logexpiry * builtin/index-pack.c: pack.indexversion * builtin/pack-objects.c: pack.indexversion * builtin/repack.c: pack.writebitmaps * commit.c: i18n.commitencoding * gpg-interface.c: user.signingkey * http.c: http.postbuffer * submodule-config.c: submodule.fetchjobs Mismatched camelCases, choose the former: * Documentation/config/transfer.txt: transfer.credentialsInUrl remote.c: transfer.credentialsInURL [^1]: https://github.com/git-l10n/git-po-helper Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-13Merge branch 'fs/gpgsm-update'Junio C Hamano
Newer version of GPGSM changed its output in a backward incompatible way to break our code that parses its output. It also added more processes our tests need to kill when cleaning up. Adjustments have been made to accommodate these changes. * fs/gpgsm-update: t/lib-gpg: kill all gpg components, not just gpg-agent t/lib-gpg: reload gpg components after updating trustlist gpg-interface/gpgsm: fix for v2.3
2022-03-04gpg-interface/gpgsm: fix for v2.3Fabian Stelzer
Checking if signing was successful will now accept '[GNUPG]: SIG_CREATED' on the beginning of the first or any subsequent line. Not just explictly the second one anymore. Gpgsm v2.3 changed its output when listing keys from `fingerprint` to `sha1/2 fpr`. This leads to the gpgsm tests silently not being executed because of a failed prerequisite. Switch to gpg's `--with-colons` output format when evaluating test prerequisites to make parsing more robust. This also allows us to combine the existing grep/cut/tr/echo pipe for writing the trustlist.txt into a single awk expression. Adjust error message checking in test for v2.3 specific output changes. Helped-By: Junio C Hamano <gitster@pobox.com> Helped-By: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-25Merge branch 'ja/i18n-common-messages'Junio C Hamano
Unify more messages to help l10n. * ja/i18n-common-messages: i18n: fix some misformated placeholders in command synopsis i18n: remove from i18n strings that do not hold translatable parts i18n: factorize "invalid value" messages i18n: factorize more 'incompatible options' messages
2022-02-04i18n: factorize "invalid value" messagesJean-Noël Avila
Use the same message when an invalid value is passed to a command line option or a configuration variable. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-07gpg-interface: trim CR from ssh-keygenFabian Stelzer
We need to trim \r from the output of 'ssh-keygen -Y find-principals' on Windows, or we end up calling 'ssh-keygen -Y verify' with a bogus signer identity. ssh-keygen.c:2841 contains a call to puts(3), which confirms this hypothesis. Signature verification passes with the fix. Helped-by: Pedro Martelletto <pedro@yubico.com> Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-21Merge branch 'fs/ssh-signing-other-keytypes'Junio C Hamano
The cryptographic signing using ssh keys can specify literal keys for keytypes whose name do not begin with the "ssh-" prefix by using the "key::" prefix mechanism (e.g. "key::ecdsa-sha2-nistp256"). * fs/ssh-signing-other-keytypes: ssh signing: make sign/amend test more resilient ssh signing: support non ssh-* keytypes
2021-12-09ssh signing: make verify-commit consider key lifetimeFabian Stelzer
If valid-before/after dates are configured for this signatures key in the allowedSigners file then the verification should check if the key was valid at the time the commit was made. This allows for graceful key rollover and revoking keys without invalidating all previous commits. This feature needs openssh > 8.8. Older ssh-keygen versions will simply ignore this flag and use the current time. Strictly speaking this feature is available in 8.7, but since 8.7 has a bug that makes it unusable in another needed call we require 8.8. Timestamp information is present on most invocations of check_signature. However signer ident is not. We will need the signer email / name to be able to implement "Trust on first use" functionality later. Since the payload contains all necessary information we can parse it from there. The caller only needs to provide us some info about the payload by setting payload_type in the signature_check struct. - Add payload_type field & enum and payload_timestamp to struct signature_check - Populate the timestamp when not already set if we know about the payload type - Pass -Overify-time={payload_timestamp} in the users timezone to all ssh-keygen verification calls - Set the payload type when verifying commits - Add tests for expired, not yet valid and keys having a commit date outside of key validity as well as within Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09ssh signing: use sigc struct to pass payloadFabian Stelzer
To be able to extend the payload metadata with things like its creation timestamp or the creators ident we remove the payload parameters to check_signature() and use the already existing sigc->payload field instead, only adding the length field to the struct. This also allows us to get rid of the xmemdupz() calls in the verify functions. Since sigc is now used to input data as well as output the result move it to the front of the function list. - Add payload_length to struct signature_check - Populate sigc.payload/payload_len on all call sites - Remove payload parameters to check_signature() - Remove payload parameters to internal verify_* functions and use sigc instead - Remove xmemdupz() used for verbose output since payload is now already populated. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-19ssh signing: support non ssh-* keytypesFabian Stelzer
The user.signingKey config for ssh signing supports either a path to a file containing the key or for the sake of convenience a literal string with the ssh public key. To differentiate between those two cases we check if the first few characters contain "ssh-" which is unlikely to be the start of a path. ssh supports other key types which are not prefixed with "ssh-" and will currently be treated as a file path and therefore fail to load. To remedy this we move the prefix check into its own function and introduce the prefix `key::` for literal ssh keys. This way we don't need to add new key types when they become available. The existing `ssh-` prefix is retained for compatibility with current user configs but removed from the official documentation to discourage its use. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-03Merge branch 'jx/message-fixes'Junio C Hamano
Fixes to recently added messages. * jx/message-fixes: i18n: fix typos found during l10n for git 2.34.0
2021-11-02gpg-interface: avoid buffer overrun in parse_ssh_output()René Scharfe
If the string "key" we found in the output of ssh-keygen happens to be located at the very end of the line, then going four characters further leaves us beyond the end of the string. Explicitly search for the space after "key" to handle a missing one gracefully. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-02gpg-interface: handle missing " with " gracefully in parse_ssh_output()René Scharfe
If the output of ssh-keygen starts with "Good \"git\" signature for ", but is not followed by " with " for some reason, then parse_ssh_output() uses -1 as the len parameter of xmemdupz(), which in turn will end the program. Reject the signature and carry on instead in that case. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-01i18n: fix typos found during l10n for git 2.34.0Jiang Xin
Emir and Jean-Noël reported typos in some i18n messages when preparing l10n for git 2.34.0. * Fix unstable spelling of config variable "gpg.ssh.defaultKeyCommand" which was introduced in commit fd9e226776 (ssh signing: retrieve a default key from ssh-agent, 2021-09-10). * Add missing space between "with" and "--python" which was introduced in commit bd0708c7eb (ref-filter: add %(raw) atom, 2021-07-26). * Fix unmatched single quote in 'builtin/index-pack.c' which was introduced in commit 8737dab346 (index-pack: refactor renaming in final(), 2021-09-09) [1] https://github.com/git-l10n/git-po/pull/567 Reported-by: Emir Sarı <bitigchi@me.com> Reported-by: Jean-Noël Avila <jn.avila@free.fr> Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-18gpg-interface: fix leak of strbufs in get_ssh_key_fingerprint()Jeff King
We read stdout from gpg into a strbuf, then split it into a list of strbufs, pull out one element, and return it. But we don't free either the original stdout buffer, nor the list returned from strbuf_split(). This patch fixes both. Note that we have to detach the returned string from its strbuf before calling strbuf_list_free(), as that would otherwise throw it away. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-18gpg-interface: fix leak of "line" in parse_ssh_output()Jeff King
We xmemdupz() this buffer, but never free it. Let's do so. We'll use a cleanup label, since there are multiple exits from the function. Note that it was also declared a "const char *". We could switch that to "char *" to indicate that it's allocated, but that make it awkward to use with skip_prefix(). So instead, we'll introduce an extra non-const pointer. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-10ssh signing: verify signatures using ssh-keygenFabian Stelzer
To verify a ssh signature we first call ssh-keygen -Y find-principal to look up the signing principal by their public key from the allowedSignersFile. If the key is found then we do a verify. Otherwise we only validate the signature but can not verify the signers identity. Verification uses the gpg.ssh.allowedSignersFile (see ssh-keygen(1) "ALLOWED SIGNERS") which contains valid public keys and a principal (usually user@domain). Depending on the environment this file can be managed by the individual developer or for example generated by the central repository server from known ssh keys with push access. This file is usually stored outside the repository, but if the repository only allows signed commits/pushes, the user might choose to store it in the repository. To revoke a key put the public key without the principal prefix into gpg.ssh.revocationKeyring or generate a KRL (see ssh-keygen(1) "KEY REVOCATION LISTS"). The same considerations about who to trust for verification as with the allowedSignersFile apply. Using SSH CA Keys with these files is also possible. Add "cert-authority" as key option between the principal and the key to mark it as a CA and all keys signed by it as valid for this CA. See "CERTIFICATES" in ssh-keygen(1). Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-10ssh signing: provide a textual signing_key_idFabian Stelzer
For ssh the user.signingkey can be a filename/path or even a literal ssh pubkey. In push certs and textual output we prefer the ssh fingerprint instead. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-10ssh signing: retrieve a default key from ssh-agentFabian Stelzer
If user.signingkey is not set and a ssh signature is requested we call gpg.ssh.defaultKeyCommand (typically "ssh-add -L") and use the first key we get Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>