Page MenuHomeFreeBSD
Differential D17115

Generate /etc/services from the IANA registry (plus local mods)
Needs ReviewPublic
Actions

Authored by vangyzen on Sep 10 2018, 9:47 PM.
Tags
None
Referenced Files
F82614185: D17115.id.diff
Tue, Apr 30, 9:37 PM
F82588168: D17115.id47899.diff
Tue, Apr 30, 4:42 PM
F82578804: D17115.diff
Tue, Apr 30, 1:57 PM
F82575939: D17115.id47897.diff
Tue, Apr 30, 12:59 PM
F82574990: D17115.id47897.diff
Tue, Apr 30, 12:37 PM
Unknown Object (File)
Tue, Apr 30, 12:09 AM
Unknown Object (File)
Mon, Apr 29, 8:32 AM
Unknown Object (File)
Mon, Apr 29, 7:30 AM
View All Files
Subscribers
asomers
badger
dab
eadler
jhb
pfg
rgrimes

Details

Reviewers
dab
Summary

Generate /etc/services from

https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xml

and a locally maintained local.xml.

This required creating the initial version of local.xml, which was also automated. (The code for this can be removed when we are sure it is no longer needed.) All of our current /etc/services names still exist, although the primary name and comment for a given number/protocol tuple are determined by the IANA registry by default, so they might have changed. The primary name can be overridden by local.xml, which we have done for port 2049.

/etc/services grows from 2,367 lines and 2,501 names to 11,538 lines and 12,005 names. /var/db/services.db grows from 2 MB to 4.2 MB.

The service-names-port-numbers.xml file from IANA is omitted from this review in order to make it more practical. Whether this file should be committed is an open question.

Test Plan

I manually read through the generated file. It looks reasonable.

I parsed the file with services_count.c. It parsed 12,005 names, the same number printed by services_parser.py when creating the file. I used the same program to benchmark the parser. Time grew linearly as the number of lines, and memory usage (RSS) grew by 1%.

I ran services_mkdb -q. It seemed happy.

Diff Detail

Lint
Lint Warnings
SeverityLocationCodeMessage
Warningusr.sbin/services_mkdb/services:1253SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:1254SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:9702SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:9703SPELL1Possible Spelling Mistake
Unit
No Test Coverage
Build Status
Buildable 19521
Build 19113: arc lint + arc unit

Event Timeline

vangyzen created this revision.Sep 10 2018, 9:47 PM
Herald added subscribers: dab, badger. · View Herald TranscriptSep 10 2018, 9:47 PM
Harbormaster completed remote builds in B19510: Diff 47897.Sep 10 2018, 9:47 PM
vangyzen added a comment.Sep 10 2018, 9:54 PM

This review replaces D17106. This review is more practical because it omits the service-names-port-numbers.xml file from IANA. Whether this file should be committed is an open question. This review also omits whitespace changes in the generated services file. These will be committed separately, before the other changes.

vangyzen mentioned this in D17106: Generate /etc/services from the IANA registry (plus local mods).Sep 10 2018, 9:55 PM
vangyzen updated this revision to Diff 47898.Sep 10 2018, 10:04 PM
  • Elide useless comments; avoid trailing whitespace
Harbormaster completed remote builds in B19511: Diff 47898.Sep 10 2018, 10:04 PM
vangyzen updated this revision to Diff 47899.Sep 10 2018, 10:15 PM
  • Adhere to PEP8
Harbormaster completed remote builds in B19512: Diff 47899.Sep 10 2018, 10:15 PM
asomers added a comment.Sep 10 2018, 11:21 PM

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?
  2. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?
usr.sbin/services_mkdb/services
877

Do you think it's worth trying to wrap long lines like this?

3527

nfsd is popular enough that it should probably be the default for these ports, not an alias.

7471

I would say that lockd should also be the default here.

usr.sbin/services_mkdb/services_parser.py
134

What will mergemaster do if somebody edits this file on an installed system?

vangyzen added a comment.Sep 11 2018, 1:07 AM
In D17115#364864, @asomers wrote:

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?

I doubt there has ever been such a policy. By default, we will remove it on the next update. I think this de facto policy will be sufficient. Administrators who need to keep the assignment can simply add it to this file on their systems.

  1. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?

I don't think the base system and ports have much bearing on defining what is useful in this file. A lot of other software (hopefully) runs on FreeBSD. A lot of humans would probably find a complete database to be useful, too. I expect that adding all of the official services will provide the most convenience for the most users, and will be a minor inconvenience for a small minority. That minority can edit the file to suit their needs.

usr.sbin/services_mkdb/services
877

Yes. Earlier today, I added it to my to-do list.

3527

I agree. I figured this would happen eventually. I'll add that capability to the script.

7471

This was added by r7685, which claims to "Upgrade to RFC1700 (IANA)". However, RFC 1700 does not mention that port.

The current registry assigns it only to npp, not lockd.

lockd on FreeBSD actually uses a non-fixed port < 1024 assigned by rpc.lockd.

lockd is not used by NFSv4.

Given all of the above, I don't see a need to change this. In fact, I don't see a need to keep "lockd" at all, but I'll save that for another day.

usr.sbin/services_mkdb/services_parser.py
134

The same thing it would do without my changes, and the same thing it does with other files in /etc. I'm not trying to be snarky, but this seems orthogonal to my changes.

asomers added a comment.Sep 11 2018, 3:10 AM
In D17115#364879, @vangyzen wrote:
In D17115#364864, @asomers wrote:

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?

I doubt there has ever been such a policy. By default, we will remove it on the next update. I think this de facto policy will be sufficient. Administrators who need to keep the assignment can simply add it to this file on their systems.

In that case there a lot of things in there currently that we should remove. Unfortunately, without any kind of version control info from IANA, it's hard to tell which of our local aliases were added by us versus which of them are holdovers of things that IANA removed.

  1. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?

I don't think the base system and ports have much bearing on defining what is useful in this file. A lot of other software (hopefully) runs on FreeBSD. A lot of humans would probably find a complete database to be useful, too. I expect that adding all of the official services will provide the most convenience for the most users, and will be a minor inconvenience for a small minority. That minority can edit the file to suit their needs.

Works for me.

asomers added inline comments.Sep 11 2018, 3:13 AM
usr.sbin/services_mkdb/services
7471

Good point. Sounds like removing it would be best, then.

usr.sbin/services_mkdb/services_parser.py
134

The ongoing pkgbase work is throwing a wrench into things. Mergemaster now blindly overwrites some files in /etc. It would be good to double check that this is not one of those.

vangyzen updated this revision to Diff 47906.Sep 11 2018, 1:53 PM
  • If the description is too long, wrap it to multiple lines and print it above the service
  • Let local services set the primary name
Harbormaster completed remote builds in B19516: Diff 47906.Sep 11 2018, 1:53 PM
vangyzen edited the summary of this revision. (Show Details)Sep 11 2018, 2:22 PM
vangyzen edited the test plan for this revision. (Show Details)
vangyzen edited the summary of this revision. (Show Details)
asomers added inline comments.Sep 11 2018, 2:41 PM
usr.sbin/services_mkdb/services
10154

These leading spaces are super-ugly. But I suppose they're in the IANA original, so it's reasonable to leave them in place.

11718

Also a really unfortunate use of whitespace in the original.

vangyzen updated this revision to Diff 47907.Sep 11 2018, 2:49 PM
  • Update the documentation for local.xml.
Harbormaster completed remote builds in B19517: Diff 47907.Sep 11 2018, 2:49 PM
vangyzen added a comment.Sep 11 2018, 3:01 PM

I think I'm finished with all outstanding changes. I would be grateful for more reviews. (Thanks for your ongoing review, @asomers.)

I plan to omit the big IANA XML file from the initial commit, then follow up on a mailing list to get opinions on how to handle it.

usr.sbin/services_mkdb/services
10154

I don't see any leading spaces.

asomers added inline comments.Sep 11 2018, 3:16 PM
usr.sbin/services_mkdb/services
10154

You don't see seven spaces between the # and targeted on line 9976?

vangyzen added inline comments.Sep 11 2018, 3:19 PM
usr.sbin/services_mkdb/services
10154

Now I see it, down on line 10164.

vangyzen updated this revision to Diff 47912.Sep 11 2018, 3:26 PM
  • Collapse multiple spaces into a single space.
Harbormaster completed remote builds in B19521: Diff 47912.Sep 11 2018, 3:26 PM
eadler added a subscriber: eadler.Sep 12 2018, 12:40 AM

I did not review closely but am supportive of the direction taken

usr.sbin/services_mkdb/services_parser.py
37

consider using https://docs.python.org/3/library/functools.html#functools.total_ordering

vangyzen added a comment.Sep 14 2018, 5:41 PM

Would anyone else care to review? I promise it won't crash your browser this time.

usr.sbin/services_mkdb/services_parser.py
37

Thanks. I hadn't heard of that.

dab added a reviewer: dab.Sep 15 2018, 11:19 PM

@vangyzen -- Did you notice this:

These changes have lint problems.
These changes have failed to build.
dab added inline comments.Sep 16 2018, 3:51 AM
usr.sbin/services_mkdb/local.xml
69

If <description> is empty,as here, must it be present in the record? If it could be omitted the specification would be just a bit more compact.

72

"null" and "sink" seem to be aliases and both have multiple protocols. It might be nice to all a single entry to specify multiple names and/or protocols for this (I think fairly common) usage to make the specification more compact. What do you think?

Something like:

<record>
  <names>null,sink</names>
  <protocols>sctp,tcp,udp</protocols>
  <description/>
  <number>9</number>
</record>
usr.sbin/services_mkdb/services
884

s/priviledged/privileged/

984

Why the change in comment from "Brunhoff remote file system" to "rfs server"?

2477

Should the "Prospero Directory Service non-priv" comment be preserved here for the alias (same on next line, too).

2486

The "cygnus bug tracker" comment was lost here for the "support" alias.

2830

The "Layer 2 Tunnelling Protocol" comment was lost here for l2tp.

2875

Lost the "Point-to-point tunnelling protocol" comment here.

3406

Lost the "MHSnet system" comment for the mshnet alias here.
Also, is it "MHSnet" or "MSHnet"?

3434

Lost "GNU finger" comment for fingerd.

pfg added a subscriber: pfg.Feb 16 2020, 11:09 PM

IANA has indeed removed some services: I think people can de-register services they don't use/develop anymore. We should remove at least Kerberos IV and netatalk, both are not in the IANA. Also IANA lists first tcp, udp and lastly sctp so perhaps its better to jeep things consistent.

I tried to do a complete update by hand, but mjg@ complained the database was too big, wrt Ubuntu and we should be much more minimalistic. In all honesty, a bunch of the services correspond to licensing schemes for proprietary applications, anonymous startups and dead services. I ended up doing what I consider is an acceptable compromise in D23671.

Revision Contents
Changeset List

PathSize
usr.sbin/
services_mkdb/
8 lines
1716 lines
12105 lines
65 lines
23 lines
2 lines
234 lines

Diff 47912

View Options

usr.sbin/services_mkdb/Makefile

Loading...
View Options

usr.sbin/services_mkdb/local.xml

Loading...
View Options

usr.sbin/services_mkdb/services

Loading...
View Options

usr.sbin/services_mkdb/services_count.c

Loading...
View Options

usr.sbin/services_mkdb/services_header

Loading...
View Options

usr.sbin/services_mkdb/services_mkdb.c

Loading...
View Options

usr.sbin/services_mkdb/services_parser.py

Loading...