[PATCH] hs20-ca: provide command-line args to setup.sh

[Ben Greear]

On 03/27/2015 10:35 AM, Jouni Malinen wrote:
> On Fri, Mar 20, 2015 at 10:44:38PM -0400, greearb at candelatech.com wrote:
>> diff --git a/hs20/server/ca/setup.sh b/hs20/server/ca/setup.sh
>> @@ -5,6 +5,45 @@ if [ -z "$OPENSSL" ]; then
>> +# -C:  SSL Commonname for Root CA ($CNR)\n
> 
> While this happens to be used with TLS, "SSL Commonname" sound quite
> confusing.. This should be the Subject name CN of the Root CA
> certificate.
> 
>> +# -d:  DNS Name ($DNS)\n
> 
> DNS Name of what? Should be something like OSU_SERVER_HOSTNAME.
> 
>> +# -I:  SSL Commonname for Intermediate CA ($CNI)\n
>> +# -o:  SSL Commonname for OSU-Client Server ($CNOC)\n
>> +# -O:  SSL Commonname for OCSP Server ($CNO)\n
> 
> Same comment about SSL.
> 
> What is "OSU-Client Server"?? CNOC is used as the CN in the test
> certificate that has an invalid usage limitations to work as a server.

What should the 'server-client.key' (and .pem, .csr) be used for?  In the end,
I did not end up using them..but I used server.pem in several
different places, which is probably not the best idea.

>> +# -p:  password ($PASS)\n
> 
> That should be "passphrase for private keys"
> 
>> +# -S:  servername ($SERVERNAME)\n
> 
> Server name of what? This should be OSU_SERVER_HOSTNAME (i.e., only one
> parameter for this.. DNS == SERVERNAME here is a requirement since
> dNSName and subject CN must be same).

Ok, I can remove the DNS option entirely and just use the server-name.

>> +# -V:  SSL Commonname for OSU-Revoked Server ($CNV)\n
> 
> Same comment about SSL.
> 
>> +if [ $DEBUG == 1 ]
> 
> That fails on many /bin/sh (which is not required to be bash)...
> --> if [ $DEBUG = 1 ]
> 
>> +if [ ! -f openssl-root.cnf.orig ]
>> +then
>> +    cp openssl-root.cnf openssl-root.cnf.orig
>> +else
>> +    cp openssl-root.cnf.orig openssl-root.cnf
>> +fi
> 
> Why? openssl-root.cnf is the file from repository and must not be
> modified. A temporary .tmp file was used here for a reason..

I wanted to do all of the common substitutions once at the top
of the file so that I didn't have to have duplicated sed logic in
each of the steps that messes with the .tmp file.

The copy to/from orig logic lets me re-run setup.sh and get
repeatable results.

I can instead make a copy and always work from the copy instead
so that it can be run in-place in the git repo if you prefer?

> 
>> +if [ ! -f openssl.cnf.orig ]
>> +then
>> +    cp openssl.cnf openssl.cnf.orig
>> +else
>> +    cp openssl.cnf.orig openssl.cnf
>> +fi
> 
> Same as openssl-root.cnf.
> 
>> +# Set the password accordingly.
>> +cat openssl-root.cnf | sed "s/@PASSWORD@/$PASS/" > openssl-root.cnf.tmp
>> +mv openssl-root.cnf.tmp openssl-root.cnf
>> +cat openssl.cnf | sed "s/@PASSWORD@/$PASS/" > openssl.cnf.tmp
>> +mv openssl.cnf.tmp openssl.cnf
> 
> And here's the .tmp file.. Hmm.. Why not simply skip the previous step
> with .orig?
> 
>> -cat openssl.cnf | sed "s/#@CN@/commonName_default = w1.fi Hotspot 2.0 Intermediate CA/" > openssl.cnf.tmp
>> +cat openssl.cnf | sed "s/#@CN@/commonName_default = $CNI/" > openssl.cnf.tmp
> 
> This looks a bit undesired flexibility.. This certificate is required to
> use "<company> Hotspot 2.0 Intermediate CA" format for the CN. If this
> can be modified, it documentation should make it clear that changing
> this to anything else will result in an invalid certificate.

So, maybe let users specify the <company> and keep the rest hard-coded as is?

Thanks,
Ben


-- 
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc  http://www.candelatech.com