[xmlsec] core methods for write of <X509SubjectName/> and <X509IssuerSerial/>
Aleksey Sanin
aleksey@aleksey.com
Wed, 23 Jul 2003 18:04:29 -0700
This is a multi-part message in MIME format.
--------------080204070904090203030009
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Hi, Roumen!
I have looked at your new patch and I have few comments:
0) It seems that xmlSecOpenSSLKeyDataX509XmlWrite() function
now writes subject, serial or full certificate only for the first
certificate
in the xmlSecKey. All other certificates are written "in-full".
This seems wrong to me. Yo don't know which certificate will be the
"first" one. I am not sure I understand why you don't want to do the
same for all certs.
I thought that the plan was:
- Read X509Data node and create a bits mask of its children
(cert, subject, serial, ski, crl).
- If mask is 0 (no children) then set cert and crl bits to
simulate
current behaiviour (write certs and crls in empty X509Data node).
- Remove X509Data node content.
- Walk thru the list of certificates and write cert and/or
subject and/or
serial and/or ski according to bits mask.
- If crls bit is set walk thru the list of crls and write them
out.
This seems more natural to me than "special case" the first cert.
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 :)
Also I am not sure I understand why you put "XXX" comments around
it. Seems
useless to me.
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.
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".
4) Which OpenSSL version do you use? I wonder if this new code works
with OpenSSL 0.9.6.
Aleksey
--------------080204070904090203030009
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=UTF-8">
<title></title>
</head>
<body text="#000000" bgcolor="#ffffff">
<meta http-equiv="Content-Type" content="text/html;charset=UTF-8">
<title></title>
Hi, Roumen!<br>
<br>
I have looked at your new patch and I have few comments:<br>
0) It seems that xmlSecOpenSSLKeyDataX509XmlWrite() function<br>
now writes subject, serial or full certificate only for the first
certificate<br>
in the xmlSecKey. All other certificates are written "in-full".<br>
This seems wrong to me. Yo don't know which certificate will be the
<br>
"first" one. I am not sure I understand why you don't want to do the<br>
same for all certs. <br>
I thought that the plan was: <br>
- Read X509Data node and create a bits mask of its children<br>
(cert, subject, serial, ski, crl).<br>
- If mask is 0 (no children) then set cert and crl bits to
simulate<br>
current behaiviour (write certs and crls in empty X509Data
node).<br>
- Remove X509Data node content.<br>
- Walk thru the list of certificates and write cert and/or
subject and/or<br>
serial and/or ski according to bits mask.<br>
- If crls bit is set walk thru the list of crls and write
them out.<br>
This seems more natural to me than "special case" the first cert.<br>
<br>
1) I don't like the way you implemented the "empty" check in
*Read() functions.<br>
IMHO, this is a bad coding style to repeat the same code again and
again.<br>
Probably a small internal static function <br>
int xmlSecOpenSSLX509IsEmpty(xmlChar*)<br>
would be better :)<br>
Also I am not sure I understand why you put "XXX" comments around
it. Seems<br>
useless to me.<br>
<br>
2) You are using the figure brackets to mark block of code all the
time<br>
(I meant the "write Issuer Name" block in the example bellow):<br>
<br>
+ if(cur == NULL) {<br>
+ cur = xmlSecAddChild(node, xmlSecNodeX509IssuerSerial,
xmlSecDSigNs);<br>
+ ....<br>
+ }<br>
+<br>
+ { /*write Issuer Name*/<br>
+ for(node_in = xmlSecGetNextElementNode(cur->children);<br>
+ ....<br>
+ }<br>
<br>
Please don't do this. It makes code difficult to read. <br>
<br>
3) In xmlSecOpenSSLX509NameWrite() function I wonder if there is a
way to <br>
print name to a buffer, not memory BIO. Mallocs might be expensive
:( But I guess<br>
the answer is "no".<br>
<br>
4) Which OpenSSL version do you use? I wonder if this new code
works with OpenSSL 0.9.6.<br>
<br>
<br>
Aleksey<br>
<br>
<br>
</body>
</html>
--------------080204070904090203030009--