1111392 - Add tests for malformed name constraints when there are no names of the constrained type

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: more-name-constraint-tests.patch

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8536227 [details] [diff] [review]
more-name-constraint-tests.patch

Review of attachment 8536227 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Just a couple of questions.

::: security/pkix/test/gtest/pkixnames_tests.cpp
@@ +1754,5 @@
> +  // Name constraints for unsupported types of names are ignored when there are
> +  // are no names of that type to constrain.
> +  { ByteString(), NO_SAN,
> +    GeneralSubtree(DNSName("!")),
> +    Success, Success

Isn't this the same as the first new test?

@@ +1758,5 @@
> +    Success, Success
> +  },
> +
> +  /////////////////////////////////////////////////////////////////////////////
> +  // Name constraints for unsupported types of names guarantee always result in

s/guarantee // ?

@@ +1760,5 @@
> +
> +  /////////////////////////////////////////////////////////////////////////////
> +  // Name constraints for unsupported types of names guarantee always result in
> +  // failure when there are names of that type to constrain.
> +

Are there supposed to be more tests here, or is this just additional documentation for existing tests?

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #1)
> Isn't this the same as the first new test?

Yes. I removed the redundant copy. Thanks for noticing that.

> > +  /////////////////////////////////////////////////////////////////////////////
> > +  // Name constraints for unsupported types of names guarantee always result in
> 
> s/guarantee // ?

Oops! I overlooked this comment. I will push a fix for this typo.

> > +  /////////////////////////////////////////////////////////////////////////////
> > +  // Name constraints for unsupported types of names guarantee always result in
> > +  // failure when there are names of that type to constrain.
> > +
> 
> Are there supposed to be more tests here, or is this just additional
> documentation for existing tests?

I decided to split those tests into a separate patch because they aren't relevant to the RFC 822 name constraint issue like this one is. I forgot to remove that comment, but I've removed the comment. Thanks for noticing this.

https://hg.mozilla.org/integration/mozilla-inbound/rev/64e991d62eeb

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #2)
> > > +  /////////////////////////////////////////////////////////////////////////////
> > > +  // Name constraints for unsupported types of names guarantee always result in
> > 
> > s/guarantee // ?
> 
> Oops! I overlooked this comment. I will push a fix for this typo.

Actually, I'll just incorporate that typo fix into the not-yet-pushed patch that will add that comment!

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/64e991d62eeb