Skip to content

Commit

Permalink
don't simply increment the serial
Browse files Browse the repository at this point in the history
  • Loading branch information
duritong committed Sep 13, 2015
1 parent 3d08f15 commit f32768d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 16 deletions.
26 changes: 15 additions & 11 deletions lib/trocla/formats/x509.rb
Expand Up @@ -21,7 +21,6 @@ def format(plain_password,options={})
sign_with = options['ca']
become_ca = options['become_ca'] || false
keysize = options['keysize'] || 4096
serial = options['serial'] || 1
days = options['days'].to_i || 365
if an = options['altnames']
altnames = an.collect { |v| "DNS:#{v}" }.join(', ')
Expand All @@ -40,7 +39,7 @@ def format(plain_password,options={})
begin
ca = OpenSSL::X509::Certificate.new(getca(sign_with))
cakey = OpenSSL::PKey::RSA.new(getca(sign_with))
caserial = getserial(sign_with, serial)
caserial = getserial(sign_with)
rescue Exception => e
raise "Value of #{sign_with} can't be loaded as CA: #{e.message}"
end
Expand All @@ -56,7 +55,7 @@ def format(plain_password,options={})
begin
csr_cert = mkcert(caserial, request.subject, ca, request.public_key, days, altnames, become_ca)
csr_cert.sign(cakey, signature(hash))
setserial(sign_with, caserial)
addserial(sign_with, caserial)
rescue Exception => e
raise "Certificate #{subject} signing failed: #{e.message}"
end
Expand All @@ -65,7 +64,7 @@ def format(plain_password,options={})
else # self-signed certificate
begin
subj = OpenSSL::X509::Name.parse(subject)
cert = mkcert(serial, subj, nil, key.public_key, days, altnames, become_ca)
cert = mkcert(getserial(subj), subj, nil, key.public_key, days, altnames, become_ca)
cert.sign(key, signature(hash))
rescue Exception => e
raise "Self-signed certificate #{subject} creation failed: #{e.message}"
Expand Down Expand Up @@ -140,16 +139,21 @@ def getca(ca)
trocla.get_password(ca,'x509')
end

def getserial(ca,serial)
newser = trocla.get_password("#{ca}_serial",'plain')
if newser
newser + 1
def getserial(ca)
newser = Trocla::Util.random_str(20,'numeric').to_i

This comment has been minimized.

Copy link
@asquelt

asquelt Oct 13, 2015

Contributor

RFC 5280 says serialNumber should fit in 20 octets (bytes), not 20 chars, you're limiting randomness in this security feature :)

or maybe it's a typo and should be random_str(20,'hex') instead.

all_serials(ca).include?(newser) ? getserial(ca) : newser
end

def all_serials(ca)
if allser = trocla.get_password("#{ca}_all_serials",'plain')
YAML.load(allser)
else
serial
[]
end
end

def setserial(ca,serial)
trocla.set_password("#{ca}_serial",'plain',serial)
def addserial(ca,serial)
serials = all_serials(ca) << serial
trocla.set_password("#{ca}_all_serials",'plain',YAML.dump(serials))
end
end
19 changes: 14 additions & 5 deletions spec/trocla/formats/x509_spec.rb
Expand Up @@ -58,7 +58,7 @@
ca_str = @trocla.password('my_shiny_selfsigned_ca', 'x509', ca_options)
@ca = OpenSSL::X509::Certificate.new(ca_str)
end
it 'shold be able to get a cert signed by the ca' do
it 'should be able to get a cert signed by the ca' do
cert_str = @trocla.password('mycert', 'x509', cert_options)
cert = OpenSSL::X509::Certificate.new(cert_str)
cert.issuer.should eql(@ca.subject)
Expand All @@ -69,7 +69,18 @@
ku.should_not match(/CRL Sign/)
end

it 'shold be able to get a cert signed by the ca that is again a ca' do
it 'should not simply increment the serial' do
cert_str = @trocla.password('mycert', 'x509', cert_options)
cert1 = OpenSSL::X509::Certificate.new(cert_str)
cert_str = @trocla.password('mycert2', 'x509', cert_options)
cert2 = OpenSSL::X509::Certificate.new(cert_str)

cert1.serial.to_i.should_not eql(1)
cert2.serial.to_i.should_not eql(2)
(cert2.serial - cert1.serial).to_i.should_not eql(1)
end

it 'should be able to get a cert signed by the ca that is again a ca' do
cert_str = @trocla.password('mycert', 'x509', cert_options.merge({
'become_ca' => true,
}))
Expand All @@ -82,7 +93,7 @@
ku.should match(/CRL Sign/)
end

it 'shold be able to get a cert signed by the ca that is again a ca that is able to sign certs' do
it 'should be able to get a cert signed by the ca that is again a ca that is able to sign certs' do
cert_str = @trocla.password('mycert_and_ca', 'x509', cert_options.merge({
'become_ca' => true,
}))
Expand All @@ -102,7 +113,6 @@
co = cert_options.merge({
'hash' => 'sha1',
'keysize' => 2048,
'serial' => 123456789,
'days' => 3650,
'subject' => nil,
'C' => 'AA',
Expand All @@ -121,7 +131,6 @@
cert.subject.to_s.should match(/#{field}=#{co[field]}/)
end
cert.signature_algorithm.should eql('sha1WithRSAEncryption')
cert.serial.should eql(123456789)
cert.not_before.should < Time.now
Date.parse(cert.not_after.to_s) == Date.parse((Time.now+3650*24*60*60).to_s)
# https://stackoverflow.com/questions/13747212/determine-key-size-from-public-key-pem-format
Expand Down

0 comments on commit f32768d

Please sign in to comment.