[pacman-dev] [PATCH] Add Keyring/--keyring option in alpm/pacman
Dan McGee
dpmcgee at gmail.com
Tue Jun 3 01:24:08 EDT 2008
On Mon, Jun 2, 2008 at 6:25 PM, <geoffroy.carrier at koon.fr> wrote:
> From: Geoffroy Carrier <geoffroy.carrier at koon.fr>
>
> Keyring in pacman.conf, --keyring in pacman,
> support for this option in libalpm and pacman.
>
> Documentation in:
> . pacman.conf(5)
> . pacman(8)
> . README
Wow, README hasn't been touched by the rest of us in forever, well
done. We really need to update this sometime, any takers?
> ---
> It is very similar to LogFile...
>
> README | 1 +
> doc/pacman.8.txt | 3 +++
> doc/pacman.conf.5.txt | 6 +++++-
> etc/pacman.conf.in | 1 +
> lib/libalpm/alpm.h | 3 +++
> lib/libalpm/handle.c | 33 +++++++++++++++++++++++++++++++++
> lib/libalpm/handle.h | 1 +
> src/pacman/Makefile.am | 2 ++
> src/pacman/conf.c | 2 ++
> src/pacman/conf.h | 1 +
> src/pacman/pacman.c | 5 +++++
> 11 files changed, 57 insertions(+), 1 deletions(-)
>
> diff --git a/README b/README
> index 46b5bb2..251862c 100644
> --- a/README
> +++ b/README
> @@ -58,6 +58,7 @@ library is initialized.
> * dbpath: The base path to pacman's databases (Default: var/lib/pacman)
> * cachedir: The base path to pacman's download cache (Default: var/cache/pacman)
> * logfile: The base path to pacman's log file (Default: var/log/pacman.log)
> +* keyring: The base path to pacman's GnuPG keyring (Default: etc/pacman.d/keyring.gpg)
> * usesyslog: Log to syslog instead of `logfile` for file-base logging.
> * xfercommand: The command to use for downloading instead of pacman's internal
> downloading functionality.
> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> index a6bc3d9..33ac7b5 100644
> --- a/doc/pacman.8.txt
> +++ b/doc/pacman.8.txt
> @@ -129,6 +129,9 @@ Options
> *\--config* <'file'>::
> Specify an alternate configuration file.
>
> +*\--keyring* <'file'>::
> + Specify an alternate GnuPG keyring.
> +
> *\--logfile* <'file'>::
> Specify an alternate log file. This is an absolute path, regardless of
> the installation root setting.
> diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> index eb9285c..44417be 100644
> --- a/doc/pacman.conf.5.txt
> +++ b/doc/pacman.conf.5.txt
> @@ -49,7 +49,7 @@ Options
> Set the default root directory for pacman to install to. This option is
> used if you want to install a package on a temporary mounted partition
> which is "owned" by another system, or for a chroot install.
> - *NOTE*: If database path or logfile are not specified on either the
> + *NOTE*: If database path, logfile or keyring are not specified on either the
> command line or in linkman:pacman.conf[5], their default location will
> be inside this root path.
>
> @@ -68,6 +68,10 @@ Options
> path, the root path is not automatically prepended. This behavior changed
> in pacman 3.1.0.
>
> +*Keyring =* '/path/to/file'::
> + Overrides the default location of the GnuPG keyring file. A typical default
> + is ``/etc/pacman.d/keyring.gpg''. If this file does not exist, packages
> + signatures will not be checked.
If this file does not exist or this option is not specified? Or what
will the behavior be in that case?
I feel like if someone takes the time to specify the option, then we
should check/fail regardless if the file exists. However, if the
option is not specified, we then don't check sigs. Does anyone else
have thoughts on this?
>
> *LogFile =* '/path/to/file'::
> Log actions directly to a file. A typical default is
> diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in
> index 582fe94..9d20a6e 100644
> --- a/etc/pacman.conf.in
> +++ b/etc/pacman.conf.in
> @@ -13,6 +13,7 @@
> #DBPath = @localstatedir@/lib/pacman/
> #CacheDir = @localstatedir@/cache/pacman/pkg/
> #LogFile = @localstatedir@/log/pacman.log
> +#Keyring = @sysconfdir@/pacman.d/keyring.gpg
Good use of autoconf vars here, thanks.
> HoldPkg = pacman glibc
> #XferCommand = /usr/bin/wget --passive-ftp -c -O %o %u
>
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 62a517b..ab60a4b 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -110,6 +110,9 @@ int alpm_option_set_logfile(const char *logfile);
> const char *alpm_option_get_lockfile();
> /* no set_lockfile, path is determined from dbpath */
>
> +const char *alpm_option_get_keyring();
> +int alpm_option_set_keyring(const char *keyring);
> +
> unsigned short alpm_option_get_usesyslog();
> void alpm_option_set_usesyslog(unsigned short usesyslog);
>
> diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
> index c01dd55..c7f2329 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -57,6 +57,7 @@ pmhandle_t *_alpm_handle_new()
> handle->cachedirs = NULL;
> handle->lockfile = NULL;
> handle->logfile = NULL;
> + handle->keyring = NULL;
> handle->usedelta = 0;
A bit OT, but we could get away with using CALLOC here so we don't
have to manually set all these to NULL.
>
> return(handle);
> @@ -86,6 +87,7 @@ void _alpm_handle_free(pmhandle_t *handle)
> FREE(handle->dbpath);
> FREELIST(handle->cachedirs);
> FREE(handle->logfile);
> + FREE(handle->keyring);
> FREE(handle->lockfile);
> FREE(handle->xfercommand);
> FREELIST(handle->dbs_sync);
> @@ -151,6 +153,15 @@ const char SYMEXPORT *alpm_option_get_logfile()
> return handle->logfile;
> }
>
> +const char SYMEXPORT *alpm_option_get_keyring()
> +{
> + if (handle == NULL) {
> + pm_errno = PM_ERR_HANDLE_NULL;
> + return NULL;
> + }
> + return handle->keyring;
> +}
> +
> const char SYMEXPORT *alpm_option_get_lockfile()
> {
> if (handle == NULL) {
> @@ -427,6 +438,28 @@ int SYMEXPORT alpm_option_set_logfile(const char *logfile)
> return(0);
> }
>
> +int SYMEXPORT alpm_option_set_keyring(const char *keyring)
> +{
> + char *oldkeyring = handle->keyring;
> +
> + ALPM_LOG_FUNC;
> +
> + if(!keyring) {
> + pm_errno = PM_ERR_WRONG_ARGS;
> + return(-1);
> + }
> +
> + handle->keyring = strdup(keyring);
Use the STRDUP macro here instead as it does some return/error checking?
> +
> + /* free the old keyring path string, and close the stream so logaction
> + * will reopen a new stream on the new logfile */
You didn't really update this comment as it still refers to logfile,
and we don't do any sort of delayed opening yet as far as I can tell.
I'd just delete this comment completely.
> + if(oldkeyring) {
> + FREE(oldkeyring);
> + }
> + _alpm_log(PM_LOG_DEBUG, "option 'keyring' = %s\n", handle->keyring);
> + return(0);
> +}
> +
> void SYMEXPORT alpm_option_set_usesyslog(unsigned short usesyslog)
> {
> handle->usesyslog = usesyslog;
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index 9c537b1..2255f43 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -45,6 +45,7 @@ typedef struct _pmhandle_t {
> char *dbpath; /* Base path to pacman's DBs */
> char *logfile; /* Name of the log file */
> char *lockfile; /* Name of the lock file */
> + char *keyring; /* Name of the GnuPG keyring */
> alpm_list_t *cachedirs; /* Paths to pacman cache directories */
>
> /* package lists */
> diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
> index 5d6fef3..16c0332 100644
> --- a/src/pacman/Makefile.am
> +++ b/src/pacman/Makefile.am
> @@ -3,6 +3,7 @@ conffile = ${sysconfdir}/pacman.conf
> dbpath = ${localstatedir}/lib/pacman/
> cachedir = ${localstatedir}/cache/pacman/pkg/
> logfile = ${localstatedir}/log/pacman.log
> +keyring = ${sysconfdir}/pacman.d/keyring.gpg
>
> bin_PROGRAMS = pacman
>
> @@ -16,6 +17,7 @@ DEFS = -DLOCALEDIR=\"@localedir@\" \
> -DDBPATH=\"$(dbpath)\" \
> -DCACHEDIR=\"$(cachedir)\" \
> -DLOGFILE=\"$(logfile)\" \
> + -DKEYRING=\"$(keyring)\" \
> @DEFS@
> INCLUDES = -I$(top_srcdir)/lib/libalpm
>
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index 48c927b..ec9a370 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -47,6 +47,7 @@ config_t *config_new(void)
> newconfig->rootdir = NULL;
> newconfig->dbpath = NULL;
> newconfig->logfile = NULL;
> + newconfig->keyring = NULL;
> newconfig->syncfirst = NULL;
>
> return(newconfig);
> @@ -63,6 +64,7 @@ int config_free(config_t *oldconfig)
> free(oldconfig->rootdir);
> free(oldconfig->dbpath);
> free(oldconfig->logfile);
> + free(oldconfig->keyring);
> free(oldconfig);
> oldconfig = NULL;
>
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index 874ce70..8be6414 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -37,6 +37,7 @@ typedef struct __config_t {
> char *rootdir;
> char *dbpath;
> char *logfile;
> + char *keyring;
> /* TODO how to handle cachedirs? */
>
> unsigned short op_q_isfile;
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 66fafa1..98bfa5f 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -131,6 +131,7 @@ static void usage(int op, const char * const myname)
> }
> printf(_(" --config <path> set an alternate configuration file\n"));
> printf(_(" --logfile <path> set an alternate log file\n"));
> + printf(_(" --keyring <path> set an alternate keyring\n"));
You added the option here, but you are also going to need to add it to
the opts array defined around line 300 of pacman.c as well as adding
it to the switch/case statement around line 360 (it will probably end
up being option 1013). The code will probably be similar to
1009/logfile or b/dpath.
> printf(_(" --noconfirm do not ask for any confirmation\n"));
> printf(_(" --noprogressbar do not show a progress bar when downloading files\n"));
> printf(_(" --noscriptlet do not execute the install scriptlet if one exists\n"));
> @@ -262,6 +263,10 @@ static void setlibpaths(void)
> snprintf(path, PATH_MAX, "%s%s", alpm_option_get_root(), LOGFILE);
> config->logfile = strdup(path);
> }
> + if(!config->keyring) {
> + snprintf(path, PATH_MAX, "%s%s", alpm_option_get_root(), KEYRING);
> + config->keyring = strdup(path);
> + }
> }
> /* Set other paths if they were configured. Note that unless rootdir
> * was left undefined, these two paths (dbpath and logfile) will have
Comment update probably needed above, as well as a code update below
(in code we can't quite see here) for setting keyring the same way we
do logfile.
> --
> 1.5.5.3
-Dan
More information about the pacman-dev
mailing list