Opened 4 years ago

Closed 4 years ago

#383 closed Bug / Defect (fixed)

tls_serial_{n} is provided in decimal

Reported by: thatsmydoing Owned by:
Priority: major Milestone: release 2.3.4
Component: Generic / unclassified Version: OpenVPN 2.3.2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

In a tls-verify script, tls_serial_{n} is provided as a decimal value, as opposed to hex that the documentation states. This also breaks the provided OCSP.sh script for values above 9.

The relevant line on the cert is:
Serial Number: 20 (0x14)

I'm not sure if that is what's causing this issue

Change History (6)

comment:1 Changed 4 years ago by plaisthos

You seem to be right. The tls_serial is decimal. If you remove the 0x in the OSCP script, the OSCP script will work. I send a patch to openvpn-devel to fix documentation and script

comment:2 Changed 4 years ago by thatsmydoing

Might it not be a bug in the code itself? I checked both the openssl and polarssl backends, and polarssl seems to set the tls_serial to be in hex

http://sourceforge.net/p/openvpn/openvpn/ci/release/2.3/~/tree/src/openvpn/ssl_verify.c#l438
http://sourceforge.net/p/openvpn/openvpn/ci/release/2.3/~/tree/src/openvpn/ssl_verify_polarssl.c#l126
https://github.com/polarssl/polarssl/blob/polarssl-1.2/library/x509parse.c#L2821

It also seemed to originally really be in hex

http://sourceforge.net/p/openvpn/openvpn/ci/fa47f0a36c2aeda972a94c93f8f83246306812a0/

i2a_ASN1_INTEGER() is said to return the serial in hex

http://www.umich.edu/~x509/ssleay/asn1-ints.html

The relevant code was actually lost in this merge

http://sourceforge.net/p/openvpn/openvpn/ci/20b18fd799e2ea9d0651f3ef913dd9ce2e481471/?page=1#diff-12

In OpenVPN < 2.1 it did return a decimal, but in OpenVPN 2.1 to 2.2 it returned them as hex numbers, so I'm not sure which is "correct" behavior now

comment:3 Changed 4 years ago by plaisthos

i2a_ASN1_INTEGER returns an int which is then converted to decimal via BN_bn2dec if I read the code correct. So I think all version return decimal numbers.

comment:4 Changed 4 years ago by thatsmydoing

Well, the docs say,

i2a_ASN1_INTEGER() writes out to BIO output file bp the data element of the ASN1_INTEGER that a points to in hexadecimal notation, breaking up the integer into lines of 70 characters each and a continuation marker '\' at the end of each line, if the integer is that long.

The merge replaced that code with the one that used BN_bn2dec which might be "wrong". Before that merge, bignum wasn't even being used.

comment:5 Changed 4 years ago by cron2

  • Milestone set to release 2.3.4

Feedback on the openvpn-devel list was "decimal is the desired format", so the PolarSSL code is incorrect, as is the documentation.

comment:6 Changed 4 years ago by cron2

  • Resolution set to fixed
  • Status changed from new to closed

Agreement on mailing list and IRC that "serial is it".

Patches for documentation and OCSP.sh script have already been merged, while patches for "have PolarSSL report in decimal as well" are only in master yet, 2.3 is being worked on.

commit 959d60789b6f0bd74296600f58f626cfa9738f78 (master)
commit d160a62852408121ce8fc023a89f47c7920c16e8 (release/2.3)

commit 6ea78cbef6367590567156a20106c620fec224c9 (master)
commit 78e1175cc510a98775eb393ec6d1ebf8309019c0 (release/2.3)

commit f80a52b09eed8e5e0cad990c56ec99256d6cc2d0 (master)

closing this ticket now, as it has been fully addressed.

Note: See TracTickets for help on using tickets.