Skip to content
Projects
Groups
Snippets
Help
Loading...
Sign in
Toggle navigation
D
dex
Project
Project
Details
Activity
Cycle Analytics
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Charts
Issues
0
Issues
0
List
Board
Labels
Milestones
Merge Requests
0
Merge Requests
0
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Charts
Create a new issue
Commits
Issue Boards
Open sidebar
go
dex
Commits
910d5986
Commit
910d5986
authored
Mar 21, 2017
by
Eric Chiang
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
vendor: revendor
parent
58882209
Hide whitespace changes
Inline
Side-by-side
Showing
6 changed files
with
370 additions
and
94 deletions
+370
-94
glide.lock
glide.lock
+5
-3
canonicalize.go
vendor/github.com/russellhaering/goxmldsig/canonicalize.go
+3
-37
namespace.go
...thub.com/russellhaering/goxmldsig/etreeutils/namespace.go
+252
-0
sort.go
...or/github.com/russellhaering/goxmldsig/etreeutils/sort.go
+43
-0
sign.go
vendor/github.com/russellhaering/goxmldsig/sign.go
+1
-1
validate.go
vendor/github.com/russellhaering/goxmldsig/validate.go
+66
-53
No files found.
glide.lock
View file @
910d5986
hash:
fbef1f81a0f86f519714bbe568692a92709c446ef487a3fa9875e58f86e14430
updated: 2017-03-
08T10:31:06.364335442-08
:00
hash:
4a3ac3a2a2b39357dc5de7ba296dbe20a97dd2fe13bc15a68e904e69afae2adb
updated: 2017-03-
21T09:24:08.089284929-07
:00
imports:
- name: github.com/beevik/etree
version: 4cd0dd976db869f817248477718071a28e978df0
...
...
@@ -46,7 +46,9 @@ imports:
subpackages:
- cacheobject
- name: github.com/russellhaering/goxmldsig
version: e2990269f42f6ddfea940870a0800a14acdb8c21
version: 51810e925e5fc495822fbddda8202f70a6e4a3f3
subpackages:
- etreeutils
- name: github.com/Sirupsen/logrus
version: d26492970760ca5d33129d2d799e34be5c4782eb
- name: github.com/spf13/cobra
...
...
vendor/github.com/russellhaering/goxmldsig/canonicalize.go
View file @
910d5986
...
...
@@ -5,6 +5,7 @@ import (
"strings"
"github.com/beevik/etree"
"github.com/russellhaering/goxmldsig/etreeutils"
)
// Canonicalizer is an implementation of a canonicalization algorithm.
...
...
@@ -67,41 +68,6 @@ func composeAttr(space, key string) string {
return
key
}
type
attrsByKey
[]
etree
.
Attr
func
(
a
attrsByKey
)
Len
()
int
{
return
len
(
a
)
}
func
(
a
attrsByKey
)
Swap
(
i
,
j
int
)
{
a
[
i
],
a
[
j
]
=
a
[
j
],
a
[
i
]
}
func
(
a
attrsByKey
)
Less
(
i
,
j
int
)
bool
{
// As I understand it: any "xmlns" attribute should come first, followed by any
// any "xmlns:prefix" attributes, presumably ordered by prefix. Lastly any other
// attributes in lexicographical order.
if
a
[
i
]
.
Space
==
""
&&
a
[
i
]
.
Key
==
"xmlns"
{
return
true
}
if
a
[
j
]
.
Space
==
""
&&
a
[
j
]
.
Key
==
"xmlns"
{
return
false
}
if
a
[
i
]
.
Space
==
"xmlns"
{
if
a
[
j
]
.
Space
==
"xmlns"
{
return
a
[
i
]
.
Key
<
a
[
j
]
.
Key
}
return
true
}
if
a
[
j
]
.
Space
==
"xmlns"
{
return
false
}
return
composeAttr
(
a
[
i
]
.
Space
,
a
[
i
]
.
Key
)
<
composeAttr
(
a
[
j
]
.
Space
,
a
[
j
]
.
Key
)
}
type
c14nSpace
struct
{
a
etree
.
Attr
used
bool
...
...
@@ -192,7 +158,7 @@ func excCanonicalPrep(el *etree.Element, _nsAlreadyDeclared map[string]c14nSpace
}
//Sort attributes lexicographically
sort
.
Sort
(
attrsByKey
(
el
.
Attr
))
sort
.
Sort
(
etreeutils
.
SortedAttrs
(
el
.
Attr
))
return
el
.
Copy
()
}
...
...
@@ -215,7 +181,7 @@ func canonicalPrep(el *etree.Element, seenSoFar map[string]struct{}) *etree.Elem
}
ne
:=
el
.
Copy
()
sort
.
Sort
(
attrsByKey
(
ne
.
Attr
))
sort
.
Sort
(
etreeutils
.
SortedAttrs
(
ne
.
Attr
))
if
len
(
ne
.
Attr
)
!=
0
{
for
_
,
attr
:=
range
ne
.
Attr
{
if
attr
.
Space
!=
nsSpace
{
...
...
vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go
0 → 100644
View file @
910d5986
package
etreeutils
import
(
"errors"
"fmt"
"sort"
"github.com/beevik/etree"
)
const
(
defaultPrefix
=
""
xmlnsPrefix
=
"xmlns"
xmlPrefix
=
"xml"
XMLNamespace
=
"http://www.w3.org/XML/1998/namespace"
XMLNSNamespace
=
"http://www.w3.org/2000/xmlns/"
)
var
(
DefaultNSContext
=
NSContext
{
prefixes
:
map
[
string
]
string
{
defaultPrefix
:
XMLNamespace
,
xmlPrefix
:
XMLNamespace
,
xmlnsPrefix
:
XMLNSNamespace
,
},
}
EmptyNSContext
=
NSContext
{}
ErrReservedNamespace
=
errors
.
New
(
"disallowed declaration of reserved namespace"
)
ErrInvalidDefaultNamespace
=
errors
.
New
(
"invalid default namespace declaration"
)
ErrTraversalHalted
=
errors
.
New
(
"traversal halted"
)
)
type
ErrUndeclaredNSPrefix
struct
{
Prefix
string
}
func
(
e
ErrUndeclaredNSPrefix
)
Error
()
string
{
return
fmt
.
Sprintf
(
"undeclared namespace prefix: '%s'"
,
e
.
Prefix
)
}
type
NSContext
struct
{
prefixes
map
[
string
]
string
}
func
(
ctx
NSContext
)
SubContext
(
el
*
etree
.
Element
)
(
NSContext
,
error
)
{
// The subcontext should inherit existing declared prefixes
prefixes
:=
make
(
map
[
string
]
string
,
len
(
ctx
.
prefixes
)
+
4
)
for
k
,
v
:=
range
ctx
.
prefixes
{
prefixes
[
k
]
=
v
}
// Merge new namespace declarations on top of existing ones.
for
_
,
attr
:=
range
el
.
Attr
{
if
attr
.
Space
==
xmlnsPrefix
{
// This attribute is a namespace declaration of the form "xmlns:<prefix>"
// The 'xml' namespace may only be re-declared with the name 'http://www.w3.org/XML/1998/namespace'
if
attr
.
Key
==
xmlPrefix
&&
attr
.
Value
!=
XMLNamespace
{
return
ctx
,
ErrReservedNamespace
}
// The 'xmlns' namespace may not be re-declared
if
attr
.
Key
==
xmlnsPrefix
{
return
ctx
,
ErrReservedNamespace
}
prefixes
[
attr
.
Key
]
=
attr
.
Value
}
else
if
attr
.
Space
==
defaultPrefix
&&
attr
.
Key
==
xmlnsPrefix
{
// This attribute is a default namespace declaration
// The xmlns namespace value may not be declared as the default namespace
if
attr
.
Value
==
XMLNSNamespace
{
return
ctx
,
ErrInvalidDefaultNamespace
}
prefixes
[
defaultPrefix
]
=
attr
.
Value
}
}
return
NSContext
{
prefixes
:
prefixes
},
nil
}
// LookupPrefix attempts to find a declared namespace for the specified prefix. If the prefix
// is an empty string this will be the default namespace for this context. If the prefix is
// undeclared in this context an ErrUndeclaredNSPrefix will be returned.
func
(
ctx
NSContext
)
LookupPrefix
(
prefix
string
)
(
string
,
error
)
{
if
namespace
,
ok
:=
ctx
.
prefixes
[
prefix
];
ok
{
return
namespace
,
nil
}
return
""
,
ErrUndeclaredNSPrefix
{
Prefix
:
prefix
,
}
}
func
nsTraverse
(
ctx
NSContext
,
el
*
etree
.
Element
,
handle
func
(
NSContext
,
*
etree
.
Element
)
error
)
error
{
ctx
,
err
:=
ctx
.
SubContext
(
el
)
if
err
!=
nil
{
return
err
}
err
=
handle
(
ctx
,
el
)
if
err
!=
nil
{
return
err
}
// Recursively traverse child elements.
for
_
,
child
:=
range
el
.
ChildElements
()
{
err
:=
nsTraverse
(
ctx
,
child
,
handle
)
if
err
!=
nil
{
return
err
}
}
return
nil
}
func
detachWithNamespaces
(
ctx
NSContext
,
el
*
etree
.
Element
)
(
*
etree
.
Element
,
error
)
{
ctx
,
err
:=
ctx
.
SubContext
(
el
)
if
err
!=
nil
{
return
nil
,
err
}
el
=
el
.
Copy
()
// Build a new attribute list
attrs
:=
make
([]
etree
.
Attr
,
0
,
len
(
el
.
Attr
))
// First copy over anything that isn't a namespace declaration
for
_
,
attr
:=
range
el
.
Attr
{
if
attr
.
Space
==
xmlnsPrefix
{
continue
}
if
attr
.
Space
==
defaultPrefix
&&
attr
.
Key
==
xmlnsPrefix
{
continue
}
attrs
=
append
(
attrs
,
attr
)
}
// Append all in-context namespace declarations
for
prefix
,
namespace
:=
range
ctx
.
prefixes
{
// Skip the implicit "xml" and "xmlns" prefix declarations
if
prefix
==
xmlnsPrefix
||
prefix
==
xmlPrefix
{
continue
}
// Also skip declararing the default namespace as XMLNamespace
if
prefix
==
defaultPrefix
&&
namespace
==
XMLNamespace
{
continue
}
if
prefix
!=
defaultPrefix
{
attrs
=
append
(
attrs
,
etree
.
Attr
{
Space
:
xmlnsPrefix
,
Key
:
prefix
,
Value
:
namespace
,
})
}
else
{
attrs
=
append
(
attrs
,
etree
.
Attr
{
Key
:
xmlnsPrefix
,
Value
:
namespace
,
})
}
}
sort
.
Sort
(
SortedAttrs
(
attrs
))
el
.
Attr
=
attrs
return
el
,
nil
}
// NSSelectOne conducts a depth-first search for an element with the specified namespace
// and tag. If such an element is found, a new *etree.Element is returned which is a
// copy of the found element, but with all in-context namespace declarations attached
// to the element as attributes.
func
NSSelectOne
(
el
*
etree
.
Element
,
namespace
,
tag
string
)
(
*
etree
.
Element
,
error
)
{
var
found
*
etree
.
Element
err
:=
nsTraverse
(
DefaultNSContext
,
el
,
func
(
ctx
NSContext
,
el
*
etree
.
Element
)
error
{
currentNS
,
err
:=
ctx
.
LookupPrefix
(
el
.
Space
)
if
err
!=
nil
{
return
err
}
// Base case, el is the sought after element.
if
currentNS
==
namespace
&&
el
.
Tag
==
tag
{
found
,
err
=
detachWithNamespaces
(
ctx
,
el
)
return
ErrTraversalHalted
}
return
nil
})
if
err
!=
nil
&&
err
!=
ErrTraversalHalted
{
return
nil
,
err
}
return
found
,
nil
}
// NSFindIterate conducts a depth-first traversal searching for elements with the
// specified tag in the specified namespace. For each such element, the passed
// handler function is invoked. If the handler function returns an error
// traversal is immediately halted. If the error returned by the handler is
// ErrTraversalHalted then nil will be returned by NSFindIterate. If any other
// error is returned by the handler, that error will be returned by NSFindIterate.
func
NSFindIterate
(
el
*
etree
.
Element
,
namespace
,
tag
string
,
handle
func
(
*
etree
.
Element
)
error
)
error
{
err
:=
nsTraverse
(
DefaultNSContext
,
el
,
func
(
ctx
NSContext
,
el
*
etree
.
Element
)
error
{
currentNS
,
err
:=
ctx
.
LookupPrefix
(
el
.
Space
)
if
err
!=
nil
{
return
err
}
// Base case, el is the sought after element.
if
currentNS
==
namespace
&&
el
.
Tag
==
tag
{
return
handle
(
el
)
}
return
nil
})
if
err
!=
nil
&&
err
!=
ErrTraversalHalted
{
return
err
}
return
nil
}
// NSFindOne conducts a depth-first search for the specified element. If such an element
// is found a reference to it is returned.
func
NSFindOne
(
el
*
etree
.
Element
,
namespace
,
tag
string
)
(
*
etree
.
Element
,
error
)
{
var
found
*
etree
.
Element
err
:=
NSFindIterate
(
el
,
namespace
,
tag
,
func
(
el
*
etree
.
Element
)
error
{
found
=
el
return
ErrTraversalHalted
})
if
err
!=
nil
{
return
nil
,
err
}
return
found
,
nil
}
vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go
0 → 100644
View file @
910d5986
package
etreeutils
import
"github.com/beevik/etree"
type
SortedAttrs
[]
etree
.
Attr
func
(
a
SortedAttrs
)
Len
()
int
{
return
len
(
a
)
}
func
(
a
SortedAttrs
)
Swap
(
i
,
j
int
)
{
a
[
i
],
a
[
j
]
=
a
[
j
],
a
[
i
]
}
func
(
a
SortedAttrs
)
Less
(
i
,
j
int
)
bool
{
// As I understand it: any "xmlns" attribute should come first, followed by any
// any "xmlns:prefix" attributes, presumably ordered by prefix. Lastly any other
// attributes in lexicographical order.
if
a
[
i
]
.
Space
==
defaultPrefix
&&
a
[
i
]
.
Key
==
xmlnsPrefix
{
return
true
}
if
a
[
j
]
.
Space
==
defaultPrefix
&&
a
[
j
]
.
Key
==
xmlnsPrefix
{
return
false
}
if
a
[
i
]
.
Space
==
xmlnsPrefix
{
if
a
[
j
]
.
Space
==
xmlnsPrefix
{
return
a
[
i
]
.
Key
<
a
[
j
]
.
Key
}
return
true
}
if
a
[
j
]
.
Space
==
xmlnsPrefix
{
return
false
}
if
a
[
i
]
.
Space
==
a
[
j
]
.
Space
{
return
a
[
i
]
.
Key
<
a
[
j
]
.
Key
}
return
a
[
i
]
.
Space
<
a
[
j
]
.
Space
}
vendor/github.com/russellhaering/goxmldsig/sign.go
View file @
910d5986
...
...
@@ -89,7 +89,7 @@ func (ctx *SigningContext) constructSignedInfo(el *etree.Element, enveloped bool
// /SignedInfo/Reference
reference
:=
ctx
.
createNamespacedElement
(
signedInfo
,
ReferenceTag
)
dataId
:=
el
.
SelectAttrValue
(
DefaultIdAttr
,
""
)
dataId
:=
el
.
SelectAttrValue
(
ctx
.
IdAttribute
,
""
)
if
dataId
==
""
{
return
nil
,
errors
.
New
(
"Missing data ID"
)
}
...
...
vendor/github.com/russellhaering/goxmldsig/validate.go
View file @
910d5986
...
...
@@ -5,16 +5,22 @@ import (
"crypto/rsa"
"crypto/x509"
"encoding/base64"
"encoding/pem"
"errors"
"fmt"
"regexp"
"github.com/beevik/etree"
"github.com/russellhaering/goxmldsig/etreeutils"
)
var
uriRegexp
=
regexp
.
MustCompile
(
"^#[a-zA-Z_][
\\
w.-]*$"
)
var
(
// ErrMissingSignature indicates that no enveloped signature was found referencing
// the top level element passed for signature verification.
ErrMissingSignature
=
errors
.
New
(
"Missing signature referencing the top-level element"
)
)
type
ValidationContext
struct
{
CertificateStore
X509CertificateStore
IdAttribute
string
...
...
@@ -186,21 +192,7 @@ func (ctx *ValidationContext) verifySignedInfo(signatureElement *etree.Element,
return
nil
}
func
(
ctx
*
ValidationContext
)
validateSignature
(
el
*
etree
.
Element
,
cert
*
x509
.
Certificate
)
(
*
etree
.
Element
,
error
)
{
el
=
el
.
Copy
()
// Verify the document minus the signedInfo against the 'DigestValue'
// Find the 'Signature' element
sig
:=
el
.
FindElement
(
SignatureTag
)
if
sig
==
nil
{
return
nil
,
errors
.
New
(
"Missing Signature"
)
}
if
!
inNamespace
(
sig
,
Namespace
)
{
return
nil
,
errors
.
New
(
"Signature element is in the wrong namespace"
)
}
func
(
ctx
*
ValidationContext
)
validateSignature
(
el
,
sig
*
etree
.
Element
,
cert
*
x509
.
Certificate
)
(
*
etree
.
Element
,
error
)
{
// Get the 'SignedInfo' element
signedInfo
:=
sig
.
FindElement
(
childPath
(
sig
.
Space
,
SignedInfoTag
))
if
signedInfo
==
nil
{
...
...
@@ -224,10 +216,6 @@ func (ctx *ValidationContext) validateSignature(el *etree.Element, cert *x509.Ce
return
nil
,
errors
.
New
(
"Reference is missing URI attribute"
)
}
if
!
uriRegexp
.
MatchString
(
uri
.
Value
)
{
return
nil
,
errors
.
New
(
"Invalid URI: "
+
uri
.
Value
)
}
// Get the element referenced in the 'SignedInfo'
referencedElement
:=
el
.
FindElement
(
fmt
.
Sprintf
(
"//[@%s='%s']"
,
ctx
.
IdAttribute
,
uri
.
Value
[
1
:
]))
if
referencedElement
==
nil
{
...
...
@@ -311,67 +299,83 @@ func contains(roots []*x509.Certificate, cert *x509.Certificate) bool {
return
false
}
func
(
ctx
*
ValidationContext
)
verifyCertificate
(
el
*
etree
.
Element
)
(
*
x509
.
Certificate
,
error
)
{
now
:=
ctx
.
Clock
.
Now
()
el
=
el
.
Copy
()
// findSignature searches for a Signature element referencing the passed root element.
func
(
ctx
*
ValidationContext
)
findSignature
(
el
*
etree
.
Element
)
(
*
etree
.
Element
,
error
)
{
idAttr
:=
el
.
SelectAttr
(
DefaultIdAttr
)
if
idAttr
==
nil
||
idAttr
.
Value
==
""
{
return
nil
,
errors
.
New
(
"Missing ID attribute"
)
}
signatureElements
:=
el
.
FindElements
(
"//"
+
SignatureTag
)
var
signatureElement
*
etree
.
Element
// Find the Signature element that references the whole Response element
for
_
,
e
:=
range
signatureElements
{
e2
:=
e
.
Copy
()
signedInfo
:=
e2
.
FindElement
(
childPath
(
e2
.
Space
,
SignedInfoTag
))
err
:=
etreeutils
.
NSFindIterate
(
el
,
Namespace
,
SignatureTag
,
func
(
sig
*
etree
.
Element
)
error
{
signedInfo
:=
sig
.
FindElement
(
childPath
(
sig
.
Space
,
SignedInfoTag
))
if
signedInfo
==
nil
{
return
nil
,
errors
.
New
(
"Missing SignedInfo"
)
return
errors
.
New
(
"Missing SignedInfo"
)
}
referenceElement
:=
signedInfo
.
FindElement
(
childPath
(
e2
.
Space
,
ReferenceTag
))
referenceElement
:=
signedInfo
.
FindElement
(
childPath
(
sig
.
Space
,
ReferenceTag
))
if
referenceElement
==
nil
{
return
nil
,
errors
.
New
(
"Missing Reference Element"
)
return
errors
.
New
(
"Missing Reference Element"
)
}
uriAttr
:=
referenceElement
.
SelectAttr
(
URIAttr
)
if
uriAttr
==
nil
||
uriAttr
.
Value
==
""
{
return
nil
,
errors
.
New
(
"Missing URI attribute"
)
return
errors
.
New
(
"Missing URI attribute"
)
}
if
!
uriRegexp
.
MatchString
(
uriAttr
.
Value
)
{
return
errors
.
New
(
"Invalid URI: "
+
uriAttr
.
Value
)
}
if
uriAttr
.
Value
[
1
:
]
==
idAttr
.
Value
{
signatureElement
=
e
break
signatureElement
=
sig
return
etreeutils
.
ErrTraversalHalted
}
return
nil
})
if
err
!=
nil
{
return
nil
,
err
}
if
signatureElement
==
nil
{
return
nil
,
errors
.
New
(
"Missing signature referencing the top-level element"
)
return
nil
,
ErrMissingSignature
}
// Get the x509 element from the signature
x509Element
:=
signatureElement
.
FindElement
(
"//"
+
childPath
(
signatureElement
.
Space
,
X509CertificateTag
))
if
x509Element
==
nil
{
return
nil
,
errors
.
New
(
"Missing x509 Element"
)
}
return
signatureElement
,
nil
}
x509Text
:=
"-----BEGIN CERTIFICATE-----
\n
"
+
x509Element
.
Text
()
+
"
\n
-----END CERTIFICATE-----"
block
,
_
:=
pem
.
Decode
([]
byte
(
x509Text
))
if
block
==
nil
{
return
nil
,
errors
.
New
(
"Failed to parse certificate PEM"
)
}
func
(
ctx
*
ValidationContext
)
verifyCertificate
(
signatureElement
*
etree
.
Element
)
(
*
x509
.
Certificate
,
error
)
{
now
:=
ctx
.
Clock
.
Now
()
cert
,
err
:=
x509
.
ParseCertificate
(
block
.
Bytes
)
roots
,
err
:=
ctx
.
CertificateStore
.
Certificates
(
)
if
err
!=
nil
{
return
nil
,
err
}
roots
,
err
:=
ctx
.
CertificateStore
.
Certificates
()
if
err
!=
nil
{
return
nil
,
err
var
cert
*
x509
.
Certificate
// Get the x509 element from the signature
x509Element
:=
signatureElement
.
FindElement
(
"//"
+
childPath
(
signatureElement
.
Space
,
X509CertificateTag
))
if
x509Element
==
nil
{
// Use root certificate if there is only one and it is not contained in signatureElement
if
len
(
roots
)
==
1
{
cert
=
roots
[
0
]
}
else
{
return
nil
,
errors
.
New
(
"Missing x509 Element"
)
}
}
else
{
certData
,
err
:=
base64
.
StdEncoding
.
DecodeString
(
x509Element
.
Text
())
if
err
!=
nil
{
return
nil
,
errors
.
New
(
"Failed to parse certificate"
)
}
cert
,
err
=
x509
.
ParseCertificate
(
certData
)
if
err
!=
nil
{
return
nil
,
err
}
}
// Verify that the certificate is one we trust
...
...
@@ -386,12 +390,21 @@ func (ctx *ValidationContext) verifyCertificate(el *etree.Element) (*x509.Certif
return
cert
,
nil
}
// Validate verifies that the passed element contains a valid enveloped signature
// matching a currently-valid certificate in the context's CertificateStore.
func
(
ctx
*
ValidationContext
)
Validate
(
el
*
etree
.
Element
)
(
*
etree
.
Element
,
error
)
{
cert
,
err
:=
ctx
.
verifyCertificate
(
el
)
// Make a copy of the element to avoid mutating the one we were passed.
el
=
el
.
Copy
()
sig
,
err
:=
ctx
.
findSignature
(
el
)
if
err
!=
nil
{
return
nil
,
err
}
cert
,
err
:=
ctx
.
verifyCertificate
(
sig
)
if
err
!=
nil
{
return
nil
,
err
}
return
ctx
.
validateSignature
(
el
,
cert
)
return
ctx
.
validateSignature
(
el
,
sig
,
cert
)
}
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment