URL: https://github.com/freeipa/freeipa/pull/1813
Author: frasertweedale
Title: #1813: csrgen: fix when attribute shortname is lower case
Action: opened
PR body:
"""
OpenSSL requires attribute short names ("CN", "O", etc) to be in upper
case, otherwise it fails to add the attribute. This can be triggered when
FreeIPA has been installed with --subject-base containing a lower-case
attribute shortname (e.g.
--subject-base="o=Red Hat").
Explicitly convert the attribute type string to an OID
(ASN1_OBJECT *). If that fails, upper-case the type string and try again.
Add some tests for the required behaviour.
This PR also fixes a python3 str/bytes issue in the csrgen error handling.
Fixes: https://pagure.io/freeipa/issue/7496
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1813/head:pr1813
git checkout pr1813
From b0063612c7e667d5140442aa838ac5e673013da3 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Mon, 16 Apr 2018 15:06:16 +1000
Subject: [PATCH 1/4] py3: fix csrgen error handling
csrgen error handling marshalls an error string from libcrypto.
This is not handled correctly under python3. Fix the error
handling.
Part of: https://pagure.io/freeipa/issue/7496
---
ipaclient/csrgen_ffi.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ipaclient/csrgen_ffi.py b/ipaclient/csrgen_ffi.py
index f1c1c78211..f185ddfba6 100644
--- a/ipaclient/csrgen_ffi.py
+++ b/ipaclient/csrgen_ffi.py
@@ -182,8 +182,12 @@ def _raise_openssl_errors():
code = ERR_get_error()
while code != 0:
- msg = ERR_error_string(code, NULL)
- msgs.append(_ffi.string(msg))
+ msg = _ffi.string(ERR_error_string(code, NULL))
+ try:
+ strmsg = msg.decode('utf-8')
+ except UnicodeDecodeError:
+ strmsg = repr(msg)
+ msgs.append(strmsg)
code = ERR_get_error()
raise errors.CSRTemplateError(reason='\n'.join(msgs))
From a2b63e838e167a39489b4c58a0b065f04729a832 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Mon, 16 Apr 2018 19:16:35 +1000
Subject: [PATCH 2/4] csrgen: support initialising OpenSSL adaptor with key
object
As a convenience for using it with the test suite, update the csrgen
OpenSSLAdaptor class to support initialisation with a
python-cryptography key object, rather than reading the key from a
file.
Part of: https://pagure.io/freeipa/issue/7496
---
ipaclient/csrgen.py | 35 +++++++++++++++++++++++------------
ipaclient/plugins/cert.py | 3 ++-
2 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/ipaclient/csrgen.py b/ipaclient/csrgen.py
index 0d170b8d7a..1cf24d2273 100644
--- a/ipaclient/csrgen.py
+++ b/ipaclient/csrgen.py
@@ -402,20 +402,31 @@ def sign_csr(self, certification_request_info):
class OpenSSLAdaptor(object):
- def __init__(self, key_filename, password_filename):
- self.key_filename = key_filename
- self.password_filename = password_filename
+ def __init__(self, key=None, key_filename=None, password_filename=None):
+ """
+ Must provide either ``key_filename`` or ``key``.
- def key(self):
- with open(self.key_filename, 'rb') as key_file:
- key_bytes = key_file.read()
- password = None
- if self.password_filename is not None:
- with open(self.password_filename, 'rb') as password_file:
- password = password_file.read().strip()
+ """
+ if key_filename is not None:
+ with open(key_filename, 'rb') as key_file:
+ key_bytes = key_file.read()
- key = load_pem_private_key(key_bytes, password, default_backend())
- return key
+ password = None
+ if password_filename is not None:
+ with open(password_filename, 'rb') as password_file:
+ password = password_file.read().strip()
+
+ self._key = load_pem_private_key(
+ key_bytes, password, default_backend())
+
+ elif key is not None:
+ self._key = key
+
+ else:
+ raise ValueError("Must provide 'key' or 'key_filename'")
+
+ def key(self):
+ return self._key
def get_subject_public_key_info(self):
pubkey_info = self.key().public_key().public_bytes(
diff --git a/ipaclient/plugins/cert.py b/ipaclient/plugins/cert.py
index edc132dfae..edcf410d8e 100644
--- a/ipaclient/plugins/cert.py
+++ b/ipaclient/plugins/cert.py
@@ -117,7 +117,8 @@ def forward(self, csr=None, **options):
if database:
adaptor = csrgen.NSSAdaptor(database, password_file)
elif private_key:
- adaptor = csrgen.OpenSSLAdaptor(private_key, password_file)
+ adaptor = csrgen.OpenSSLAdaptor(
+ key_filename=private_key, password_file=password_file)
else:
raise errors.InvocationError(
message=u"One of 'database' or 'private_key' is required")
From 14605e5a9db718d83ff3d8a28634eaba30a91651 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Mon, 16 Apr 2018 19:26:31 +1000
Subject: [PATCH 3/4] csrgen: drive-by docstring
Part of: https://pagure.io/freeipa/issue/7496
---
ipaclient/csrgen_ffi.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ipaclient/csrgen_ffi.py b/ipaclient/csrgen_ffi.py
index f185ddfba6..a005bcd415 100644
--- a/ipaclient/csrgen_ffi.py
+++ b/ipaclient/csrgen_ffi.py
@@ -220,6 +220,12 @@ def _parse_dn_section(subj, dn_sk):
def build_requestinfo(config, public_key_info):
+ '''
+ Return a cffi buffer containing a DER-encoded CertificationRequestInfo.
+
+ The returned object implements the buffer protocol.
+
+ '''
reqdata = NULL
req = NULL
nconf_bio = NULL
From 847499d458ef1ec24768ff056d65fe23cc934446 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Mon, 16 Apr 2018 16:11:05 +1000
Subject: [PATCH 4/4] csrgen: fix when attribute shortname is lower case
OpenSSL requires attribute short names ("CN", "O", etc) to be in
upper case, otherwise it fails to add the attribute. This can be
triggered when FreeIPA has been installed with --subject-base
containing a lower-case attribute shortname (e.g.
--subject-base="o=Red Hat").
Explicitly convert the attribute type string to an OID
(ASN1_OBJECT *). If that fails, upper-case the type string and try
again.
Add some tests for the required behaviour.
Fixes: https://pagure.io/freeipa/issue/7496
---
ipaclient/csrgen_ffi.py | 32 ++++++++++++++---
ipatests/test_ipaclient/test_csrgen.py | 64 +++++++++++++++++++++++++++++++++-
2 files changed, 90 insertions(+), 6 deletions(-)
diff --git a/ipaclient/csrgen_ffi.py b/ipaclient/csrgen_ffi.py
index a005bcd415..0362a6c0ca 100644
--- a/ipaclient/csrgen_ffi.py
+++ b/ipaclient/csrgen_ffi.py
@@ -59,6 +59,7 @@
/* openssl/x509.h */
typedef ... ASN1_INTEGER;
typedef ... ASN1_BIT_STRING;
+typedef ... ASN1_OBJECT;
typedef ... X509;
typedef ... X509_ALGOR;
typedef ... X509_CRL;
@@ -86,11 +87,14 @@
void X509_REQ_free(X509_REQ *);
EVP_PKEY *d2i_PUBKEY_bio(BIO *bp, EVP_PKEY **a);
int X509_REQ_set_pubkey(X509_REQ *x, EVP_PKEY *pkey);
-int X509_NAME_add_entry_by_txt(X509_NAME *name, const char *field, int type,
+int X509_NAME_add_entry_by_OBJ(X509_NAME *name, const ASN1_OBJECT *obj, int type,
const unsigned char *bytes, int len, int loc,
int set);
int X509_NAME_entry_count(X509_NAME *name);
-int i2d_X509_REQ_INFO(X509_REQ_INFO *a, unsigned char **out); \
+int i2d_X509_REQ_INFO(X509_REQ_INFO *a, unsigned char **out);
+
+/* openssl/objects.h */
+ASN1_OBJECT *OBJ_txt2obj(const char *s, int no_name);
/* openssl/x509v3.h */
typedef ... X509V3_CONF_METHOD;
@@ -154,13 +158,16 @@ def sk_CONF_VALUE_value(sk, i):
X509_REQ_set_pubkey = _libcrypto.X509_REQ_set_pubkey
d2i_PUBKEY_bio = _libcrypto.d2i_PUBKEY_bio
i2d_X509_REQ_INFO = _libcrypto.i2d_X509_REQ_INFO
-X509_NAME_add_entry_by_txt = _libcrypto.X509_NAME_add_entry_by_txt
+X509_NAME_add_entry_by_OBJ = _libcrypto.X509_NAME_add_entry_by_OBJ
X509_NAME_entry_count = _libcrypto.X509_NAME_entry_count
def X509_REQ_get_subject_name(req):
return req.req_info.subject
+# openssl/objects.h
+OBJ_txt2obj = _libcrypto.OBJ_txt2obj
+
# openssl/evp.h
EVP_PKEY_free = _libcrypto.EVP_PKEY_free
@@ -209,8 +216,23 @@ def _parse_dn_section(subj, dn_sk):
mval = -1
else:
mval = 0
- if not X509_NAME_add_entry_by_txt(
- subj, rdn_type, MBSTRING_UTF8,
+
+ # convert rdn_type to an OID
+ #
+ # OpenSSL is fussy about the case of the string. For example,
+ # lower-case 'o' (for "organization name") is not recognised.
+ # Therefore, try to convert the given string into an OID. If
+ # that fails, convert it upper case and try again.
+ #
+ oid = OBJ_txt2obj(rdn_type, 0)
+ if oid == NULL:
+ oid = OBJ_txt2obj(rdn_type.upper(), 0)
+ if oid == NULL:
+ raise errors.CSRTemplateError(
+ reason='unrecognised attribute type: {}'.format(rdn_type))
+
+ if not X509_NAME_add_entry_by_OBJ(
+ subj, oid, MBSTRING_UTF8,
_ffi.cast("unsigned char *", v.value), -1, -1, mval):
_raise_openssl_errors()
diff --git a/ipatests/test_ipaclient/test_csrgen.py b/ipatests/test_ipaclient/test_csrgen.py
index d0798f836d..572955e1b7 100644
--- a/ipatests/test_ipaclient/test_csrgen.py
+++ b/ipatests/test_ipaclient/test_csrgen.py
@@ -2,15 +2,24 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
+import base64
import os
import pytest
-from ipaclient import csrgen
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography import x509
+
+from ipaclient import csrgen, csrgen_ffi
from ipalib import errors
+from ipapython.dn import AVA, DN
BASE_DIR = os.path.dirname(__file__)
CSR_DATA_DIR = os.path.join(BASE_DIR, 'data', 'test_csrgen')
+CN = x509.NameOID.COMMON_NAME
+O = x509.NameOID.ORGANIZATION_NAME
+
@pytest.fixture
def formatter():
@@ -201,6 +210,59 @@ def test_caIPAserviceCert_OpenSSL(self, generator):
expected_script = f.read()
assert script == expected_script
+ def test_works_with_lowercase_attr_type_shortname(self, generator):
+ principal = {
+ 'uid': ['testuser'],
+ 'mail': ['testu...@example.com'],
+ }
+ template_env = {
+ 'ipacertificatesubjectbase': [
+ 'o=DOMAIN.EXAMPLE.COM' # lower-case attr type shortname
+ ],
+ }
+ config = generator.csr_config(principal, template_env, 'userCert')
+
+ key = rsa.generate_private_key(
+ public_exponent=65537,
+ key_size=2048,
+ backend=default_backend(),
+ )
+ adaptor = csrgen.OpenSSLAdaptor(key=key)
+
+ reqinfo = bytes(csrgen_ffi.build_requestinfo(
+ config.encode('utf-8'), adaptor.get_subject_public_key_info()))
+ csr_der = adaptor.sign_csr(reqinfo)
+ csr = x509.load_der_x509_csr(csr_der, default_backend())
+ assert csr.subject.get_attributes_for_oid(CN) \
+ == [x509.NameAttribute(CN, u'testuser')]
+ assert csr.subject.get_attributes_for_oid(O) \
+ == [x509.NameAttribute(O, u'DOMAIN.EXAMPLE.COM')]
+
+ def test_unrecognised_attr_type_raises(self, generator):
+ principal = {
+ 'uid': ['testuser'],
+ 'mail': ['testu...@example.com'],
+ }
+ template_env = {
+ 'ipacertificatesubjectbase': [
+ 'X=DOMAIN.EXAMPLE.COM' # unrecognised attr type
+ ],
+ }
+ config = generator.csr_config(principal, template_env, 'userCert')
+
+ key = rsa.generate_private_key(
+ public_exponent=65537,
+ key_size=2048,
+ backend=default_backend(),
+ )
+ adaptor = csrgen.OpenSSLAdaptor(key=key)
+
+ with pytest.raises(
+ errors.CSRTemplateError,
+ message='unrecognised attribute type: X'):
+ csrgen_ffi.build_requestinfo(
+ config.encode('utf-8'), adaptor.get_subject_public_key_info())
+
class test_rule_handling(object):
def test_optionalAttributeMissing(self, generator):
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
