[pacman-dev] [PATCH] Add Keyring/--keyring option in alpm/pacman
Dan McGee
dpmcgee at gmail.com
Thu Jun 19 09:28:45 EDT 2008
On Tue, Jun 3, 2008 at 12:24 AM, Dan McGee <dpmcgee at gmail.com> wrote:
> 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.
Ping? Patches don't rewrite themselves.
-Dan
More information about the pacman-dev
mailing list