FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 06-30-2012, 01:16 PM
Florian Pritz
 
Default signing.c: warn if time went backwards

GPG signatures have a timestamp which is checked and if it's in the
future, verification will fail.

Signed-off-by: Florian Pritz <bluewind@xinu.at>
---
Allan suggested to improve the error message printed when verification
fails, but since I already have this patch I can just as well submit it.

lib/libalpm/signing.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 1e41716..6148976 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -20,6 +20,8 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
+#include <time.h>
+#include <errno.h>

#if HAVE_LIBGPGME
#include <locale.h> /* setlocale() */
@@ -127,6 +129,10 @@ static int init_gpgme(alpm_handle_t *handle)
gpgme_error_t err;
gpgme_engine_info_t enginfo;

+ const static char *timestamp_file = "timestamp";
+ int len;
+ char *timestamp_path;
+
if(init) {
/* we already successfully initialized the library */
return 0;
@@ -143,6 +149,34 @@ static int init_gpgme(alpm_handle_t *handle)
"pacman-key --init");
}

+ len = strlen(sigdir) + strlen(timestamp_file) + 1;
+ CALLOC(timestamp_path, len, sizeof(char), RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+ snprintf(timestamp_path, len, "%s%s", sigdir, timestamp_file);
+
+ /* check if time went backwards since last run */
+ if(!_alpm_access(handle, sigdir, timestamp_file, F_OK)) {
+ struct stat sb;
+ time_t t;
+
+ if(stat(timestamp_path, &sb) == -1) {
+ /* TODO: should we really error here? */
+ RET_ERR(handle, ALPM_ERR_SYSTEM, -1);
+ }
+
+ t = time(NULL);
+ if(t == ((time_t)-1)) {
+ _alpm_log(handle, ALPM_LOG_ERROR, _("Failed to determine system time: %s
"), strerror(errno));
+ }
+
+ if(sb.st_mtime > t) {
+ _alpm_log(handle, ALPM_LOG_WARNING, _("Time went backwards, signature verification might fail.
"));
+ }
+ }
+ /* create file if missing, else update timestamp */
+ close(creat(timestamp_path, O_RDWR));
+
+ free(timestamp_path);
+
/* calling gpgme_check_version() returns the current version and runs
* some internal library setup code */
version = gpgme_check_version(NULL);
--
1.7.11.1
 
Old 07-01-2012, 05:23 PM
Dan McGee
 
Default signing.c: warn if time went backwards

On Sat, Jun 30, 2012 at 8:16 AM, Florian Pritz <bluewind@xinu.at> wrote:
> GPG signatures have a timestamp which is checked and if it's in the
> future, verification will fail.
>
> Signed-off-by: Florian Pritz <bluewind@xinu.at>
> ---
> Allan suggested to improve the error message printed when verification
> fails, but since I already have this patch I can just as well submit it.
>
> lib/libalpm/signing.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
> index 1e41716..6148976 100644
> --- a/lib/libalpm/signing.c
> +++ b/lib/libalpm/signing.c
> @@ -20,6 +20,8 @@
> #include <stdlib.h>
> #include <stdio.h>
> #include <string.h>
> +#include <time.h>
> +#include <errno.h>
>
> #if HAVE_LIBGPGME
> #include <locale.h> /* setlocale() */
> @@ -127,6 +129,10 @@ static int init_gpgme(alpm_handle_t *handle)
> gpgme_error_t err;
> gpgme_engine_info_t enginfo;
>
> + const static char *timestamp_file = "timestamp";
> + int len;
size_t, and why the blank line between these and the rest of the variables?
> + char *timestamp_path;
> +
> if(init) {
> /* we already successfully initialized the library */
> return 0;
> @@ -143,6 +149,34 @@ static int init_gpgme(alpm_handle_t *handle)
> "pacman-key --init");
> }
>
> + len = strlen(sigdir) + strlen(timestamp_file) + 1;
> + CALLOC(timestamp_path, len, sizeof(char), RET_ERR(handle, ALPM_ERR_MEMORY, -1));
> + snprintf(timestamp_path, len, "%s%s", sigdir, timestamp_file);

Right now this code only runs as root, but this all breaks down if we
ever add standalone verification capability or anything- chances of
being able to write to sigdir are slim to none.

> +
> + /* check if time went backwards since last run */
> + if(!_alpm_access(handle, sigdir, timestamp_file, F_OK)) {
> + struct stat sb;
> + time_t t;
> +
> + if(stat(timestamp_path, &sb) == -1) {
> + /* TODO: should we really error here? */
> + RET_ERR(handle, ALPM_ERR_SYSTEM, -1);
> + }
> +
> + t = time(NULL);
> + if(t == ((time_t)-1)) {
No need for extra parenthesis here:
if(t == (time_t)-1) {

> + _alpm_log(handle, ALPM_LOG_ERROR, _("Failed to determine system time: %s
"), strerror(errno));
> + }
I do like being careful about errors, but given that we don't worry
about time() or gettimeofday() failing anywhere else, we probably can
skip this here as well. The reasons it can fail are pretty much not
possible.
> +
> + if(sb.st_mtime > t) {
> + _alpm_log(handle, ALPM_LOG_WARNING, _("Time went backwards, signature verification might fail.
"));
> + }
> + }
> + /* create file if missing, else update timestamp */
> + close(creat(timestamp_path, O_RDWR));

DESCRIPTION
This interface is made obsolete by: open(2).
The creat() function is the same as:
open(path, O_CREAT | O_TRUNC | O_WRONLY, mode);

And with all the error checking elsewhere, the chance of open()
failing is about 100x greater than the chance of anything else, and
yet we do no error checking here.

> +
> + free(timestamp_path);
> +
> /* calling gpgme_check_version() returns the current version and runs
> * some internal library setup code */
> version = gpgme_check_version(NULL);
> --

With all this said, looking at the problem a different way- what
actually happens as far as return codes go from GPGME when the
signature time is in the past? Wouldn't it make sense to just add a
bit of checking in that failure block to print a message if the
returned signature generation time is > current system time, rather
than writing all this stuff to a file?

-Dan
 
Old 07-09-2012, 08:31 AM
Florian Pritz
 
Default signing.c: warn if time went backwards

GPG signatures have a timestamp which is checked and if it's in the
future, verification will fail.

Signed-off-by: Florian Pritz <bluewind@xinu.at>
---
Way simpler than the last version, but I'm not sure if this is the
appropriate place or if we should use the status variable to tell the
front end about the failure and handle it there.

lib/libalpm/signing.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 1e41716..f39e037 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -530,6 +530,10 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
string_validity(gpgsig->validity),
gpgme_strerror(gpgsig->validity_reason));

+ if(gpgsig->timestamp > time(NULL)) {
+ _alpm_log(handle, ALPM_LOG_WARNING, _("System time is behind signature timestamp. Verification will fail.
"));
+ }
+
result = siglist->results + sigcount;
err = gpgme_get_key(ctx, gpgsig->fpr, &key, 0);
if(gpg_err_code(err) == GPG_ERR_EOF) {
--
1.7.11.1
 

Thread Tools




All times are GMT. The time now is 05:18 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org