|
From: Gémes G. <ge...@kz...> - 2013-05-11 14:16:25
|
Hi Branko, First of all thank you for taking time to look trough it! I've followed your suggestions, see bellow and attached version. > On Mon, 06 May 2013 16:36:44 +0200 > Gémes Géza <ge...@kz...> wrote: > >> Hi, >> >> The attached python script generates a certificate request and an >> info file to be submitted to EJBCA in order to generate an AD DC >> certificate for samba. >> It requires m2crypto and a samba (>=4.x) installation (no need for a >> provision, it can be run remotely by specifying the name of the DC on >> command line, then it asks for a username and a password). >> >> Please write your comments/suggestions/patches about it. >> >> I'm going forward to trying to code the certificate installation to >> the Samba DC. >> >> Cheers >> >> Geza Gemes > Hello Géza > > Going through the script, seems to be ok, although I have no clue about > correctness of the Samba-related code, but you know what you're doing > there I believe :) > > Take the below comments with a grain of salt, since I'm a bit nitpicky, > and this in no way means your code is bad. Not to mention it's always > easy to criticise other people's code :) > > Of course, thanks for the contribution, it's really great to get more > instructions, scripts etc that help with integration of X.509 and > EJBCA! > > Here's a couple of general remarks (most of them are PEP8 styling ones, > actually): > > - [PEP8] Break-up import os, sys, socket, argparse into separate import > lines. Done > > - [PEP8] Use 4-space indentation instead of tabs. Done > - Don't hard-code Python path detection. It's better to let the user > handle this (you could show informative message to user). New command line option added, although /usr/local/samba (as the default installation path) remains > - [PEP8] Don't use lower-case class name (the getdc). Use CamelCase. > I'd possibly name it a bit differently - I guess that one started off > as a plain function? Typo, fixed > > - Move the 'import getpass' to the top (not much reason to have it in > the middle of code). Done > > - The LDAP is hard-coded to non-encrypted (no STARTTLS/SSL) > connection. No idea how the SamDB handles it afterwards, though. Thats a catch 22 situation, you can't use SSL or StartTLS without a valid certificate. > > - If possible, you should add some explanation on the self.hexGUID=GUID > line. Six months later when you get back to it you'll be thankful for > it :) Done (http://en.wikipedia.org/wiki/Globally_unique_identifier) > > - Existing files will be overwritten (.txt, .pem etc). No idea if it's > worth it to make the code more resilient to this or not, though. Not anymore, however could be problematic only if you have sent your request to EJBCA and waiting for the cert. > > - Seeing that you're not defining any meaningful callback function > for gen_key, you can completely drop it (by default it's None). Fixed > > - RSA key length is hard-coded to 1024 bits. This might be a bit too > low as well (2048 would be a saner value). Having an option to > specify this would be a very good idea. Option added (1024 was selected based on the code at: http://download.primekey.se/ejbca/smartcardlogon/) > > - Signing algorithm hash is hard-coded to SHA1 (then again, this might > not be too big of a problem). As this could be a problem for WinXP clients around (http://blogs.msdn.com/b/alejacma/archive/2009/01/23/sha-2-support-on-windows-xp.aspx) sha1 remains the default, with option added to select other digest method > > - There's not much error handling in the script. What happens if log-in > to LDAP/domain fails? Error handling added > > - Small warning on argparse - that module is available starting > with Python 2.7. On RHEL 6 they still have 2.6 series > (unfortunately). Added failback to optparse if argparse import fails. > > Best regards > > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and > their applications. This 200-page book is written by three acclaimed > leaders in the field. The early access version is available now. > Download your free book today! http://p.sf.net/sfu/neotech_d2d_may > > > _______________________________________________ > Ejbca-develop mailing list > Ejb...@li... > https://lists.sourceforge.net/lists/listinfo/ejbca-develop As before comments/patches welcome! Cheers Geza Gemes |