ISC DHCP new parser is unable to parse LITERAL with escaped quotes

Support
1

Problem:

isc dhcp proxy does not start

E, [2018-04-24T17:58:57.996180 ] ERROR -- : Couldn't enable 'dhcp_isc':
in /var/lib/dhcp/dhcpd.leases:9285 at 22, expect token [ include_keyword ]
  uid "\001X\357h}g\"";

Expected outcome:

isc dhcp proxy starts

Foreman and Proxy versions:

1.16.0

Foreman and Proxy plugin versions:

1.16.0

Other relevant data:

LITERAL = /".*?"|'.*?'/.r

should be

LITERAL = /"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'/.r

Tested OK at http://rubular.com/ and in updated /usr/share/foreman-proxy/modules/dhcp_common/isc/configuration_parser.rb

2

This has been fixed in 1.17. Please see https://github.com/theforeman/smart-proxy/pull/571, although a bit differently.

The fix you are proposing breaks the parser when it expects a list of comma-separated literals.

3

Please also see Bug #23031: ISC DHCP new parser is still unable to parse DUID - Smart Proxy - Foreman and update the ticket if you are still experiencing this problem.

4

I like lzap’s shorter regexp in https://github.com/theforeman/smart-proxy/pull/571 better than mine.

The fix you are proposing breaks the parser when it expects a list of comma-separated literals.

Are you sure about that? See http://rubular.com/r/9m9GRrNaTP (uses lzap’s regexp).

5

Are you sure about that? See http://rubular.com/r/9m9GRrNaTP (uses lzap’s regexp).

I haven’t checked lzap’s version, as I opted for a different fix.

6

Fair enough. For completeness’ sake I did find a bug in lzap’s regexp so it does indeed suffer the problem you point out. \" should have been \\". The rubular link I posted has \\" and does not have that problem.

Unfortunately it doesn’t fix my problem.

7

I’ve gone back to my original regexp which I got from https://stackoverflow.com/questions/249791/regex-for-quoted-string-with-escaping-quotes

8

Sorry to dig up an old topic.

Is there a work-around for this in 1.17.x? We’re not connected to the Internet, and my mirror isn’t updated w/ 1.18.0.

This particular foreman instance was installed at 1.17.0, and is currently on 1.17.1. We’ve done dhcp provisioning on this system before, and the leases for those systems are still configured in dhcp. I’m not sure whether the bug was introduced in 1.17.1 or if something else changed. Any advice would be appreciated, thanks!

9

Hey Sean! I created backport request for you, can you go ahead and apply the patch on your instance to confirm it solves the issue? Since Mark reports it does not work for him, I just want to make sure this solves the issue for you.

10

@tbrisker hey I know that we might not be doing any 1.17.x version anymore, but I want to mark the bug Bug #23031: ISC DHCP new parser is still unable to parse DUID - Smart Proxy - Foreman into possible 1.17.x release, how do I do that? In case there is a serious security issue found before we release 1.19 we might be doing it.

I am wondering if we should always create N+1 version in RM for such an issues.

11

In general, our support policy is that only critical bug fixes are ported back to the version before latest, and considering that 1.19 is expected in under 2 weeks and 1.17.3 was released this week, I highly doubt we’ll do a 1.17.4 version before the 1.17 line is completely unsupported. Specifically in this case, @Sean, I think the best approach would be to try to manually patch your proxy with the code @lzap provided - since if i understand correctly, in any case your mirror won’t get a new version even if it is released with the fix.

Regarding the question of how to get fixes into stable releases - I think the best way is to open cherry-picks to the stable branches like you did and asking the person in charge of the specific release to consider it.

12

If upgrading the entire Foreman installation to 1.18 is not an option, I think that just upgrading the proxy to 1.18 and keeping Foreman at 1.17 is a good alternative. I’m not aware of any incompatibilities and generally the API between them is very stable.

13

I’m not totally sure I understand why, but rebooting seemed to have resolved the issue. I didn’t reboot for the purpose of fixing the problem, just installing kernel updates. but it as far as I can tell it’s not throwing the error anymore.

That said, our internal mirrors will be updated this weekend, so I will update to 1.18 next week. On a side note, I incorrectly posted that I was running 1.17.1. The RPMs installed were on 1.17.3, but the login page showed it’s version as 1.17.1.

14

1.18.1 should hopefully be out by the weekend, so be sure to upgrade to that - it contains a whole bunch of bug fixes.

15

DUID is regenerated I think each reastart and it may or may not contain the faulty " character.