[xmlsec] Re: core methods for write of <X509SubjectName/> and <X509IssuerSerial/>

Roumen Petrov xmlsec@roumenpetrov.info
Mon, 28 Jul 2003 17:39:20 +0300


This is a multi-part message in MIME format.
--------------070209010501070500090804
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

  Hi Aleksey,

please find attached file x509-sn_is__.patch.gz with latest changes.

======================================================

> On 18 Jul 2003 Aleksey Sanin wrote:
> <SNIP>
> 1) xmlSecOpenSSLX509NameWrite() function: xmlMalloc may fail. You need
> to check that returned pointer is not NULL and return an error if it's 
> the case.

fixed

> 2) xmlSecOpenSSLASN1IntegerWrite() function: the ASN1_INTEGER_to_BN()
> may return NULL. Instead of assert you should use if() to check the 
> result. 

fixed

> Also I wonder why do you use '_xxx' variable? Why do you need '_'?

now method args are without leadnig '_'

> 2) xmlSecOpenSSLASN1IntegerWrite() function: The function returns
> xmlChar* allocated using OpenSSL function BN_bn2dec(). This is wrong!
> xmlChar* is assumed to be allocated with one of LibXML2 malloc functions
> and is freed with xmlFree. If there is a different memory callbacks 
> initialized
> in LibXML2 this code would crash.

fixed

> 3) testDSig.sh: I don't see reasons to modify existing tests. The 
> right way is to add
> new tests to the suite to test new functionality.

removed

> IMHO, the better approach would be:
> 0) At the very beggining of the xmlSecOpenSSLKeyDataX509XmlWrite()
> function you read the <X509Data/> node content and determine what do 
> you want
> to write (certs, subject names, ...) based on the content of 
> <X509Data/> node
> and the xmlSecKeyInfoCtx flags.

new method xmlSecGetX509NodeWriter

> 1) Create separate methods like:
>       xmlSecOpenSSLX509CertificateNodeWrite
>       xmlSecOpenSSLX509SubjectNameNodeWrite
>       xmlSecOpenSSLX509IssuerSerialNodeWrite
>       xmlSecOpenSSLX509SKINodeWrite

implemented

> 2) Call one these methods from the for() loop in 
> xmlSecOpenSSLKeyDataX509XmlWrite()
> for each cert in the keys data.

implemented call for selected method

> 3) Determine if you want to write CRLs 
> (XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_WRITE CRLS
> flag in the xmlSecKeyInfoCtx and call the new
>       xmlSecOpenSSLX509CRLNodeWrite
> function for each CRL in xmlSecOpenSSLX509Data if needed.

implemented


======================================================

> On 24 Jul 2003 Aleksey Sanin wrote:
> <SNIP>
>     1) I don't like the way you implemented the "empty" check in 
> *Read() functions.
>     IMHO, this is a bad coding style to repeat the same code again 
> and  again.
>     Probably a small internal static function
>         int xmlSecOpenSSLX509IsEmpty(xmlChar*)
>     would be better :)

:-) yes of course - new method xmlSecIsWhiteSpaceString

>     Also I am not sure I understand why you put "XXX" comments around 
> it. Seems
>     useless to me.

now code has real comments instead of XXX and calls for 
xmlSecIsWhiteSpaceString

>     2) You are using the figure brackets to mark block of code all the 
> time
>     (I meant the "write Issuer Name" block in the example bellow):
>      
> +   if(cur == NULL) {
> +       cur = xmlSecAddChild(node, xmlSecNodeX509IssuerSerial, 
> xmlSecDSigNs);
> +       ....
> +    }
> +
> +    { /*write Issuer Name*/
> +       for(node_in = xmlSecGetNextElementNode(cur->children);
> +       ....
> +    }
>
>     Please don't do this. It makes code difficult to read. 

now code is without "figure brackets", but I dont agree that code in 
brackets is difficult to read. See comments at end of mail

>     3) In xmlSecOpenSSLX509NameWrite() function I wonder if there is a 
> way to
>     print name to a buffer, not memory BIO. Mallocs might be expensive 
> :( But I guess
>     the answer is "no".

somebody experienced in BIO and libxml2 architecture might can write 
method BIO_xmlNode for reading/writind data direct from/to xml node 
content, but I'm not that person :-(

>     4) Which OpenSSL version do you use? I wonder if this new code 
> works with OpenSSL 0.9.6.

:-[  code is compiled with OpenSSL 0.9.6e and 0.9.7b




======================================================
Miscellaneous:

In a method we can put part of code in "figure brackets" to make source 
more readable.
Suppose we have method:
int method() {
  int A, .../*5 variables to compute A*/;
  int B, .../*variables to compute B which cannot be shared with already 
defined*/;

  /*code to compute A*/
  ......./*N lines code */
  /*code to compute B*/
  ......./*K lines code */

  return(...);
}

My opinion is to rewrite like this.

int method() {
  int A;
  int B;

  { /*code to compute A*/
    definition of  5 necessary variables to compute A
    ..../*N lines code */
  }

  { /*code to compute B*/
    definition of  variables necessary to compute B
    ..../*K lines code */
  }

  return(...);
}

Code in brackets is complete and I can find easy start/end of 
"code-block". This style has other advantages as easy to modify, low 
stack requrenment.



Best regards
Roumen Petrov

--------------070209010501070500090804
Content-Type: application/gzip;
 name="x509-sn_is__.patch.gz"
Content-Transfer-Encoding: base64
Content-Disposition: inline;
 filename="x509-sn_is__.patch.gz"

H4sICH80JT8AA3g1MDktc25faXNfXy5wYXRjaADdXHtT20gS/9t8ilm29uKHDJLxAwPJhYDh
XIBJ2c4mW7dXKmPJoIuRfZJ8gb3Nd7/umZE0elmSMeCsKwGsmenp7un+dU9rpGq1SnZ2bWu8
O5vrpm1Pdx8acntnvHPd754XarK8V5Vb1doeUeSDRuOgsb8jux9SkfdkeatSqcRS8AfvE6V5
UGsc1GuRwe/fk2q9LtXqpAK/6g3y/v0WIWXyBUiQhWNMDeeRTBbm2DFmpo1N+H9Nn92tivM4
1zV9QgzTKRTLD/fTgT6+BjEGg0tk4XTkjAaLm95M0z9bhqNf6c7dTCsVithYJmPdcqStSoF/
CIzHrh8di5jwO9QEpC/0x645mZ04D9jnq/etdLhVsZ2RY4xJRh4oadb3XHewn9fBgpZiiBWc
YIvwKVDYQuw8OKSvjzRGgDGM15GMBr+lLZIsLMku7HJOTkCtxsQYjxzdY+h5+ank4Ycq+TlM
YLlWwAb+rY+d3uh+M7QS4ueVtNK17YVuDXTLGE03Qi1hhl7LWi66L+XMKWbCGKFqAKIvroiT
/uVmKIIzwuyB6UGFa6ALa/r8quAi46y/gm1OHo9NrfPgWKOxA00JeiG+9F4HPqcU05TEFQb5
liI1SaVdkxQW4zmj1BoSIPfDyNab9VPd8nz65G5klcnNYiKI6l5OIxL1Q0E6VBi5oV0vDRPW
aDQPhuSkKRD9AivaO77qlIl5n204cijgaITH7FQY7CQSSSNzPOgpXdPRb11FFYp4Se32hp3z
Tp+UR7Zp5JCJwd/i/iZG8Vn44cghqlYYLFoP9aEYLqypazmu1394dHRqPYGVZ20D4w+d2PBD
sKzl1JdbJ5r8fg1NXlFqUouavDflWfcSbGS2cOYLJ1MyBBne0Lj3FmXYRRtzRGxyoFl1ysTS
7Xgg6tqf70CXg/lorA8cyzBvgdoYcmrHWwJkZYvsriu7ppk6VUSz1ZAUUEVLbkp7bZbiw8fS
nYVlFmWc9ztMXfW5JvnT4YRcOJIIk/9tVXB24fp4YVGT5JeBzLFtg9prRRxBfnpLep8uL6Uo
L3GJYckjNZlZRSBN3hKPt57+4HSm+r1uOjiA0q++G98ZU83STRxawBF8RvfrEgLQXH1nwsUS
FQwGGBNu7id3+vgrdkJQwI6uAHgtxL3bdDowbns2o1UQ1iiz5IXvuVgQsC83C3H55wosiPla
bh5ik71V9HDRzS+/kFi5U2J//ivvyn0XQBk8cCto7UL68OV+SgcFc4auRgxN8jOUYJoQzmnS
MhkQnvjeGEpLDreqcQ7sXvRgmF/yY4dwwcu8DkW3T4UZ8o2CCnH9U2DSiyASmc9sPhXKjvEq
iiyGRt6+TdRxF1RZVUqHcSNBTT4kJXUKAhftRbMwWZYUBZC4UZeUPRaVdiFdmRGs2VDtwuIT
507H1SDlXWj3YbrAZuIG5qkiCXnduoOwWAPdOZlBmmE6RWYHyB+LO/gBVihVulo2nR4/qFWS
rCyYGY3axhUoogiuShCAYS1gqHyIi0KOKKlDUqnANzSyagFnSqddZOkwjkJP800q51CCmMDG
sbWhll7w169jWcD0l6vLQedE7fT71/2B+o9Ov8P8SOhjD0YTHsiDjngxHdk2RgqEGkMrlSSe
f+9JLYzBzbZUV9x8ZBvYevuLtu3zJ0AHsy2AlWoB/sHa2LqbJJMpZMk2W5iJNbsnY1zVBweX
rFoA3wvrJT4dL7I83Hf86js/CR/QbAzSAtQZJemrjAe6YpkZYYkTYjYl4sgRkbOruLq6inHo
dgaJt1nX4Ox9lX8/O4bM8DSuS+9aveoMBsfnHWqAq7JZieMyHuq2ae8URiM9AnwusSYxrTnW
tBPMgIpiaEhPUg6ZZVBKvmVUX3ClXcazL+o2ivj2F3s7AwsxGihRobmAZ5auF9luoxpWNKr4
O3NaZ6bNDshI08jvJhlZs4WpuU5M4b4a48CBYTplj4wxh2HNUTAXUhomw0mfspXQ0+U6KAUL
LvgDNwPBeGBNaTioZgwH0F2MBtXM0cCaplIOADozQRz1OiaYxGQOkwyFgIgtcVNaTwgQ9s08
AljTVQLAK+g4lv+14PkSlWfHyf7lXwIf9W/qKhjZv8yBjTQjxfRBtLvJdHRrk78RztVF57du
7+xaPbs8Ph+oNEweD4/V0+veUP3c7w47uIMYlFC1MturRcCqUsiPVJVCOkyxbWFgF5MPtBiB
KHAF84pYw4AuIfvJmX7g6HTs4h3Tsw9KLoRgcFn0JSYt3d66ci+tzTNISkwlX0dJYSbz6mh5
NhmnLrLxCUSBxXWWMfDSB2Hpg+DyvLyIe6D2XpPehGi0JKXOt0ApyiEB3YjbX8+MopXV4pjx
WvKA4fNxv9ftnR+gdsvkDW9/Q+y72WKqgbFBVNWJfj93HiXW52bhkG8GJOLw25lxYcgfujUj
3wBiRiaOmS+wUordj0KZ4jvcxh+52f074oxuycgmDkwxheYdOgohytdSQI1M1LgdbuzdGU9i
rqOYXS5dgGadLkB7T6q3EuvAYhWqkql+Fbirs0KtyS1ReKUjr8ATUyr22+IqxlTsYOElta6c
1MtnMtyX9c6cj9He69yVey4Q3pdnwEYcSWXhf2fcNleyoV0q2AWwzosM1NrXsSd1FRNKu56g
GCGLyqEDYadZSY9GCXvNSmiHtkx7K0WK6MhwsEioHsbHi2WduQTRBLESqJfLbi08cFduK+o3
Mcdxku6i50ekQkz5e+DMLEoDS7X0C6vptts0qilys/nMYc1mIieGNd6+7rAmaPp5wxpV6plh
arRu66lZIlwwiWOw+DN4/CIu9rHQp8h1md0OVvbdAmz+4Bd3z+spwY8cD4f97odPsJ351Ps0
6JxuTDRMjVnhYxNF93TAc0em8MSbFZuS76j+6LFJkCx7bHqegEBi4kHcQcT1BYQc8YAGgjYi
TastKX6a7cNf3jMJSWYTiQBDgOMjXxkMsXkw0B8M28kZBYQgIKp3hSgQigCYctxBnmxYtkMM
Gzj4z8KwdI0uBwlK4NWdsWIU0MGff5KfHvKcMkg658Cpr4UYSfVmkr084VYmaHWqlG1olEt+
C1KR92HjLYNl1uR9qdYUUpW8BJNSmGoG+/aPzByS9JzHECZNSHv8LuvOfJ5o9MHUJxJHsuom
h9+L5/42zfO536N08S4vMr8WpxcJrsHtU8hthOO744B99+QBZLyKtI9eX9/HY4DU692IK7jX
C7s002aKU7NOfyW39vco/jnDZ7ceZgYtdh52T2lJ+/KKG6DYA3ebsgMKteH0Kqzua+2QQkfC
X2yDFJp3s/ZHS457/ugbpG4A1/JW79i9S0aD4OK5xTffjEPqzZOZ+rr1yP2g+l3H/pPrIH0P
uqz+EX7EItW/Q4Im3bp0tZxwni22/PG0c2KBc85LjJOJTJjMq5jnkgzqr2CgongvZqI5yyTC
c4mvUx2pyQ2ahtRr9WC5PJuKL7qJm8/0+vlXI7l2/tVYe938ovtK9fJomRzEy1Err9XbbI1a
zSfUyoXnI541QyTlTayRu0/PvUja5062Wble9KmaTUrxAifd8qA8g6BNq38Lz1k/A7AzP+On
TFwkb7coSjQabanO6goUspYfHxFPegXO0L36yRFr+mMdHFnlLO/zbj5jTuduEBzFHdH98eGI
H73NDkd/tYMimR6azPHOBe9y8BHKQIabJREOPeHDu4+dBw6fzSZLhNvNlqS06uEsiz1JHsiz
XJyLgVW/ykQx1XsLQiRVArJBiPzQvSble/0+eHU6M2/ZueeEB7PvfWTjzxDyfowUUFXBpIv4
21bhWrEkeBvtI3gb4Z9UpyPCh/ueeGmbT7sdup7mghEalOVtl2P3w1fGlTeAQsaE+KpX52Dp
jqo/oKQSrINEZIl86dHT7Gr/7KRWa+yVyNFbeb3iRxl4iiaW4bHbB5U1ARdVR9MpCiu2LVMX
HTdd2HdsEI2IbPM1BkqENpG/U1xhA/hRfhwHVq8hDgrTMasG/V3B6Nm4SHtXiCLYHO3yRIQX
X8mhOjP1Q2+NES5OlYVlOhRUYmHWR20NxJTcF2m4qvknfv8XdH3zu/zmMLAEMUsXwJ8Awi7B
n9gTQcJbQwQcStx6BXEk6fn5ENLd6o7Kj4epJr3zhdOVcrIeLtW/FOfsLtcKjIffFxP3tpgs
2H/e+3QF+2eTXxjTfnP+LR76kXaS6DdYB4x6CR3j9+UeefP0Gt+zOuQy35uj4/XUG7Om6WOQ
xBeLFOdPFsujvE546VFv57wmCEeNItDRW9XAzgngmhsiGUHueGncwNw1rIORu9F/daIZk4lu
QUqH6cDMeqSofjMafwVwMnaw7sVjxLc73eSvPbAJxe6Ro2tYNXPp87Bwg5sjHZqwCuc2emTu
6SPTEq3c0Q4uR6zBnc9NU71wgX4Baam2mBfna44VAeprXMfrj50eiM7WaJ6yltHOrvEGlnN1
yI/cBFk7ckaQzod9YfL8sO/V5+I5TkBMLL4YpqY/8O+Umc6XIai5e90jZdgu8RYKTdcnw85Q
HQz73d45KeNWQctUhQyqhU4IfHiSwyzqzaPa657yZ0XgLy8KwiyqoYHvGRMDb7NUFRGcGC3v
WboVDFt4RsErifPQjzsjb2YCXOqmbczMZ8dkFqzwJ25Xg4riGqJyi4qgPZ/o4+I8LxR4qA1x
EX/dQ9NTtZpRZGdLPOF4rzWI58+xRgGv1Ih3MIiifKfqwINvEIgVnDrmGM/ZcLqcIoYVTlEi
waapbt46d16jd0wtnTHf1KKgGXlxXVyZNvTmuuCL69wKAsIM0OeVgnZ9X6qTSq2u7Lu3zAQG
lt2OiX82UnjnHLE90JvMLFI8JGX7kNiVCjMaPB0HOSoOLwJ0OKVy9M1YRMb1in35leJp5mfc
s00wb+CGARaB6qDn/OgrsA1zPF1o+i5wZuvjXVC4YU5mO3dJL8JWEl6EnUQn8Dps5UCuHTTq
sa/DVvZYWXvPe0qVvgjpZ02fGKYe/3x6p3cCF9jT6YPh9Uf1usf9QT3tnPR/+zjE6CA/4FQK
/MfV2i2X0ZLK8RSTnng/oEPoj+6E4EPzmDlhNdnBVMq/Gem/yImDtY2GMaMjjzTbuD1wK1Nk
9x3R2Wk1TJQwSVoqbBJrhQITsIYCVuibDPHJLZgvXDc7oFffLyD0IQMHhQKeBp/PgEGIG86M
YAtLC4tsHWmayf6sjq3HOfTZ+j/F/2C9O10AAA==
--------------070209010501070500090804--