Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Get the certificate from freeIPA #81

Merged
merged 5 commits into from
Oct 30, 2014
Merged

Get the certificate from freeIPA #81

merged 5 commits into from
Oct 30, 2014

Conversation

samary
Copy link

@samary samary commented Oct 2, 2014

These commands extracts the host certificate and key and the IPA CA certificate from nssdb. This is required when using ccm-fetch over SSL with a non-kerberos apache webserver.

@hpcugentbot
Copy link

Automatic reply from Jenkins: Can I test this?

stdweird
stdweird reviewed Oct 2, 2014
View reviewed changes
aii-freeipa/src/main/perl/freeipa.pm
@@ -112,6 +112,16 @@ yum -c /tmp/aii/yum/yum.conf -y install ipa-client
--realm=$tree->{realm} \\
--server=$tree->{server} \\
|| fail "ipa-client-install failed"

mkdir -p /etc/ipa/quattor/certs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch the location of the key adn cert from the ccm configuration.

@jrha jrha added this to the 14.10 milestone Oct 2, 2014
@samary
Copy link
Author

samary commented Oct 2, 2014

@StephaneGerardVUB wrote in his documentation (http://mon.iihe.ac.be/trac/t2b/wiki/QuattorFreeIPA) :
The goal of these commands is to extract the host certificate and key and the IPA CA certificate from nssdb, and to copy them in the /etc/ipa/quattor/certs directory.

@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

test this please

3 similar comments
@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

test this please

@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

test this please

@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

test this please

@hpcugentbot
Copy link

Merged build finished. Test PASSed.

@hpcugentbot
Copy link

Test PASSed.

@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

retest this please

@hpcugentbot
Copy link

Test PASSed.

@stdweird
Copy link
Member

stdweird commented Oct 6, 2014

retest this please

@hpcugentbot
Copy link

Test PASSed.

@piojo-zz
Copy link
Member

piojo-zz commented Oct 6, 2014

@samary , but the goal is to put these certificates where CCM can use them. So the right location is given by /software/components/ccm.

@stdweird
Copy link
Member

@samary we are 2 weeks from 14.10, will you have time address the remarks?

@samary
Copy link
Author

samary commented Oct 15, 2014

Hi,

I'm currently on holiday, but I think I will have time to see with @StephaneGerardVUB how we can improve this next week. If I don't, I suggest to slip this to the next milestone. Sorry for the delay. Keep you updated.

Samir Amary added 2 commits October 24, 2014 16:10
- Check and Use certificates path defined in CCM
53d2603 
- Added 'use_ssl' in the CCM schemas
- If use_ssl is true, extract certificates from IPA
add check for absolute path in CCM certs variables
f094386
@samary
Copy link
Author

samary commented Oct 24, 2014

Hi,
We've updated our code.
A little modification must be made in the CCM schema : Add "use_ssl" boolean (default : false) to check if we need to extract certificates or not. I'll put this in another PR for CCM.

stdweird
stdweird reviewed Oct 24, 2014
View reviewed changes
aii-freeipa/src/main/perl/freeipa.pm
--realm=$tree->{realm} \\
--server=$tree->{server} \\
|| fail "ipa-client-install failed"
--domain=$tree->{domain} \\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you change the indentation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for that, I'll revert this.

@samary samary mentioned this pull request Oct 24, 2014
Closed
@stdweird
Copy link
Member

@samary i realised that this is sort of a hack and probably doesn't deserve an entry under the CCM schema.
maybe make an new entry under the aii hook called extract_x509 or so.
don't forget that there's still something else required that with each update in the ipa server, there certs are re-extracted.
a better solution would be to have NSS support in CCM and eg ncm-download (maybe under a CAF::Download?)

@piojo-zz
Copy link
Member

CAF::Download makes total sense to me.

If you think support for NSS in CCM makes sense, have a look at Mozilla::CA, and try to integrate it with LWP::UserAgent. I don't see many examples out there. I think the extract_x509 hook (or extra function here) will be easier, and will give most of the benefits of this hook.

@stdweird
Copy link
Member

@Piojo what about http://search.cpan.org/~claesjac/Crypt-NSS-0.04/lib/Net/NSS/SSL/LWPCompat.pm?

@piojo-zz
Copy link
Member

Looks interesting. Is it worth the effort of integrating it into CCM?

@stdweird
Copy link
Member

yeah, and then redo it for ncm-download? i'll have a look at what is required for CAF::Download quattor/CAF#62

@samary
Copy link
Author

samary commented Oct 27, 2014

Should we remove the code at this point and let ccm handle the key validity check and key extraction on profile download ?

@stdweird
Copy link
Member

@samary no, i think the hook can do this. but would stay away from ccm wrt the key extraction and use the aii schema.

but ccm should work with nss, so the key extraction is not needed in the first place (but that rerquires a lot more code and testing)

code optimization
47c46ce 
- add "extract_x509" boolean (default : false) to run the extraction in
the hook schema
- use a function to extract certificates
@samary
Copy link
Author

samary commented Oct 29, 2014

Following your recommendations, @StephaneGerardVUB have updated the code.
He also moved the boolean from the ccm schema to the freeipa hook schema.

Let me know if it looks good to you.

Cheers

stdweird
stdweird reviewed Oct 29, 2014
View reviewed changes
aii-freeipa/src/main/pan/quattor/aii/freeipa/schema.pan
@@ -66,5 +66,7 @@ type structure_aii_freeipa = {

"dns" : boolean = false # DNS is controlled by FreeIPA (to register the host ip)
"disable" : boolean = true # disable the host on AII removal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty newline

@stdweird
Copy link
Member

@samary minor programming style remarks. otherwise good to go imho

nice work!

@samary
Copy link
Author

samary commented Oct 29, 2014

Updated following code style requirement

@stdweird
Copy link
Member

LGTM

piojo-zz pushed a commit that referenced this pull request Oct 30, 2014
@Piojo
Merge pull request #81 from samary/freeipa_cert_fix
23dd7d4 
Get the certificate from freeIPA
@piojo-zz piojo-zz merged commit 23dd7d4 into quattor:master Oct 30, 2014
@samary samary deleted the freeipa_cert_fix branch October 30, 2014 13:14
@stdweird stdweird mentioned this pull request May 11, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@stdweird stdweird

Labels
None yet
Milestone
14.10
Development

Successfully merging this pull request may close these issues.
5 participants
@samary @hpcugentbot @stdweird @piojo-zz @jrha