On 06/18/2014 02:09 PM, Jan Cholasta wrote: ... >>>> 3) I am thinking why do we need to introduce all the ASN parsing? I am >>>> talking >>>> about _decode_krb5principalname and others. If we do not use the result >>>> anywhere, why should we include this part at all? >>> >>> To work around shortcomings of NSS/python-nss. In particular, >>> _decode_krb5principalname is used to decode KRB5PrincipalName SANs to a >>> string. >>> This is necessary because certmonger puts such SANs in certificate requests >>> it >>> generates. >> >> I know, but we do not use the values besides DNS SAN type, right? I was just >> afraid that a decode error in a decoding of an unused SAN type would cause >> the >> entire CSR processing to crash. > > If we do not allow principal name SANs, requests from certmonger will fail. If > we allow them, but ignore them, you could request a certificate for an > arbitrary unrelated principal. Thus, the only thing we can do is allow them > and > validate them, which is what the patch does and why decoding KRB5PrincipalName > is necessary.
True, we need to make sure the principal type SANs match. ack on the intent. >>>> 4) I get crash in the certmonger request: >>>> >>>> ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 >>>> -r >>>> -N "cn=`hostname`" -K host/`hostname` -D test1.example.com -D >>>> test2.example.com >>>> -E [email protected] >>>> >>>> # ipa-getcert list -i test-san-1 >>>> Number of certificates and requests being tracked: 8. >>>> Request ID 'test-san-1': >>>> status: CA_UNREACHABLE >>>> ca-error: Server failed request, will retry: -504 (HTTP response code >>>> is >>>> 500, >>>> not 200). >>>> stuck: yes >>>> >>>> >>>> HTTPD traceback >>>> [Mon Jun 16 13:06:14.528642 2014] [:error] [pid 12941] [remote ... >>>> PyAsn1Error: TagSet(Tag(tagClass=128, tagFormat=0, tagId=2)) not in >>>> asn1Spec: >>>> _GeneralName() >>> >>> What version of certmonger are you using? >> >> Latest Fedora 20, i.e. should be certmonger-0.71.2-1.fc20 (do not have access >> to my VM atm). Is this a bug in certmonger? > > No, it's bug in my code. Fixed. Works. > Also added patch 301 which fixes a bug in ldap2.get_effective_rights I was > hitting with patch 234. > > Updated patches attached. > Thanks. Functionally, the patch looks ok to me, we are close to final ack. Just couple more minor changes need to happen: 1) We decided to drop ipaVirtualOperation concept in the end and allow people reading this list. This implies following changes: 234 - drop ipaVirtualOperation from cn=request certificate with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX 300 - drop the entire patch (sorry) 2) I would like to have more confidence that no unauthorized SAN extension gets in. I know we have the + for name_type, name in subjectaltname: + if name_type not in (pkcs10.SAN_DNSNAME, loop, but I would also like to add "else" path to the SAN type validation loop, in case somebody just expands the part above, but forgot to also add validation. I want us to be explicit in this case: + for name_type, name in subjectaltname: + if name_type == pkcs10.SAN_DNSNAME: ... + elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME, + pkcs10.SAN_OTHERNAME_UPN): ... > else: > raise errors.ACIError(info=_( > "Unauthorized subject alt name '%s'.") % name) When this is fixed, we should be done. Thanks, Martin _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
