[pacman-dev,1/2] libalpm: Add dlclientcert and dlclientkey options.

Message ID 20181114163714.7318-1-maarten@de-vri.es
State New
Headers show
Series
  • [pacman-dev,1/2] libalpm: Add dlclientcert and dlclientkey options.
Related show

Commit Message

Maarten de Vries Nov. 14, 2018, 4:37 p.m. UTC
These patches add support for client certificates to alpm and pacman.

This can already be achieved currently by setting an XferCommand,
but doing so significantly reduces the quality of the feedback pacman
gives during the downloads. Especially annoying are the 404 errors on
most database signature files, but that's not the only issue.

I admit this is a bit of an edge case, but I find myself in the
situation where I have to download packages from a private repository
that requires a valid client certificate. I really want the nice regular
pacman feedback back though, so I figured I'd hack it in myself.

I tried to follow naming schemes and other conventions the best I could,
but please let me know if I should change anything, or forgot something.

---
 lib/libalpm/alpm.h   |  6 ++++
 lib/libalpm/dload.c  |  3 ++
 lib/libalpm/handle.c | 75 ++++++++++++++++++++++++++++++++++++++++++++
 lib/libalpm/handle.h |  2 ++
 4 files changed, 86 insertions(+)

Comments

Eli Schwartz Nov. 14, 2018, 4:51 p.m. UTC | #1
On 11/14/18 11:37 AM, Maarten de Vries wrote:
> These patches add support for client certificates to alpm and pacman.
> 
> This can already be achieved currently by setting an XferCommand,
> but doing so significantly reduces the quality of the feedback pacman
> gives during the downloads. Especially annoying are the 404 errors on
> most database signature files, but that's not the only issue.
> 
> I admit this is a bit of an edge case, but I find myself in the
> situation where I have to download packages from a private repository
> that requires a valid client certificate. I really want the nice regular
> pacman feedback back though, so I figured I'd hack it in myself.
Surely this can also be achieved by adding the certificate to your
system certificate store?
Maarten de Vries Nov. 14, 2018, 5:04 p.m. UTC | #2
On Wed, 14 Nov 2018 at 17:51, Eli Schwartz <eschwartz@archlinux.org> wrote:

> On 11/14/18 11:37 AM, Maarten de Vries wrote:
> > These patches add support for client certificates to alpm and pacman.
> >
> > This can already be achieved currently by setting an XferCommand,
> > but doing so significantly reduces the quality of the feedback pacman
> > gives during the downloads. Especially annoying are the 404 errors on
> > most database signature files, but that's not the only issue.
> >
> > I admit this is a bit of an edge case, but I find myself in the
> > situation where I have to download packages from a private repository
> > that requires a valid client certificate. I really want the nice regular
> > pacman feedback back though, so I figured I'd hack it in myself.
> Surely this can also be achieved by adding the certificate to your
> system certificate store?
>
>
No, that would work if I want to verify a self signed server certificate
(or a server certificate issues by a private CA). But I need to present a
client certificate to the server.





> --
> Eli Schwartz
> Bug Wrangler and Trusted User
>
>
Maarten de Vries Nov. 21, 2018, 6:42 p.m. UTC | #3
On 14-11-2018 18:04, Maarten de Vries wrote:
>
>
> On Wed, 14 Nov 2018 at 17:51, Eli Schwartz <eschwartz@archlinux.org 
> <mailto:eschwartz@archlinux.org>> wrote:
>
>     On 11/14/18 11:37 AM, Maarten de Vries wrote:
>     > These patches add support for client certificates to alpm and
>     pacman.
>     >
>     > This can already be achieved currently by setting an XferCommand,
>     > but doing so significantly reduces the quality of the feedback
>     pacman
>     > gives during the downloads. Especially annoying are the 404
>     errors on
>     > most database signature files, but that's not the only issue.
>     >
>     > I admit this is a bit of an edge case, but I find myself in the
>     > situation where I have to download packages from a private
>     repository
>     > that requires a valid client certificate. I really want the nice
>     regular
>     > pacman feedback back though, so I figured I'd hack it in myself.
>     Surely this can also be achieved by adding the certificate to your
>     system certificate store?
>
>
> No, that would work if I want to verify a self signed server 
> certificate (or a server certificate issues by a private CA). But I 
> need to present a client certificate to the server.
>

So, any thoughts on adding support for client certificates?


Kind regards,

Maarten de Vries
Allan McRae Nov. 28, 2018, 4:08 a.m. UTC | #4
On 15/11/18 2:37 am, Maarten de Vries wrote:
> These patches add support for client certificates to alpm and pacman.
> 
> This can already be achieved currently by setting an XferCommand,
> but doing so significantly reduces the quality of the feedback pacman
> gives during the downloads. Especially annoying are the 404 errors on
> most database signature files, but that's not the only issue.
> 
> I admit this is a bit of an edge case, but I find myself in the
> situation where I have to download packages from a private repository
> that requires a valid client certificate. I really want the nice regular
> pacman feedback back though, so I figured I'd hack it in myself.
> 
> I tried to follow naming schemes and other conventions the best I could,
> but please let me know if I should change anything, or forgot something.

I am very, very reluctant to include this.  We have been quite strict on
which download options we have included in pacman in the past - it took
quite some time for DisableDownloadTimeout to be added and we still
don't have real speed limiting - although this was (still is?) due to
curl implementation limitation.  This is way too much of an edge case,
and we do have XferCommand for such things.

Note, database signature file errors can be removed by adding "SigLevel
= DatabaseNone" to the relevant databases.

Allan
Maarten de Vries Nov. 29, 2018, 6:42 p.m. UTC | #5
On 28-11-18 05:08, Allan McRae wrote:
> On 15/11/18 2:37 am, Maarten de Vries wrote:
>> These patches add support for client certificates to alpm and pacman.
>>
>> This can already be achieved currently by setting an XferCommand,
>> but doing so significantly reduces the quality of the feedback pacman
>> gives during the downloads. Especially annoying are the 404 errors on
>> most database signature files, but that's not the only issue.
>>
>> I admit this is a bit of an edge case, but I find myself in the
>> situation where I have to download packages from a private repository
>> that requires a valid client certificate. I really want the nice regular
>> pacman feedback back though, so I figured I'd hack it in myself.
>>
>> I tried to follow naming schemes and other conventions the best I could,
>> but please let me know if I should change anything, or forgot something.
> I am very, very reluctant to include this.  We have been quite strict on
> which download options we have included in pacman in the past - it took
> quite some time for DisableDownloadTimeout to be added and we still
> don't have real speed limiting - although this was (still is?) due to
> curl implementation limitation.  This is way too much of an edge case,
> and we do have XferCommand for such things.
>
> Note, database signature file errors can be removed by adding "SigLevel
> = DatabaseNone" to the relevant databases.
>
> Allan

Well, all I can say is that for this at least curl support is excellent. 
And this does make pacman useful as package manager for internal company 
repositories that need authentication.

I would love to see it in mainline pacman, but if you feel it's too much 
of an edge case, I understand. At any rate, thank you for looking at the 
patches.


-- Maarten

Patch

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 2d3d198a..b894e249 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -933,6 +933,12 @@  int alpm_option_set_remote_file_siglevel(alpm_handle_t *handle, int level);
 
 int alpm_option_set_disable_dl_timeout(alpm_handle_t *handle, unsigned short disable_dl_timeout);
 
+const char *alpm_option_get_dlclientcert(alpm_handle_t *handle);
+int alpm_option_set_dlclientcert(alpm_handle_t *handle, const char * path);
+
+const char *alpm_option_get_dlclientkey(alpm_handle_t *handle);
+int alpm_option_set_dlclientkey(alpm_handle_t *handle, const char * path);
+
 /** @} */
 
 /** @addtogroup alpm_api_databases Database Functions
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 36ae4ee1..df57e175 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -276,6 +276,9 @@  static void curl_set_handle_opts(struct dload_payload *payload,
 	curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
 	curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 
+	curl_easy_setopt(curl, CURLOPT_SSLCERT, alpm_option_get_dlclientcert(handle));
+	curl_easy_setopt(curl, CURLOPT_SSLKEY, alpm_option_get_dlclientkey(handle));
+
 	_alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
 
 	if(payload->max_size) {
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index 2213ce53..a9481837 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -88,6 +88,8 @@  void _alpm_handle_free(alpm_handle_t *handle)
 	FREE(handle->lockfile);
 	FREE(handle->arch);
 	FREE(handle->gpgdir);
+	FREE(handle->dlclientcert);
+	FREE(handle->dlclientkey);
 	FREELIST(handle->noupgrade);
 	FREELIST(handle->noextract);
 	FREELIST(handle->ignorepkg);
@@ -417,6 +419,37 @@  alpm_errno_t _alpm_set_directory_option(const char *value,
 	return 0;
 }
 
+alpm_errno_t _alpm_set_file_option(const char *value,
+		char **storage, int must_exist)
+{
+	struct stat st;
+	char real[PATH_MAX];
+	const char *path;
+
+	path = value;
+	if(!path) {
+		return ALPM_ERR_WRONG_ARGS;
+	}
+	if(must_exist) {
+		if(stat(path, &st) == -1 || !S_ISREG(st.st_mode)) {
+			return ALPM_ERR_NOT_A_FILE;
+		}
+		if(!realpath(path, real)) {
+			return ALPM_ERR_NOT_A_FILE;
+		}
+		path = real;
+	}
+
+	if(*storage) {
+		FREE(*storage);
+	}
+	*storage = strdup(path);
+	if(!*storage) {
+		return ALPM_ERR_MEMORY;
+	}
+	return 0;
+}
+
 int SYMEXPORT alpm_option_add_hookdir(alpm_handle_t *handle, const char *hookdir)
 {
 	char *newhookdir;
@@ -876,3 +909,45 @@  int SYMEXPORT alpm_option_set_disable_dl_timeout(alpm_handle_t *handle,
 #endif
 	return 0;
 }
+
+const char * SYMEXPORT alpm_option_get_dlclientcert(alpm_handle_t *handle)
+{
+	CHECK_HANDLE(handle, return NULL);
+	return handle->dlclientcert;
+}
+
+int SYMEXPORT alpm_option_set_dlclientcert(alpm_handle_t *handle, const char * path)
+{
+	int err;
+	CHECK_HANDLE(handle, return -1);
+	if(!path) {
+		FREE(handle->dlclientcert);
+		return 0;
+	}
+	if((err = _alpm_set_file_option(path, &(handle->dlclientcert), 1))) {
+		RET_ERR(handle, err, -1);
+	}
+	_alpm_log(handle, ALPM_LOG_DEBUG, "option 'dlclient_cert' = %s\n", handle->dlclientcert);
+	return 0;
+}
+
+const char * SYMEXPORT alpm_option_get_dlclientkey(alpm_handle_t *handle)
+{
+	CHECK_HANDLE(handle, return NULL);
+	return handle->dlclientkey;
+}
+
+int SYMEXPORT alpm_option_set_dlclientkey(alpm_handle_t *handle, const char * path)
+{
+	int err;
+	CHECK_HANDLE(handle, return -1);
+	if(!path) {
+		FREE(handle->dlclientkey);
+		return 0;
+	}
+	if((err = _alpm_set_file_option(path, &(handle->dlclientkey), 1))) {
+		RET_ERR(handle, err, -1);
+	}
+	_alpm_log(handle, ALPM_LOG_DEBUG, "option 'dlclient_key' = %s\n", handle->dlclientkey);
+	return 0;
+}
diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
index 44c4904b..4bcfeb8a 100644
--- a/lib/libalpm/handle.h
+++ b/lib/libalpm/handle.h
@@ -82,6 +82,8 @@  struct __alpm_handle_t {
 	char *logfile;           /* Name of the log file */
 	char *lockfile;          /* Name of the lock file */
 	char *gpgdir;            /* Directory where GnuPG files are stored */
+	char *dlclientcert;        /* Name of the client certificate */
+	char *dlclientkey;         /* Name of the client key */
 	alpm_list_t *cachedirs;  /* Paths to pacman cache directories */
 	alpm_list_t *hookdirs;   /* Paths to hook directories */
 	alpm_list_t *overwrite_files; /* Paths that may be overwritten */